date:20211112

[PATCH] D109128: [VFS] Use original path when falling back to external FS

2021-11-12 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits

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

In D109128#3097060 , @keith wrote:

> @dexonsmith turns out the test I was adding is broken on main today too. If 
> it's alright with you I will punt that to another diff to not increase the 
> scope of this one.

Yes, SGTM.

In D109128#3113058 , @keith wrote:

> @dexonsmith can you take another look?

LGTM! Thanks for seeing this through. (And thanks for your patience; this (and 
the pings) just got buried somehow in my inbox.)




Comment at: llvm/unittests/Support/VirtualFileSystemTest.cpp:1738
+
+TEST_F(VFSFromYAMLTest, RelativePathHitWithoutCWD) {
+  IntrusiveRefCntPtr BaseFS(

keith wrote:
> So this test case is actually failing. The difference between it and the 
> others is I don't call `FS->setCurrentWorkingDirectory("//root/foo")`. This 
> results in us (with my most recent change here) performing this logic:
> 
> 1. Fetch the absolute //root/foo/vfsname
> 2. This results in `realname` being returned
> 3. We attempt to canonicalize `realname`, but we have no `pwd`, so this 
> doesn't result in a valid path
> 4. everything fails past this
> 
> It seems to me, without having a ton of context here, that the value returned 
> from the VFS lookup should actually be `//root/foo/realname`, since otherwise 
> we could likely hit one of the same issues as those discussed above where if 
> you actually had this situation:
> 
> - `mkdir /root/foo /root/bar`
> - `touch /root/foo/realname /root/bar/realname`
> - `cd /root/bar`
> - lookup `/root/foo/vfsname`, get back `realname`
> - canonicalize `realname` to the wrong path `/root/bar/realname`
> 
> You'd end up with the wrong file, but I don't think this is actually new 
> behavior with my change?
> 
> @dexonsmith  wdyt?
I agree with your analysis of the correct behaviour. Until the first call to 
`setCurrentWorkingDirectory()`, it seems like the working directory should 
implicitly be the one in the underlying FS (not sure whether it should be 
captured on construction, or just used going forward).



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109128

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

[PATCH] D113393: [c++2b] Implement P0849R8 auto(x)

2021-11-12 Thread Richard Smith - zygoloid via Phabricator via cfe-commits

rsmith added inline comments.



Comment at: clang/lib/Parse/ParseDeclCXX.cpp:1032
+  // the typename-specifier in a function-style cast expression may
+  // be 'auto' since C++2b
   Diag(Tok.getLocation(),

Quuxplusone wrote:
> rsmith wrote:
> > Nice catch :)
> I see zero hits for `git grep 'decltype[(]auto[^)] clang/`. So it seems this 
> corner of the grammar has been missing any tests for a long time, but I hope 
> this PR will add some.
> ```
> decltype(auto*) i = 42; // should be a syntax error
> decltype(auto(42)) i = 42;  // should be OK 
> decltype(auto()) i = 42;  // should be a syntax error
> ```
> Right now I don't see any tests for this stuff in this PR either. So it needs 
> some.
Some of this is tested in the new file 
test/CXX/dcl.dcl/dcl.spec/dcl.type/dcl.type.auto.deduct/p2.cpp (you might need 
to expand it manually; full-page search for `decltype(auto(` might otherwise 
not find it). It looks like we are missing tests for things like 
`decltype(auto*)`. It's not obvious to me what diagnostic would be most useful 
if `decltype(auto` is not followed by `)`, `(`, or `{`, but my guess is that 
"expected `)`" pointing at the `*`, rather than an error on the `auto` token, 
would be the least surprising. So maybe this should be
```
if (Tok.is(tok::kw_auto) && NextToken().isNot(tok::l_paren, tok::l_brace)) {
```
?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113393

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

[PATCH] D109557: Adds a BlockIndent option to AlignAfterOpenBracket

2021-11-12 Thread Owen Pan via Phabricator via cfe-commits

owenpan added inline comments.



Comment at: clang/lib/Format/ContinuationIndenter.cpp:951
 
+  if (PreviousNonComment && PreviousNonComment->is(tok::l_paren)) {
+State.Stack.back().BreakBeforeClosingParen =

HazardyKnusperkeks wrote:
> Remove the braces
I think the braces should not be removed here. The [[ 
https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements
 | LLVM Coding Standards ]] says "braces should be used when a single-statement 
body is complex enough" which is rather subjective, but line wrapping is an 
objective metric and perhaps the only one we can apply.





Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109557

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

[PATCH] D113393: [c++2b] Implement P0849R8 auto(x)

2021-11-12 Thread Arthur O'Dwyer via Phabricator via cfe-commits

Quuxplusone added a comment.

Comments on tests.




Comment at: clang/lib/Parse/ParseDeclCXX.cpp:1032
+  // the typename-specifier in a function-style cast expression may
+  // be 'auto' since C++2b
   Diag(Tok.getLocation(),

rsmith wrote:
> Nice catch :)
I see zero hits for `git grep 'decltype[(]auto[^)] clang/`. So it seems this 
corner of the grammar has been missing any tests for a long time, but I hope 
this PR will add some.
```
decltype(auto*) i = 42; // should be a syntax error
decltype(auto(42)) i = 42;  // should be OK 
decltype(auto()) i = 42;  // should be a syntax error
```
Right now I don't see any tests for this stuff in this PR either. So it needs 
some.



Comment at: 
clang/test/CXX/dcl.dcl/dcl.spec/dcl.type/dcl.type.auto.deduct/p2.cpp:6-28
+  static_assert(__is_same(decltype(auto(v)), int *));
+  static_assert(__is_same(decltype(auto("lit")), char const *));
+
+  int fn(char *);
+  static_assert(__is_same(decltype(auto(fn)), int (*)(char *)));
+
+  constexpr long i = 1;

It is definitely a good idea to repeat all of these tests with braces too: 
`auto{v}`, `auto{"lit"}`, etc.



Comment at: 
clang/test/CXX/dcl.dcl/dcl.spec/dcl.type/dcl.type.auto.deduct/p2.cpp:51
+f(A(*this));// ok
+f(auto(*this)); // ok in P0849
+  }

Should you check that `__is_same(decltype(auto(*this)), A)`?
Should you check that `__is_same(decltype(auto(this)), A*)`?
Should you test inside a const member function too?
Should you check that the friend function actually can call 
`auto(some_a_object)`, the same way it can call `A(some_a_object)` — and more 
to the point, that a non-friend //can't//?



Comment at: 
clang/test/CXX/dcl.dcl/dcl.spec/dcl.type/dcl.type.auto.deduct/p2.cpp:56
+  A(const A &);
+};

I'm not sure which test file it should go in, but you definitely want a test 
somewhere proving that `auto` doesn't copy prvalues. E.g.
```
struct Uncopyable {
explicit Uncopyable(int);
Uncopyable(Uncopyable&&) = delete;
};
Uncopyable u = auto(Uncopyable(auto(Uncopyable(42;
```



Comment at: clang/test/CXX/expr/expr.post/expr.type.conv/p1-2b.cpp:17-18
+
+  foo(auto(a));
+  foo(auto {a});
+  foo(auto(a));

What is the significance of the whitespace here? Is this a lexer test?
Normally it'd be just `auto(a)`, `auto{a}` — just like `T(a)`, `T{a}`. I think 
we should follow convention (again, unless this is a lexer test — in which case 
we should probably test //both// syntaxes with and without whitespace).



Comment at: clang/test/CXX/expr/expr.post/expr.type.conv/p1-2b.cpp:25
+  foo(auto {a, 3.14});  // expected-error {{initializer for functional-style 
cast to 'auto' contains multiple expressions}}
+  foo(auto({a, 3.14})); // expected-error {{initializer for functional-style 
cast to 'auto' contains multiple expressions}}
+}

I'd also check `auto{1,2}` (ill-formed, does //not// deduce 
`initializer_list`) and `auto{{1,2}}` (also ill-formed).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113393

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

[PATCH] D113393: [c++2b] Implement P0849R8 auto(x)

2021-11-12 Thread Richard Smith - zygoloid via Phabricator via cfe-commits

rsmith added a comment.

I also wonder whether we should accept `decltype(auto)(x)` as an extension, but 
we can discuss that separately.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113393

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

[PATCH] D113393: [c++2b] Implement P0849R8 auto(x)

2021-11-12 Thread Richard Smith - zygoloid via Phabricator via cfe-commits

rsmith added a comment.

Thanks, this generally looks great.

It looks like we'll need some additional work on disambiguation to handle cases 
like:

  struct A { int n; } a;
  void f() { auto()->n = 0; }

I think that's valid, but right now we misparse it as a declaration of a 
variable ``. (This syntax also resembles a function declaration with a 
trailing return type, but that interpretation should be rejected for various 
reasons, such as because `n` isn't a type.) This disambiguation failure seems 
to be specific to `auto`; this similar case is parsed properly:

  struct A { int n; } a;
  using T = A*;
  void f() { T()->n = 1; }




Comment at: clang/lib/Parse/ParseDeclCXX.cpp:1032
+  // the typename-specifier in a function-style cast expression may
+  // be 'auto' since C++2b
   Diag(Tok.getLocation(),

Nice catch :)



Comment at: clang/lib/Sema/SemaExprCXX.cpp:1481
+  } else if (Deduced) {
+auto Inits = Exprs;
+if (Exprs.size() == 1) {

Please use an explicit type here.



Comment at: clang/lib/Sema/SemaExprCXX.cpp:1482-1486
+if (Exprs.size() == 1) {
+  if (auto p = dyn_cast_or_null(Exprs[0])) {
+Inits = MultiExprArg(p->getInits(), p->getNumInits());
+  }
+}

You should only do this if `ListInitialization` is `true`. Otherwise we'll 
accept the invalid syntax `auto({a})`.



Comment at: clang/test/CXX/expr/expr.post/expr.type.conv/p1-2b.cpp:19
+  foo(auto {a});
+  foo(auto(a));
+

We're missing a test that we reject `auto({a})`. I assume that's what this line 
was intended to do.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113393

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

[PATCH] D113455: [clang][objc][codegen] Skip emitting ObjC category metadata when the category is empty

2021-11-12 Thread Akira Hatanaka via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG7611e16fce9c: [clang][objc][codegen] Skip emitting ObjC 
category metadata when the (authored by guitard0g, committed by ahatanak).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113455

Files:
  clang/lib/CodeGen/CGObjCMac.cpp
  clang/test/CodeGenObjC/category-class-empty.m
  clang/test/CodeGenObjC/non-lazy-classes.m

Index: clang/test/CodeGenObjC/non-lazy-classes.m
===
--- clang/test/CodeGenObjC/non-lazy-classes.m
+++ clang/test/CodeGenObjC/non-lazy-classes.m
@@ -42,4 +42,7 @@
 @implementation E @end
 
 __attribute__((objc_nonlazy_class))
-@implementation E (MyCat) @end
+@implementation E (MyCat)
+-(void) load {
+}
+@end
Index: clang/test/CodeGenObjC/category-class-empty.m
===
--- /dev/null
+++ clang/test/CodeGenObjC/category-class-empty.m
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -triple i386-apple-darwin9 -O0 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s
+// PR7431
+
+// CHECK-NOT: @"OBJC_LABEL_CATEGORY_$" = private global [1 x i8*] [i8* bitcast (%struct._category_t* @"_OBJC_$_CATEGORY_A_$_foo"
+
+@interface A
+@end
+__attribute__((objc_direct_members))
+@interface A(foo)
+- (void)foo_myStuff;
+@end
+@implementation A(foo)
+- (void)foo_myStuff {
+}
+@end
+
Index: clang/lib/CodeGen/CGObjCMac.cpp
===
--- clang/lib/CodeGen/CGObjCMac.cpp
+++ clang/lib/CodeGen/CGObjCMac.cpp
@@ -6672,33 +6672,53 @@
 }
   }
 
-  values.add(emitMethodList(listName, MethodListType::CategoryInstanceMethods,
-instanceMethods));
-  values.add(emitMethodList(listName, MethodListType::CategoryClassMethods,
-classMethods));
+  auto instanceMethodList = emitMethodList(
+  listName, MethodListType::CategoryInstanceMethods, instanceMethods);
+  auto classMethodList = emitMethodList(
+  listName, MethodListType::CategoryClassMethods, classMethods);
+  values.add(instanceMethodList);
+  values.add(classMethodList);
+  // Keep track of whether we have actual metadata to emit.
+  bool isEmptyCategory =
+  instanceMethodList->isNullValue() && classMethodList->isNullValue();
 
   const ObjCCategoryDecl *Category =
-Interface->FindCategoryDeclaration(OCD->getIdentifier());
+  Interface->FindCategoryDeclaration(OCD->getIdentifier());
   if (Category) {
 SmallString<256> ExtName;
-llvm::raw_svector_ostream(ExtName) << Interface->getObjCRuntimeNameAsString() << "_$_"
-   << OCD->getName();
-values.add(EmitProtocolList("_OBJC_CATEGORY_PROTOCOLS_$_"
-   + Interface->getObjCRuntimeNameAsString() + "_$_"
-   + Category->getName(),
-Category->protocol_begin(),
-Category->protocol_end()));
-values.add(EmitPropertyList("_OBJC_$_PROP_LIST_" + ExtName.str(),
-OCD, Category, ObjCTypes, false));
-values.add(EmitPropertyList("_OBJC_$_CLASS_PROP_LIST_" + ExtName.str(),
-OCD, Category, ObjCTypes, true));
+llvm::raw_svector_ostream(ExtName)
+<< Interface->getObjCRuntimeNameAsString() << "_$_" << OCD->getName();
+auto protocolList =
+EmitProtocolList("_OBJC_CATEGORY_PROTOCOLS_$_" +
+ Interface->getObjCRuntimeNameAsString() + "_$_" +
+ Category->getName(),
+ Category->protocol_begin(), Category->protocol_end());
+auto propertyList = EmitPropertyList("_OBJC_$_PROP_LIST_" + ExtName.str(),
+ OCD, Category, ObjCTypes, false);
+auto classPropertyList =
+EmitPropertyList("_OBJC_$_CLASS_PROP_LIST_" + ExtName.str(), OCD,
+ Category, ObjCTypes, true);
+values.add(protocolList);
+values.add(propertyList);
+values.add(classPropertyList);
+isEmptyCategory &= protocolList->isNullValue() &&
+   propertyList->isNullValue() &&
+   classPropertyList->isNullValue();
   } else {
 values.addNullPointer(ObjCTypes.ProtocolListnfABIPtrTy);
 values.addNullPointer(ObjCTypes.PropertyListPtrTy);
 values.addNullPointer(ObjCTypes.PropertyListPtrTy);
   }
 
-  unsigned Size = CGM.getDataLayout().getTypeAllocSize(ObjCTypes.CategorynfABITy);
+  if (isEmptyCategory) {
+// Empty category, don't emit any metadata.
+values.abandon();
+MethodDefinitions.clear();
+return;
+  }
+
+  unsigned Size =
+  CGM.getDataLayout().getTypeAllocSize(ObjCTypes.CategorynfABITy);
   values.addInt(ObjCTypes.IntTy, Size);
 
   llvm::GlobalVariable *GCATV =

[clang] 7611e16 - [clang][objc][codegen] Skip emitting ObjC category metadata when the

2021-11-12 Thread Akira Hatanaka via cfe-commits


Author: Josh Learn
Date: 2021-11-12T16:21:21-08:00
New Revision: 7611e16fce9ce36b6bd6dceda691f6bc7e754347

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

LOG: [clang][objc][codegen] Skip emitting ObjC category metadata when the
category is empty

Currently, if we create a category in ObjC that is empty, we still emit
runtime metadata for that category. This is a scenario that could
commonly be run into when using __attribute__((objc_direct_members)),
which elides the need for much of the category metadata. This is
slightly wasteful and can be easily skipped by checking the category
metadata contents during CodeGen.

rdar://66177182

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

Added: 
clang/test/CodeGenObjC/category-class-empty.m

Modified: 
clang/lib/CodeGen/CGObjCMac.cpp
clang/test/CodeGenObjC/non-lazy-classes.m

Removed: 




diff  --git a/clang/lib/CodeGen/CGObjCMac.cpp b/clang/lib/CodeGen/CGObjCMac.cpp
index 36555ce3cbd9..5b925359ac25 100644
--- a/clang/lib/CodeGen/CGObjCMac.cpp
+++ b/clang/lib/CodeGen/CGObjCMac.cpp
@@ -6672,33 +6672,53 @@ void CGObjCNonFragileABIMac::GenerateCategory(const 
ObjCCategoryImplDecl *OCD) {
 }
   }
 
-  values.add(emitMethodList(listName, MethodListType::CategoryInstanceMethods,
-instanceMethods));
-  values.add(emitMethodList(listName, MethodListType::CategoryClassMethods,
-classMethods));
+  auto instanceMethodList = emitMethodList(
+  listName, MethodListType::CategoryInstanceMethods, instanceMethods);
+  auto classMethodList = emitMethodList(
+  listName, MethodListType::CategoryClassMethods, classMethods);
+  values.add(instanceMethodList);
+  values.add(classMethodList);
+  // Keep track of whether we have actual metadata to emit.
+  bool isEmptyCategory =
+  instanceMethodList->isNullValue() && classMethodList->isNullValue();
 
   const ObjCCategoryDecl *Category =
-Interface->FindCategoryDeclaration(OCD->getIdentifier());
+  Interface->FindCategoryDeclaration(OCD->getIdentifier());
   if (Category) {
 SmallString<256> ExtName;
-llvm::raw_svector_ostream(ExtName) << 
Interface->getObjCRuntimeNameAsString() << "_$_"
-   << OCD->getName();
-values.add(EmitProtocolList("_OBJC_CATEGORY_PROTOCOLS_$_"
-   + Interface->getObjCRuntimeNameAsString() + 
"_$_"
-   + Category->getName(),
-Category->protocol_begin(),
-Category->protocol_end()));
-values.add(EmitPropertyList("_OBJC_$_PROP_LIST_" + ExtName.str(),
-OCD, Category, ObjCTypes, false));
-values.add(EmitPropertyList("_OBJC_$_CLASS_PROP_LIST_" + ExtName.str(),
-OCD, Category, ObjCTypes, true));
+llvm::raw_svector_ostream(ExtName)
+<< Interface->getObjCRuntimeNameAsString() << "_$_" << OCD->getName();
+auto protocolList =
+EmitProtocolList("_OBJC_CATEGORY_PROTOCOLS_$_" +
+ Interface->getObjCRuntimeNameAsString() + "_$_" +
+ Category->getName(),
+ Category->protocol_begin(), Category->protocol_end());
+auto propertyList = EmitPropertyList("_OBJC_$_PROP_LIST_" + ExtName.str(),
+ OCD, Category, ObjCTypes, false);
+auto classPropertyList =
+EmitPropertyList("_OBJC_$_CLASS_PROP_LIST_" + ExtName.str(), OCD,
+ Category, ObjCTypes, true);
+values.add(protocolList);
+values.add(propertyList);
+values.add(classPropertyList);
+isEmptyCategory &= protocolList->isNullValue() &&
+   propertyList->isNullValue() &&
+   classPropertyList->isNullValue();
   } else {
 values.addNullPointer(ObjCTypes.ProtocolListnfABIPtrTy);
 values.addNullPointer(ObjCTypes.PropertyListPtrTy);
 values.addNullPointer(ObjCTypes.PropertyListPtrTy);
   }
 
-  unsigned Size = 
CGM.getDataLayout().getTypeAllocSize(ObjCTypes.CategorynfABITy);
+  if (isEmptyCategory) {
+// Empty category, don't emit any metadata.
+values.abandon();
+MethodDefinitions.clear();
+return;
+  }
+
+  unsigned Size =
+  CGM.getDataLayout().getTypeAllocSize(ObjCTypes.CategorynfABITy);
   values.addInt(ObjCTypes.IntTy, Size);
 
   llvm::GlobalVariable *GCATV =

diff  --git a/clang/test/CodeGenObjC/category-class-empty.m 
b/clang/test/CodeGenObjC/category-class-empty.m
new file mode 100644
index ..d89a3e7eac42
--- /dev/null
+++ b/clang/test/CodeGenObjC/category-class-empty.m
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -triple i386-apple-darwin9 -O0

[PATCH] D77598: Integral template argument suffix and cast printing

2021-11-12 Thread Richard Smith - zygoloid via Phabricator via cfe-commits

rsmith added inline comments.



Comment at: clang/lib/AST/DeclPrinter.cpp:1101
   Out << ", ";
-Args[I].print(Policy, Out);
+if (TemplOverloaded || !Params)
+  Args[I].print(Policy, Out, /*IncludeType*/ true);

dblaikie wrote:
> dblaikie wrote:
> > Looks like this (& the `TemplOverloaded` in the other function, below) is 
> > undertested.
> > 
> > Hardcoding:
> > true in this function results in no failures
> > false in this function results in 1 failure in 
> > `SemaTemplate/temp_arg_enum_printing.cpp`
> >  - this failure, at least to me, seems to not have much to do with 
> > overloading - at least syntactically foo<2> (where the parameter is an enum 
> > type) doesn't compile, not due to any overloading ambiguity, but due to a 
> > lack of implicit conversion from int to the enum type, I think? and the 
> > example doesn't look like it involves any overloading... 
> > 
> > true in the other overload results in no failures
> > false in the other overload results in no failures
> > 
> > I came across this because I was confused by how this feature works - when 
> > the suffixes are used and when they are not to better understand the 
> > implications this might have for debug info. (though I'm still generally 
> > leaning towards just always putting the suffixes on for debug info)
> > 
> > @rsmith - could you give an example of what you meant by passing in a 
> > parameter when the template is overloaded & that should use the suffix? My 
> > simple examples, like this:
> > ```
> > template
> > void f1() { }
> > template
> > void f1() { }
> > int main() {
> >   f1<3U>();
> > }
> > ```
> > That's just ambiguous, apparently, and doesn't compile despite the type 
> > suffix on the literal. If I put a parameter on one of the functions it 
> > doesn't seem to trigger the "TemplOverloaded" case - both instantiations 
> > still get un-suffixed names "f1<3>"... 
> Ping on this (posted https://reviews.llvm.org/D111477 for some further 
> discussion on the debug info side of things)
I think the `TemplOverloaded` parameter is unnecessary: passing a null `Params` 
list will result in `shouldIncludeTypeForArgument` returning `true`, which 
would have the same effect as passing in `TemplOverloaded == true`. We don't 
need two different ways to request that fully-unambiguous printing is used for 
every argument, so removing this flag and passing a null `Params` list instead 
might be a good idea.

Here's an unambiguous example where we need suffixes to identify which 
specialization we're referring to:
```
template void f() {}
void (*p)() = f<0>;
template void f() {}
void (*q)() = f<>;
```

For this, `-ast-print` prints:
```
template  void f() {
}
template<> void f<0L>() {
}
void (*p)() = f<0>;
template  void f() {
}
template<> void f<0U>() {
}
void (*q)() = f<>;
```
... where the `0U` is intended to indicate that the second specialization is 
for the second template not the first. (This `-ast-print` output isn't actually 
valid code because `f<0U>` is still ambiguous, but we've done the best we can.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77598

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

[PATCH] D113455: [clang][objc][codegen] Skip emitting ObjC category metadata when the category is empty

2021-11-12 Thread Josh Learn via Phabricator via cfe-commits

guitard0g added a comment.

@ahatanak I don't have commit access, any chance you could commit this for me?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113455

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

[PATCH] D113455: [clang][objc][codegen] Skip emitting ObjC category metadata when the category is empty

2021-11-12 Thread Akira Hatanaka via Phabricator via cfe-commits

ahatanak accepted this revision.
ahatanak added a comment.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113455

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

[PATCH] D112024: [clang] diagnose_as attribute for Fortify diagnosing like builtins.

2021-11-12 Thread George Burgess IV via Phabricator via cfe-commits

george.burgess.iv added inline comments.



Comment at: clang/test/Sema/attr-diagnose-as-builtin.c:62
+
+#ifdef __cplusplus
+template 

mbenfield wrote:
> george.burgess.iv wrote:
> > nit: can we also add a non-templated overload check in here?
> > 
> > if the diag isn't beautiful, that's fine IMO. just having a test-case to 
> > show the expected behavior would be nice
> Sorry, not sure what is being requested. By a "non-templated overload check" 
> do you mean something different from `memcpy2` above?
something like the new edit suggestion below is what i have in mind :)



Comment at: clang/test/Sema/attr-diagnose-as-builtin.c:71
+void failure_template_instantiation(int x) 
__attribute__((diagnose_as_builtin(some_templated_function, 1))) {} // 
expected-error{{'diagnose_as_builtin' attribute requires parameter 1 to be a 
builtin function}}
+#endif




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112024

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

[PATCH] D110745: Redefine deref(N) attribute family as point-in-time semantics (aka deref-at-point)

2021-11-12 Thread Philip Reames via Phabricator via cfe-commits

reames added a comment.
Herald added a subscriber: jeroen.dobbelaere.

@nikic ping on previous question.  It's been a month, and this has been LGTMed. 
 Without response, I plan to land this.

In D110745#3038848 , @xbolva00 wrote:

> This really needs to be properly benchmarked.

This has been benchmarked on every workload I care about, and shows no 
interesting regressions.   Unfortunately, those are all non-public Java 
workloads,

On the C/C++ side, I don't have a ready environment in which to run anything 
representative.  From the semantic change, I wouldn't expect C++ to show much 
difference, and besides, this is fixing a long standing fairly major 
correctness issue.  If you have particular suites you care about, please run 
them and share results.

At this point, I strongly lean towards committing and letting regressions be 
reported.  We might revert, or we might simply fix forward depending on what 
comes up


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110745

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

[PATCH] D113761: [clang][modules] NFC: Factor out path-based submodule lookup

2021-11-12 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113761

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

[PATCH] D113523: Add toggling for -fnew-infallible/-fno-new-infallible

2021-11-12 Thread Di Mo via Phabricator via cfe-commits

modimo added a comment.

Gentle ping @bruno


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113523

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

[PATCH] D113455: [clang][objc][codegen] Skip emitting ObjC category metadata when the category is empty

2021-11-12 Thread Josh Learn via Phabricator via cfe-commits

guitard0g updated this revision to Diff 386949.
guitard0g marked 5 inline comments as done.
guitard0g added a comment.

Nit: deleted empty line.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113455

Files:
  clang/lib/CodeGen/CGObjCMac.cpp
  clang/test/CodeGenObjC/category-class-empty.m
  clang/test/CodeGenObjC/non-lazy-classes.m

Index: clang/test/CodeGenObjC/non-lazy-classes.m
===
--- clang/test/CodeGenObjC/non-lazy-classes.m
+++ clang/test/CodeGenObjC/non-lazy-classes.m
@@ -42,4 +42,7 @@
 @implementation E @end
 
 __attribute__((objc_nonlazy_class))
-@implementation E (MyCat) @end
+@implementation E (MyCat)
+-(void) load {
+}
+@end
Index: clang/test/CodeGenObjC/category-class-empty.m
===
--- /dev/null
+++ clang/test/CodeGenObjC/category-class-empty.m
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -triple i386-apple-darwin9 -O0 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s
+// PR7431
+
+// CHECK-NOT: @"OBJC_LABEL_CATEGORY_$" = private global [1 x i8*] [i8* bitcast (%struct._category_t* @"_OBJC_$_CATEGORY_A_$_foo"
+
+@interface A
+@end
+__attribute__((objc_direct_members))
+@interface A(foo)
+- (void)foo_myStuff;
+@end
+@implementation A(foo)
+- (void)foo_myStuff {
+}
+@end
+
Index: clang/lib/CodeGen/CGObjCMac.cpp
===
--- clang/lib/CodeGen/CGObjCMac.cpp
+++ clang/lib/CodeGen/CGObjCMac.cpp
@@ -6672,33 +6672,53 @@
 }
   }
 
-  values.add(emitMethodList(listName, MethodListType::CategoryInstanceMethods,
-instanceMethods));
-  values.add(emitMethodList(listName, MethodListType::CategoryClassMethods,
-classMethods));
+  auto instanceMethodList = emitMethodList(
+  listName, MethodListType::CategoryInstanceMethods, instanceMethods);
+  auto classMethodList = emitMethodList(
+  listName, MethodListType::CategoryClassMethods, classMethods);
+  values.add(instanceMethodList);
+  values.add(classMethodList);
+  // Keep track of whether we have actual metadata to emit.
+  bool isEmptyCategory =
+  instanceMethodList->isNullValue() && classMethodList->isNullValue();
 
   const ObjCCategoryDecl *Category =
-Interface->FindCategoryDeclaration(OCD->getIdentifier());
+  Interface->FindCategoryDeclaration(OCD->getIdentifier());
   if (Category) {
 SmallString<256> ExtName;
-llvm::raw_svector_ostream(ExtName) << Interface->getObjCRuntimeNameAsString() << "_$_"
-   << OCD->getName();
-values.add(EmitProtocolList("_OBJC_CATEGORY_PROTOCOLS_$_"
-   + Interface->getObjCRuntimeNameAsString() + "_$_"
-   + Category->getName(),
-Category->protocol_begin(),
-Category->protocol_end()));
-values.add(EmitPropertyList("_OBJC_$_PROP_LIST_" + ExtName.str(),
-OCD, Category, ObjCTypes, false));
-values.add(EmitPropertyList("_OBJC_$_CLASS_PROP_LIST_" + ExtName.str(),
-OCD, Category, ObjCTypes, true));
+llvm::raw_svector_ostream(ExtName)
+<< Interface->getObjCRuntimeNameAsString() << "_$_" << OCD->getName();
+auto protocolList =
+EmitProtocolList("_OBJC_CATEGORY_PROTOCOLS_$_" +
+ Interface->getObjCRuntimeNameAsString() + "_$_" +
+ Category->getName(),
+ Category->protocol_begin(), Category->protocol_end());
+auto propertyList = EmitPropertyList("_OBJC_$_PROP_LIST_" + ExtName.str(),
+ OCD, Category, ObjCTypes, false);
+auto classPropertyList =
+EmitPropertyList("_OBJC_$_CLASS_PROP_LIST_" + ExtName.str(), OCD,
+ Category, ObjCTypes, true);
+values.add(protocolList);
+values.add(propertyList);
+values.add(classPropertyList);
+isEmptyCategory &= protocolList->isNullValue() &&
+   propertyList->isNullValue() &&
+   classPropertyList->isNullValue();
   } else {
 values.addNullPointer(ObjCTypes.ProtocolListnfABIPtrTy);
 values.addNullPointer(ObjCTypes.PropertyListPtrTy);
 values.addNullPointer(ObjCTypes.PropertyListPtrTy);
   }
 
-  unsigned Size = CGM.getDataLayout().getTypeAllocSize(ObjCTypes.CategorynfABITy);
+  if (isEmptyCategory) {
+// Empty category, don't emit any metadata.
+values.abandon();
+MethodDefinitions.clear();
+return;
+  }
+
+  unsigned Size =
+  CGM.getDataLayout().getTypeAllocSize(ObjCTypes.CategorynfABITy);
   values.addInt(ObjCTypes.IntTy, Size);
 
   llvm::GlobalVariable *GCATV =
___
cfe-commits mailing list

[PATCH] D113455: [clang][objc][codegen] Skip emitting ObjC category metadata when the category is empty

2021-11-12 Thread Josh Learn via Phabricator via cfe-commits

guitard0g marked 6 inline comments as done.
guitard0g added inline comments.



Comment at: clang/lib/CodeGen/CGObjCMac.cpp:6683
+  values.add(classMethodList);
+  isEmptyCategory &=
+  instanceMethodList->isNullValue() && classMethodList->isNullValue();

zoecarver wrote:
> Does this really need to be an `&=`? Is there any case where 
> `isEmptyCategory` isn't `true` here?
Good point, fixed. 



Comment at: clang/lib/CodeGen/CGObjCMac.cpp:6713
   }
 
+  unsigned Size =

ahatanak wrote:
> Is it not possible to check whether the category is empty here? If it's 
> empty, you can avoid creating the global variable and abandon the builder.
Good point, done. 



Comment at: clang/test/CodeGenObjC/category-class-empty.m:1
+// RUN: %clang_cc1 -triple i386-apple-darwin9 -O3 -emit-llvm -o - %s | 
FileCheck %s
+// PR7431

jkorous wrote:
> zoecarver wrote:
> > Is `-O3` needed here?
> I would even think `-O0` is better and consider also `-disable-llvm-passes`. 
> Ultimately the goal is to make sure frontend codegen doesn't emit the 
> metadata so the less processing of the IR in the backend the more sensitive 
> the test will be.
Good catch, changed to -O0 with -disable-llvm-passes. 



Comment at: clang/test/CodeGenObjC/category-class-empty.m:10
+@interface A(foo)
+- (void)foo_myStuff;
+@end

jkorous wrote:
> I assume all the cases when we want to emit the metadata (at least one 
> instance method, at least one class method, ...) are covered by other tests, 
> right?
> If there's a case that's missing we should add a test for it.
Yeah a fair number of tests would break if we started removing categories with 
anything inside of them. 



Comment at: clang/test/CodeGenObjC/non-lazy-classes.m:45
 __attribute__((objc_nonlazy_class))
-@implementation E (MyCat) @end
+@implementation E (MyCat)
+-(void) load {

zoecarver wrote:
> This is to prevent another test from failing? Makes sense. 
Yeah this is the one case where the IRGen check was looking for the category 
even though it's empty. Should be a harmless change. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113455

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

[PATCH] D113455: [clang][objc][codegen] Skip emitting ObjC category metadata when the category is empty

2021-11-12 Thread Josh Learn via Phabricator via cfe-commits

guitard0g updated this revision to Diff 386947.
guitard0g added a comment.

Abandon builder earlier before creating global variable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113455

Files:
  clang/lib/CodeGen/CGObjCMac.cpp
  clang/test/CodeGenObjC/category-class-empty.m
  clang/test/CodeGenObjC/non-lazy-classes.m

Index: clang/test/CodeGenObjC/non-lazy-classes.m
===
--- clang/test/CodeGenObjC/non-lazy-classes.m
+++ clang/test/CodeGenObjC/non-lazy-classes.m
@@ -42,4 +42,7 @@
 @implementation E @end
 
 __attribute__((objc_nonlazy_class))
-@implementation E (MyCat) @end
+@implementation E (MyCat)
+-(void) load {
+}
+@end
Index: clang/test/CodeGenObjC/category-class-empty.m
===
--- /dev/null
+++ clang/test/CodeGenObjC/category-class-empty.m
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -triple i386-apple-darwin9 -O0 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s
+// PR7431
+
+// CHECK-NOT: @"OBJC_LABEL_CATEGORY_$" = private global [1 x i8*] [i8* bitcast (%struct._category_t* @"_OBJC_$_CATEGORY_A_$_foo"
+
+@interface A
+@end
+__attribute__((objc_direct_members))
+@interface A(foo)
+- (void)foo_myStuff;
+@end
+@implementation A(foo)
+- (void)foo_myStuff {
+}
+@end
+
Index: clang/lib/CodeGen/CGObjCMac.cpp
===
--- clang/lib/CodeGen/CGObjCMac.cpp
+++ clang/lib/CodeGen/CGObjCMac.cpp
@@ -6672,37 +6672,58 @@
 }
   }
 
-  values.add(emitMethodList(listName, MethodListType::CategoryInstanceMethods,
-instanceMethods));
-  values.add(emitMethodList(listName, MethodListType::CategoryClassMethods,
-classMethods));
+  auto instanceMethodList = emitMethodList(
+  listName, MethodListType::CategoryInstanceMethods, instanceMethods);
+  auto classMethodList = emitMethodList(
+  listName, MethodListType::CategoryClassMethods, classMethods);
+  values.add(instanceMethodList);
+  values.add(classMethodList);
+  // Keep track of whether we have actual metadata to emit.
+  bool isEmptyCategory =
+  instanceMethodList->isNullValue() && classMethodList->isNullValue();
 
   const ObjCCategoryDecl *Category =
-Interface->FindCategoryDeclaration(OCD->getIdentifier());
+  Interface->FindCategoryDeclaration(OCD->getIdentifier());
   if (Category) {
 SmallString<256> ExtName;
-llvm::raw_svector_ostream(ExtName) << Interface->getObjCRuntimeNameAsString() << "_$_"
-   << OCD->getName();
-values.add(EmitProtocolList("_OBJC_CATEGORY_PROTOCOLS_$_"
-   + Interface->getObjCRuntimeNameAsString() + "_$_"
-   + Category->getName(),
-Category->protocol_begin(),
-Category->protocol_end()));
-values.add(EmitPropertyList("_OBJC_$_PROP_LIST_" + ExtName.str(),
-OCD, Category, ObjCTypes, false));
-values.add(EmitPropertyList("_OBJC_$_CLASS_PROP_LIST_" + ExtName.str(),
-OCD, Category, ObjCTypes, true));
+llvm::raw_svector_ostream(ExtName)
+<< Interface->getObjCRuntimeNameAsString() << "_$_" << OCD->getName();
+auto protocolList =
+EmitProtocolList("_OBJC_CATEGORY_PROTOCOLS_$_" +
+ Interface->getObjCRuntimeNameAsString() + "_$_" +
+ Category->getName(),
+ Category->protocol_begin(), Category->protocol_end());
+auto propertyList = EmitPropertyList("_OBJC_$_PROP_LIST_" + ExtName.str(),
+ OCD, Category, ObjCTypes, false);
+auto classPropertyList =
+EmitPropertyList("_OBJC_$_CLASS_PROP_LIST_" + ExtName.str(), OCD,
+ Category, ObjCTypes, true);
+values.add(protocolList);
+values.add(propertyList);
+values.add(classPropertyList);
+isEmptyCategory &= protocolList->isNullValue() &&
+   propertyList->isNullValue() &&
+   classPropertyList->isNullValue();
   } else {
 values.addNullPointer(ObjCTypes.ProtocolListnfABIPtrTy);
 values.addNullPointer(ObjCTypes.PropertyListPtrTy);
 values.addNullPointer(ObjCTypes.PropertyListPtrTy);
   }
 
-  unsigned Size = CGM.getDataLayout().getTypeAllocSize(ObjCTypes.CategorynfABITy);
+  if (isEmptyCategory) {
+// Empty category, don't emit any metadata.
+values.abandon();
+MethodDefinitions.clear();
+return;
+  }
+
+  unsigned Size =
+  CGM.getDataLayout().getTypeAllocSize(ObjCTypes.CategorynfABITy);
   values.addInt(ObjCTypes.IntTy, Size);
 
   llvm::GlobalVariable *GCATV =
   finishAndCreateGlobal(values, ExtCatName.str(), CGM);
+

[PATCH] D113455: [clang][objc][codegen] Skip emitting ObjC category metadata when the category is empty

2021-11-12 Thread Josh Learn via Phabricator via cfe-commits

guitard0g updated this revision to Diff 386943.
guitard0g added a comment.

Address comments and fix fragile test to use -O0 and -disable-llvm-passes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113455

Files:
  clang/lib/CodeGen/CGObjCMac.cpp
  clang/test/CodeGenObjC/category-class-empty.m
  clang/test/CodeGenObjC/non-lazy-classes.m

Index: clang/test/CodeGenObjC/non-lazy-classes.m
===
--- clang/test/CodeGenObjC/non-lazy-classes.m
+++ clang/test/CodeGenObjC/non-lazy-classes.m
@@ -42,4 +42,7 @@
 @implementation E @end
 
 __attribute__((objc_nonlazy_class))
-@implementation E (MyCat) @end
+@implementation E (MyCat)
+-(void) load {
+}
+@end
Index: clang/test/CodeGenObjC/category-class-empty.m
===
--- /dev/null
+++ clang/test/CodeGenObjC/category-class-empty.m
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -triple i386-apple-darwin9 -O0 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s
+// PR7431
+
+// CHECK-NOT: @"OBJC_LABEL_CATEGORY_$" = private global [1 x i8*] [i8* bitcast (%struct._category_t* @"_OBJC_$_CATEGORY_A_$_foo"
+
+@interface A
+@end
+__attribute__((objc_direct_members))
+@interface A(foo)
+- (void)foo_myStuff;
+@end
+@implementation A(foo)
+- (void)foo_myStuff {
+}
+@end
+
Index: clang/lib/CodeGen/CGObjCMac.cpp
===
--- clang/lib/CodeGen/CGObjCMac.cpp
+++ clang/lib/CodeGen/CGObjCMac.cpp
@@ -6672,46 +6672,62 @@
 }
   }
 
-  values.add(emitMethodList(listName, MethodListType::CategoryInstanceMethods,
-instanceMethods));
-  values.add(emitMethodList(listName, MethodListType::CategoryClassMethods,
-classMethods));
+  auto instanceMethodList = emitMethodList(
+  listName, MethodListType::CategoryInstanceMethods, instanceMethods);
+  auto classMethodList = emitMethodList(
+  listName, MethodListType::CategoryClassMethods, classMethods);
+  values.add(instanceMethodList);
+  values.add(classMethodList);
+  // Keep track of whether we have actual metadata to emit.
+  bool isEmptyCategory =
+  instanceMethodList->isNullValue() && classMethodList->isNullValue();
 
   const ObjCCategoryDecl *Category =
-Interface->FindCategoryDeclaration(OCD->getIdentifier());
+  Interface->FindCategoryDeclaration(OCD->getIdentifier());
   if (Category) {
 SmallString<256> ExtName;
-llvm::raw_svector_ostream(ExtName) << Interface->getObjCRuntimeNameAsString() << "_$_"
-   << OCD->getName();
-values.add(EmitProtocolList("_OBJC_CATEGORY_PROTOCOLS_$_"
-   + Interface->getObjCRuntimeNameAsString() + "_$_"
-   + Category->getName(),
-Category->protocol_begin(),
-Category->protocol_end()));
-values.add(EmitPropertyList("_OBJC_$_PROP_LIST_" + ExtName.str(),
-OCD, Category, ObjCTypes, false));
-values.add(EmitPropertyList("_OBJC_$_CLASS_PROP_LIST_" + ExtName.str(),
-OCD, Category, ObjCTypes, true));
+llvm::raw_svector_ostream(ExtName)
+<< Interface->getObjCRuntimeNameAsString() << "_$_" << OCD->getName();
+auto protocolList =
+EmitProtocolList("_OBJC_CATEGORY_PROTOCOLS_$_" +
+ Interface->getObjCRuntimeNameAsString() + "_$_" +
+ Category->getName(),
+ Category->protocol_begin(), Category->protocol_end());
+auto propertyList = EmitPropertyList("_OBJC_$_PROP_LIST_" + ExtName.str(),
+ OCD, Category, ObjCTypes, false);
+auto classPropertyList =
+EmitPropertyList("_OBJC_$_CLASS_PROP_LIST_" + ExtName.str(), OCD,
+ Category, ObjCTypes, true);
+values.add(protocolList);
+values.add(propertyList);
+values.add(classPropertyList);
+isEmptyCategory &= protocolList->isNullValue() &&
+   propertyList->isNullValue() &&
+   classPropertyList->isNullValue();
   } else {
 values.addNullPointer(ObjCTypes.ProtocolListnfABIPtrTy);
 values.addNullPointer(ObjCTypes.PropertyListPtrTy);
 values.addNullPointer(ObjCTypes.PropertyListPtrTy);
   }
 
-  unsigned Size = CGM.getDataLayout().getTypeAllocSize(ObjCTypes.CategorynfABITy);
+  unsigned Size =
+  CGM.getDataLayout().getTypeAllocSize(ObjCTypes.CategorynfABITy);
   values.addInt(ObjCTypes.IntTy, Size);
 
   llvm::GlobalVariable *GCATV =
   finishAndCreateGlobal(values, ExtCatName.str(), CGM);
-  CGM.addCompilerUsedGlobal(GCATV);
-  if (Interface->hasAttr())
-DefinedStubCategories.push_back(GCATV);
-  else
-DefinedCategories.push_back(GCATV);
 
-

[PATCH] D113636: format_arg attribute does not support nullable instancetype return type

2021-11-12 Thread Félix Cloutier via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG12ab3e6c8402: format_arg attribute does not support nullable 
instancetype return type (authored by fcloutier).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113636

Files:
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/SemaObjC/format-arg-attribute.m


Index: clang/test/SemaObjC/format-arg-attribute.m
===
--- clang/test/SemaObjC/format-arg-attribute.m
+++ clang/test/SemaObjC/format-arg-attribute.m
@@ -2,7 +2,10 @@
 
 @interface NSString
 +(instancetype)stringWithCString:(const char *)cstr 
__attribute__((format_arg(1)));
-+(instancetype)stringWithString:(NSString *)cstr 
__attribute__((format_arg(1)));
+-(instancetype)initWithString:(NSString *)str __attribute__((format_arg(1)));
+
++(instancetype _Nonnull)nonNullableString:(NSString *)str 
__attribute__((format_arg(1)));
++(instancetype _Nullable)nullableString:(NSString *)str 
__attribute__((format_arg(1)));
 @end
 
 @protocol MaybeString
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -3389,7 +3389,8 @@
   }
   Ty = getFunctionOrMethodResultType(D);
   // replace instancetype with the class type
-  if (Ty.getTypePtr() == S.Context.getObjCInstanceTypeDecl()->getTypeForDecl())
+  auto Instancetype = S.Context.getObjCInstanceTypeDecl()->getTypeForDecl();
+  if (Ty->getAs() == Instancetype)
 if (auto *OMD = dyn_cast(D))
   if (auto *Interface = OMD->getClassInterface())
 Ty = S.Context.getObjCObjectPointerType(


Index: clang/test/SemaObjC/format-arg-attribute.m
===
--- clang/test/SemaObjC/format-arg-attribute.m
+++ clang/test/SemaObjC/format-arg-attribute.m
@@ -2,7 +2,10 @@
 
 @interface NSString
 +(instancetype)stringWithCString:(const char *)cstr __attribute__((format_arg(1)));
-+(instancetype)stringWithString:(NSString *)cstr __attribute__((format_arg(1)));
+-(instancetype)initWithString:(NSString *)str __attribute__((format_arg(1)));
+
++(instancetype _Nonnull)nonNullableString:(NSString *)str __attribute__((format_arg(1)));
++(instancetype _Nullable)nullableString:(NSString *)str __attribute__((format_arg(1)));
 @end
 
 @protocol MaybeString
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -3389,7 +3389,8 @@
   }
   Ty = getFunctionOrMethodResultType(D);
   // replace instancetype with the class type
-  if (Ty.getTypePtr() == S.Context.getObjCInstanceTypeDecl()->getTypeForDecl())
+  auto Instancetype = S.Context.getObjCInstanceTypeDecl()->getTypeForDecl();
+  if (Ty->getAs() == Instancetype)
 if (auto *OMD = dyn_cast(D))
   if (auto *Interface = OMD->getClassInterface())
 Ty = S.Context.getObjCObjectPointerType(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 12ab3e6 - format_arg attribute does not support nullable instancetype return type

2021-11-12 Thread Félix Cloutier via cfe-commits


Author: Félix Cloutier
Date: 2021-11-12T13:35:43-08:00
New Revision: 12ab3e6c8402078f58959847277858eb47a43a19

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

LOG: format_arg attribute does not support nullable instancetype return type

* The format_arg attribute tells the compiler that the attributed function
  returns a format string that is compatible with a format string that is being
  passed as a specific argument.
* Several NSString methods return copies of their input, so they would ideally
  have the format_arg attribute. A previous differential (D112670) added
  support for instancetype methods having the format_arg attribute when used
  in the context of NSString method declarations.
* D112670 failed to account that instancetype can be sugared in certain narrow
  (but critical) scenarios, like by using nullability specifiers. This patch
  resolves this problem.

Differential Revision: https://reviews.llvm.org/D113636
Reviewed By: ahatanak

Radar-Id: rdar://85278860

Added: 


Modified: 
clang/lib/Sema/SemaDeclAttr.cpp
clang/test/SemaObjC/format-arg-attribute.m

Removed: 




diff  --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index 743b292d29759..ef889a36bd55c 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -3389,7 +3389,8 @@ static void handleFormatArgAttr(Sema , Decl *D, const 
ParsedAttr ) {
   }
   Ty = getFunctionOrMethodResultType(D);
   // replace instancetype with the class type
-  if (Ty.getTypePtr() == S.Context.getObjCInstanceTypeDecl()->getTypeForDecl())
+  auto Instancetype = S.Context.getObjCInstanceTypeDecl()->getTypeForDecl();
+  if (Ty->getAs() == Instancetype)
 if (auto *OMD = dyn_cast(D))
   if (auto *Interface = OMD->getClassInterface())
 Ty = S.Context.getObjCObjectPointerType(

diff  --git a/clang/test/SemaObjC/format-arg-attribute.m 
b/clang/test/SemaObjC/format-arg-attribute.m
index f137879880768..41b416a6efd42 100644
--- a/clang/test/SemaObjC/format-arg-attribute.m
+++ b/clang/test/SemaObjC/format-arg-attribute.m
@@ -2,7 +2,10 @@
 
 @interface NSString
 +(instancetype)stringWithCString:(const char *)cstr 
__attribute__((format_arg(1)));
-+(instancetype)stringWithString:(NSString *)cstr 
__attribute__((format_arg(1)));
+-(instancetype)initWithString:(NSString *)str __attribute__((format_arg(1)));
+
++(instancetype _Nonnull)nonNullableString:(NSString *)str 
__attribute__((format_arg(1)));
++(instancetype _Nullable)nullableString:(NSString *)str 
__attribute__((format_arg(1)));
 @end
 
 @protocol MaybeString



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

[PATCH] D113804: [clang-tidy] Fix behavior of `modernize-use-using` with nested structs/unions

2021-11-12 Thread Fabian Wolff via Phabricator via cfe-commits

fwolff created this revision.
fwolff added reviewers: alexfh, whisperity.
fwolff added a project: clang-tools-extra.
Herald added subscribers: carlosgalvezp, rnkovacs, xazax.hun.
fwolff requested review of this revision.
Herald added a subscriber: cfe-commits.

Fixes PR#50990.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D113804

Files:
  clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
  clang-tools-extra/clang-tidy/modernize/UseUsingCheck.h
  clang-tools-extra/test/clang-tidy/checkers/modernize-use-using.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/modernize-use-using.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize-use-using.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-use-using.cpp
@@ -302,3 +302,7 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use 'using' instead of 'typedef'
   // CHECK-FIXES: using b = InjectedClassNameWithUnnamedArgument;
 };
+
+typedef struct { int a; union { int b; }; } PR50990;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
+// CHECK-FIXES: using PR50990 = struct { int a; union { int b; }; };
Index: clang-tools-extra/clang-tidy/modernize/UseUsingCheck.h
===
--- clang-tools-extra/clang-tidy/modernize/UseUsingCheck.h
+++ clang-tools-extra/clang-tidy/modernize/UseUsingCheck.h
@@ -10,6 +10,7 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USE_USING_H
 
 #include "../ClangTidyCheck.h"
+#include 
 
 namespace clang {
 namespace tidy {
@@ -23,7 +24,8 @@
 
   const bool IgnoreMacros;
   SourceLocation LastReplacementEnd;
-  SourceRange LastTagDeclRange;
+  std::map LastTagDeclRanges;
+
   std::string FirstTypedefType;
   std::string FirstTypedefName;
 
Index: clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
===
--- clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
@@ -25,19 +25,42 @@
 }
 
 void UseUsingCheck::registerMatchers(MatchFinder *Finder) {
-  Finder->addMatcher(typedefDecl(unless(isInstantiated())).bind("typedef"),
+  Finder->addMatcher(typedefDecl(unless(isInstantiated()),
+ hasParent(decl().bind("parent-decl")))
+ .bind("typedef"),
  this);
-  // This matcher used to find tag declarations in source code within typedefs.
-  // They appear in the AST just *prior* to the typedefs.
-  Finder->addMatcher(tagDecl(unless(isImplicit())).bind("tagdecl"), this);
+
+  // This matcher is used to find tag declarations in source code within
+  // typedefs. They appear in the AST just *prior* to the typedefs.
+  Finder->addMatcher(
+  tagDecl(
+  anyOf(allOf(unless(anyOf(isImplicit(),
+   classTemplateSpecializationDecl())),
+  hasParent(decl().bind("parent-decl"))),
+// We want the parent of the ClassTemplateDecl, not the parent
+// of the specialization.
+classTemplateSpecializationDecl(hasAncestor(
+classTemplateDecl(hasParent(decl().bind("parent-decl")))
+  .bind("tagdecl"),
+  this);
 }
 
 void UseUsingCheck::check(const MatchFinder::MatchResult ) {
+  const auto *ParentDecl = Result.Nodes.getNodeAs("parent-decl");
+  if (!ParentDecl) {
+return;
+  }
+
   // Match CXXRecordDecl only to store the range of the last non-implicit full
   // declaration, to later check whether it's within the typdef itself.
   const auto *MatchedTagDecl = Result.Nodes.getNodeAs("tagdecl");
   if (MatchedTagDecl) {
-LastTagDeclRange = MatchedTagDecl->getSourceRange();
+// It is not sufficient to just track the last TagDecl that we've seen,
+// because if one struct or union is nested inside another, the last TagDecl
+// before the typedef will be the nested one (PR#50990). Therefore, we also
+// keep track of the parent declaration, so that we can look up the last
+// TagDecl that is a sibling of the typedef in the AST.
+LastTagDeclRanges[ParentDecl] = MatchedTagDecl->getSourceRange();
 return;
   }
 
@@ -102,12 +125,14 @@
   auto Diag = diag(ReplaceRange.getBegin(), UseUsingWarning);
 
   // If typedef contains a full tag declaration, extract its full text.
-  if (LastTagDeclRange.isValid() &&
-  ReplaceRange.fullyContains(LastTagDeclRange)) {
+  auto LastTagDeclRange = LastTagDeclRanges.find(ParentDecl);
+  if (LastTagDeclRange != LastTagDeclRanges.end() &&
+  LastTagDeclRange->second.isValid() &&
+  ReplaceRange.fullyContains(LastTagDeclRange->second)) {
 bool Invalid;
-Type = std::string(
-Lexer::getSourceText(CharSourceRange::getTokenRange(LastTagDeclRange),
- *Result.SourceManager, getLangOpts(), ));
+Type =

[PATCH] D113775: [clang][modules] Umbrella with missing submodule: unify implicit & explicit

2021-11-12 Thread Volodymyr Sapsai via Phabricator via cfe-commits

vsapsai added inline comments.



Comment at: clang/test/Modules/missing-submodule.m:3
 // RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -F 
%S/Inputs %s -verify
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -F 
%S/Inputs %s -verify
 #include  // expected-warning{{missing submodule 
'Module.NotInModule'}}

Haven't checked the implementation but does the test cover any new behavior? 
Based on description it should test explicit modules. But the added line is 
testing `-fimplicit-module-maps`, just using the system-wide shared modules 
cache.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113775

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

[PATCH] D113645: [clangd] Allow Unix config paths on Darwin

2021-11-12 Thread Keith Smiley via Phabricator via cfe-commits

keith added a comment.

In D113645#3126652 , @kadircet wrote:

> I got a couple of concerns about the general ideas in the change. Even though 
> the convenience of using ~/.config/clangd/config.yaml seems nice, I think 
> it'll just end up creating confusion in the long run.
>
> According to various macOS developer documentation & QAs, canonical user 
> config directory for applications are `~/Library/Preferences` (see 
> https://developer.apple.com/library/archive/qa/qa1170/_index.html#//apple_ref/doc/uid/DTS10001702-CH1-SECTION3
>  and various others).

While this makes sense, I think the practical realities today are that 
`~/.config` is always preferred by unix style tools, even when on macOS.

> Regarding XDG_CONFIG_HOME, AFAICT macOS doesn't really implement `XDG Base 
> Directory Specification`, so querying its existence in that platform seems 
> controversial.

Yea I considered still omitting that one, but figured it was fair to simplify 
the compiler conditionals and assume that if someone had that variable set on 
macOS it was intentional. But I think it would be fine to exclude.

> Another concern is around multiple user config files. Clangd's config 
> mechanism actually implements overriding of previous config options, and user 
> config file is the most authoritative one and in the case of multiple such 
> config files it's unclear which one should have precedence. So i don't think 
> we should ever use "multiple" user config files.

Yea this makes sense. Theoretically we could check these "search paths" to 
determine if the configs actually existed to make that more clear, but with 
`--log=verbose` the order of operations is clear where clangd is searching for 
these so I think that shouldn't be a common issue.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113645

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

[PATCH] D113707: [clang] Make -masm=intel affect inline asm style

2021-11-12 Thread Reid Kleckner via Phabricator via cfe-commits

rnk added a comment.

One alternative would be to control this setting with a pragma stack, like we 
do for warnings, struct packing, etc. Something like:

  // foo.h
  #pragma clang asm_dialect push "att"
  static inline ... foo { asm("..."); }
  #pragma clang asm_dialect pop

The pragma really would just control the dialect on the inline asm blob, not 
the output assembly.

It seems more user friendly since they don't have to write two variants of 
their inline asm, but it is new and not compatible with any existing code.


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

https://reviews.llvm.org/D113707

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

[PATCH] D113793: Comment Sema: Run checks only when appropriate (NFC)

2021-11-12 Thread Aaron Puchert via Phabricator via cfe-commits

aaronpuchert added a comment.

Forget to run `clang-format`, will do this when I have to update or land.




Comment at: clang/lib/AST/CommentSema.cpp:628
 
-void Sema::checkBlockCommandDuplicate(const BlockCommandComment *Command) {
-  const CommandInfo *Info = Traits.getCommandInfo(Command->getCommandID());
+void Sema::checkBlockCommandDuplicate(const BlockCommandComment *Command, 
const BlockCommandComment *Sema::*LastOccurrence) {
   const BlockCommandComment *PrevCommand = nullptr;

If you'd be happier here with a regular pointer I don't mind.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113793

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

[PATCH] D113795: Comment Sema: Eliminate or factor out DeclInfo inspection (NFC)

2021-11-12 Thread Aaron Puchert via Phabricator via cfe-commits

aaronpuchert created this revision.
aaronpuchert added a reviewer: gribozavr2.
aaronpuchert requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

We make all predicates expect an already inspected DeclInfo, and
introduce a function to run such a predicate after inspecting it.
Checks that were only used once have been inlined.

Before every call to inspectThisDecl the caller was checking IsFilled,
so we move that into the function itself.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D113795

Files:
  clang/include/clang/AST/CommentSema.h
  clang/lib/AST/CommentSema.cpp

Index: clang/lib/AST/CommentSema.cpp
===
--- clang/lib/AST/CommentSema.cpp
+++ clang/lib/AST/CommentSema.cpp
@@ -86,6 +86,12 @@
   }
 }
 
+/// \returns \c true if the declaration that this comment is attached to
+/// is a pointer to function/method/block type or has such a type.
+static bool involvesFunctionType(const DeclInfo ) {
+  return Info.involvesFunctionType();
+}
+
 ParamCommandComment *Sema::actOnParamCommandStart(
   SourceLocation LocBegin,
   SourceLocation LocEnd,
@@ -95,7 +101,7 @@
   new (Allocator) ParamCommandComment(LocBegin, LocEnd, CommandID,
   CommandMarker);
 
-  if (!involvesFunctionType())
+  if (!checkDecl(involvesFunctionType))
 Diag(Command->getLocation(),
  diag::warn_doc_param_not_attached_to_a_function_decl)
   << CommandMarker
@@ -105,23 +111,30 @@
 }
 
 void Sema::checkFunctionDeclVerbatimLine(const BlockCommandComment *Comment) {
+  if (ThisDeclInfo)
+inspectThisDecl();
   unsigned DiagSelect;
   switch (Comment->getCommandID()) {
   case CommandTraits::KCI_function:
   case CommandTraits::KCI_functiongroup:
-if (isAnyFunctionDecl() || isFunctionTemplateDecl())
+if (ThisDeclInfo && ThisDeclInfo->getKind() == DeclInfo::FunctionKind &&
+(isa(ThisDeclInfo->CurrentDecl) ||
+ isa(ThisDeclInfo->CurrentDecl)))
   return;
 DiagSelect = 0;
 break;
   case CommandTraits::KCI_method:
   case CommandTraits::KCI_methodgroup:
-if (isObjCMethodDecl())
+if (ThisDeclInfo && ThisDeclInfo->getKind() == DeclInfo::FunctionKind &&
+isa(ThisDeclInfo->CurrentDecl))
   return;
 DiagSelect = 1;
 break;
   case CommandTraits::KCI_callback:
-if (isFunctionPointerVarDecl())
-  return;
+if (ThisDeclInfo && ThisDeclInfo->getKind() == DeclInfo::VariableKind)
+  if (const auto *VD = dyn_cast(ThisDeclInfo->CurrentDecl))
+if (VD->getType()->isFunctionPointerType())
+  return;
 DiagSelect = 2;
 break;
   default:
@@ -133,35 +146,80 @@
   << Comment->getSourceRange();
 }
 
+static bool isClassOrStructDeclImpl(const Decl *D) {
+  if (auto *record = dyn_cast_or_null(D))
+return !record->isUnion();
+
+  return false;
+}
+
+/// \return \c true if the declaration that this comment is attached to
+/// declares either struct, class or tag typedef.
+static bool isClassOrStructOrTagTypedefDecl(const DeclInfo ) {
+  if (!Info.CurrentDecl)
+return false;
+
+  if (isClassOrStructDeclImpl(Info.CurrentDecl))
+return true;
+
+  if (auto *ThisTypedefDecl = dyn_cast(Info.CurrentDecl)) {
+auto UnderlyingType = ThisTypedefDecl->getUnderlyingType();
+if (auto ThisElaboratedType = dyn_cast(UnderlyingType)) {
+  auto DesugaredType = ThisElaboratedType->desugar();
+  if (auto *DesugaredTypePtr = DesugaredType.getTypePtrOrNull()) {
+if (auto *ThisRecordType = dyn_cast(DesugaredTypePtr)) {
+  return isClassOrStructDeclImpl(ThisRecordType->getAsRecordDecl());
+}
+  }
+}
+  }
+
+  return false;
+}
+
+static bool isObjCInterfaceDecl(const DeclInfo ) {
+  return isa_and_nonnull(Info.CurrentDecl);
+}
+
+static bool isObjCProtocolDecl(const DeclInfo ) {
+  return isa_and_nonnull(Info.CurrentDecl);
+}
+
 void Sema::checkContainerDeclVerbatimLine(const BlockCommandComment *Comment) {
-  switch (Comment->getCommandID()) {
+  if (ThisDeclInfo) {
+inspectThisDecl();
+switch (Comment->getCommandID()) {
 case CommandTraits::KCI_class:
-  if (isClassOrStructOrTagTypedefDecl() || isClassTemplateDecl())
+  if (isClassOrStructOrTagTypedefDecl(*ThisDeclInfo) ||
+  isa_and_nonnull(ThisDeclInfo->CurrentDecl))
 return;
   // Allow @class command on @interface declarations.
   // FIXME. Currently, \class and @class are indistinguishable. So,
   // \class is also allowed on an @interface declaration
-  if (Comment->getCommandMarker() && isObjCInterfaceDecl())
+  if (Comment->getCommandMarker() && isObjCInterfaceDecl(*ThisDeclInfo))
 return;
   break;
 case CommandTraits::KCI_interface:
-  if (isObjCInterfaceDecl())
+  if (isObjCInterfaceDecl(*ThisDeclInfo))
 return;

[PATCH] D113794: Comment Sema: Use Name from CommandInfo for HeaderDoc diagnostics (NFC)

2021-11-12 Thread Aaron Puchert via Phabricator via cfe-commits

aaronpuchert created this revision.
aaronpuchert added a reviewer: gribozavr2.
aaronpuchert requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

We don't have to figure out the name, we can just fetch it from the
metadata table. Also there is no reason to %select over the names since
they're not natural language but keywords.

Subsequently we can focus on the actual logic and merge some branches
together. Unhandled cases are now marked unreachable, but this shouldn't
have any effect as of now. (We handle all existing commands.)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D113794

Files:
  clang/include/clang/AST/CommentCommands.td
  clang/include/clang/Basic/DiagnosticCommentKinds.td
  clang/lib/AST/CommentSema.cpp

Index: clang/lib/AST/CommentSema.cpp
===
--- clang/lib/AST/CommentSema.cpp
+++ clang/lib/AST/CommentSema.cpp
@@ -107,115 +107,75 @@
 void Sema::checkFunctionDeclVerbatimLine(const BlockCommandComment *Comment) {
   unsigned DiagSelect;
   switch (Comment->getCommandID()) {
-case CommandTraits::KCI_function:
-  DiagSelect = (!isAnyFunctionDecl() && !isFunctionTemplateDecl())? 1 : 0;
-  break;
-case CommandTraits::KCI_functiongroup:
-  DiagSelect = (!isAnyFunctionDecl() && !isFunctionTemplateDecl())? 2 : 0;
-  break;
-case CommandTraits::KCI_method:
-  DiagSelect = !isObjCMethodDecl() ? 3 : 0;
-  break;
-case CommandTraits::KCI_methodgroup:
-  DiagSelect = !isObjCMethodDecl() ? 4 : 0;
-  break;
-case CommandTraits::KCI_callback:
-  DiagSelect = !isFunctionPointerVarDecl() ? 5 : 0;
-  break;
-default:
-  DiagSelect = 0;
-  break;
+  case CommandTraits::KCI_function:
+  case CommandTraits::KCI_functiongroup:
+if (isAnyFunctionDecl() || isFunctionTemplateDecl())
+  return;
+DiagSelect = 0;
+break;
+  case CommandTraits::KCI_method:
+  case CommandTraits::KCI_methodgroup:
+if (isObjCMethodDecl())
+  return;
+DiagSelect = 1;
+break;
+  case CommandTraits::KCI_callback:
+if (isFunctionPointerVarDecl())
+  return;
+DiagSelect = 2;
+break;
+  default:
+llvm_unreachable("Unhandled command with IsFunctionDeclarationCommand");
   }
-  if (DiagSelect)
-Diag(Comment->getLocation(), diag::warn_doc_function_method_decl_mismatch)
-<< Comment->getCommandMarker()
-<< (DiagSelect-1) << (DiagSelect-1)
-<< Comment->getSourceRange();
+  Diag(Comment->getLocation(), diag::warn_doc_function_method_decl_mismatch)
+  << Comment->getCommandMarker()
+  << Traits.getCommandInfo(Comment->getCommandID())->Name << DiagSelect
+  << Comment->getSourceRange();
 }
 
 void Sema::checkContainerDeclVerbatimLine(const BlockCommandComment *Comment) {
-  unsigned DiagSelect;
   switch (Comment->getCommandID()) {
 case CommandTraits::KCI_class:
-  DiagSelect =
-  (!isClassOrStructOrTagTypedefDecl() && !isClassTemplateDecl()) ? 1
- : 0;
+  if (isClassOrStructOrTagTypedefDecl() || isClassTemplateDecl())
+return;
   // Allow @class command on @interface declarations.
   // FIXME. Currently, \class and @class are indistinguishable. So,
   // \class is also allowed on an @interface declaration
-  if (DiagSelect && Comment->getCommandMarker() && isObjCInterfaceDecl())
-DiagSelect = 0;
+  if (Comment->getCommandMarker() && isObjCInterfaceDecl())
+return;
   break;
 case CommandTraits::KCI_interface:
-  DiagSelect = !isObjCInterfaceDecl() ? 2 : 0;
+  if (isObjCInterfaceDecl())
+return;
   break;
 case CommandTraits::KCI_protocol:
-  DiagSelect = !isObjCProtocolDecl() ? 3 : 0;
+  if (isObjCProtocolDecl())
+return;
   break;
 case CommandTraits::KCI_struct:
-  DiagSelect = !isClassOrStructOrTagTypedefDecl() ? 4 : 0;
+  if (isClassOrStructOrTagTypedefDecl())
+return;
   break;
 case CommandTraits::KCI_union:
-  DiagSelect = !isUnionDecl() ? 5 : 0;
+  if (isUnionDecl())
+return;
   break;
 default:
-  DiagSelect = 0;
-  break;
+  llvm_unreachable("Unhandled command with IsRecordLikeDeclarationCommand");
   }
-  if (DiagSelect)
-Diag(Comment->getLocation(), diag::warn_doc_api_container_decl_mismatch)
-<< Comment->getCommandMarker()
-<< (DiagSelect-1) << (DiagSelect-1)
-<< Comment->getSourceRange();
+  Diag(Comment->getLocation(), diag::warn_doc_api_container_decl_mismatch)
+  << Comment->getCommandMarker()
+  << Traits.getCommandInfo(Comment->getCommandID())->Name
+  << Comment->getSourceRange();
 }
 
 void Sema::checkContainerDecl(const BlockCommandComment *Comment) {
   if (isRecordLikeDecl())
 return;
-  unsigned DiagSelect;
-  switch (Comment->getCommandID()) {
-case

[PATCH] D113793: Comment Sema: Run checks only when appropriate (NFC)

2021-11-12 Thread Aaron Puchert via Phabricator via cfe-commits

aaronpuchert created this revision.
aaronpuchert added a reviewer: gribozavr2.
aaronpuchert requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Instead of checking within the checks whether they apply, we check
before calling them. This allows us to fetch the CommandInfo only once
instead of for every check. Also eliminates some redundant checks and
deduplicates code in Sema::checkBlockCommandDuplicate.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D113793

Files:
  clang/include/clang/AST/CommentSema.h
  clang/lib/AST/CommentSema.cpp

Index: clang/lib/AST/CommentSema.cpp
===
--- clang/lib/AST/CommentSema.cpp
+++ clang/lib/AST/CommentSema.cpp
@@ -55,7 +55,9 @@
   BlockCommandComment *BC = new (Allocator) BlockCommandComment(LocBegin, LocEnd,
 CommandID,
 CommandMarker);
-  checkContainerDecl(BC);
+  const CommandInfo *Info = Traits.getCommandInfo(CommandID);
+  if (Info->IsRecordLikeDetailCommand)
+checkContainerDecl(BC);
   return BC;
 }
 
@@ -67,13 +69,20 @@
 void Sema::actOnBlockCommandFinish(BlockCommandComment *Command,
ParagraphComment *Paragraph) {
   Command->setParagraph(Paragraph);
-  checkBlockCommandEmptyParagraph(Command);
-  checkBlockCommandDuplicate(Command);
+  const CommandInfo *Info = Traits.getCommandInfo(Command->getCommandID());
+  if (!Info->IsEmptyParagraphAllowed)
+checkBlockCommandEmptyParagraph(Command);
+  if (Info->IsBriefCommand)
+checkBlockCommandDuplicate(Command, ::BriefCommand);
+  else if (Info->IsHeaderfileCommand)
+checkBlockCommandDuplicate(Command, ::HeaderfileCommand);
   if (ThisDeclInfo) {
 // These checks only make sense if the comment is attached to a
 // declaration.
-checkReturnsCommand(Command);
-checkDeprecatedCommand(Command);
+if (Info->IsReturnsCommand)
+  checkReturnsCommand(Command);
+if (Info->IsDeprecatedCommand)
+  checkDeprecatedCommand(Command);
   }
 }
 
@@ -96,10 +105,6 @@
 }
 
 void Sema::checkFunctionDeclVerbatimLine(const BlockCommandComment *Comment) {
-  const CommandInfo *Info = Traits.getCommandInfo(Comment->getCommandID());
-  if (!Info->IsFunctionDeclarationCommand)
-return;
-
   unsigned DiagSelect;
   switch (Comment->getCommandID()) {
 case CommandTraits::KCI_function:
@@ -129,9 +134,6 @@
 }
 
 void Sema::checkContainerDeclVerbatimLine(const BlockCommandComment *Comment) {
-  const CommandInfo *Info = Traits.getCommandInfo(Comment->getCommandID());
-  if (!Info->IsRecordLikeDeclarationCommand)
-return;
   unsigned DiagSelect;
   switch (Comment->getCommandID()) {
 case CommandTraits::KCI_class:
@@ -168,8 +170,7 @@
 }
 
 void Sema::checkContainerDecl(const BlockCommandComment *Comment) {
-  const CommandInfo *Info = Traits.getCommandInfo(Comment->getCommandID());
-  if (!Info->IsRecordLikeDetailCommand || isRecordLikeDecl())
+  if (isRecordLikeDecl())
 return;
   unsigned DiagSelect;
   switch (Comment->getCommandID()) {
@@ -275,6 +276,7 @@
 void Sema::actOnParamCommandFinish(ParamCommandComment *Command,
ParagraphComment *Paragraph) {
   Command->setParagraph(Paragraph);
+  assert(!Traits.getCommandInfo(Command->getCommandID())->IsEmptyParagraphAllowed);
   checkBlockCommandEmptyParagraph(Command);
 }
 
@@ -358,6 +360,7 @@
 void Sema::actOnTParamCommandFinish(TParamCommandComment *Command,
 ParagraphComment *Paragraph) {
   Command->setParagraph(Paragraph);
+  assert(!Traits.getCommandInfo(Command->getCommandID())->IsEmptyParagraphAllowed);
   checkBlockCommandEmptyParagraph(Command);
 }
 
@@ -450,8 +453,12 @@
   CommandID,
   TextBegin,
   Text);
-  checkFunctionDeclVerbatimLine(VL);
-  checkContainerDeclVerbatimLine(VL);
+
+  const CommandInfo *Info = Traits.getCommandInfo(CommandID);
+  if (Info->IsFunctionDeclarationCommand)
+checkFunctionDeclVerbatimLine(VL);
+  if (Info->IsRecordLikeDeclarationCommand)
+checkContainerDeclVerbatimLine(VL);
   return VL;
 }
 
@@ -561,9 +568,6 @@
 }
 
 void Sema::checkBlockCommandEmptyParagraph(BlockCommandComment *Command) {
-  if (Traits.getCommandInfo(Command->getCommandID())->IsEmptyParagraphAllowed)
-return;
-
   ParagraphComment *Paragraph = Command->getParagraph();
   if (Paragraph->isWhitespace()) {
 SourceLocation DiagLoc;
@@ -579,9 +583,6 @@
 }
 
 void Sema::checkReturnsCommand(const BlockCommandComment *Command) {
-  if (!Traits.getCommandInfo(Command->getCommandID())->IsReturnsCommand)
-return;
-
   assert(ThisDeclInfo && "should not call this check on a bare comment");
 
   // We allow the return command for all @properties because it can be used
@@

[PATCH] D113691: Comment AST: Recognize function-like objects via return type (NFC)

2021-11-12 Thread Aaron Puchert via Phabricator via cfe-commits

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG3010883fc296: Comment AST: Recognize function-like objects 
via return type (NFC) (authored by aaronpuchert).

Changed prior to commit:
  https://reviews.llvm.org/D113691?vs=386572=386911#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113691

Files:
  clang/include/clang/AST/Comment.h
  clang/include/clang/AST/CommentSema.h
  clang/lib/AST/Comment.cpp
  clang/lib/AST/CommentSema.cpp

Index: clang/lib/AST/CommentSema.cpp
===
--- clang/lib/AST/CommentSema.cpp
+++ clang/lib/AST/CommentSema.cpp
@@ -86,7 +86,7 @@
   new (Allocator) ParamCommandComment(LocBegin, LocEnd, CommandID,
   CommandMarker);
 
-  if (!isFunctionDecl())
+  if (!involvesFunctionType())
 Diag(Command->getLocation(),
  diag::warn_doc_param_not_attached_to_a_function_decl)
   << CommandMarker
@@ -588,7 +588,7 @@
   // to document the value that the property getter returns.
   if (isObjCPropertyDecl())
 return;
-  if (isFunctionDecl()) {
+  if (involvesFunctionType()) {
 assert(!ThisDeclInfo->ReturnType.isNull() &&
"should have a valid return type");
 if (ThisDeclInfo->ReturnType->isVoidType()) {
@@ -728,7 +728,7 @@
 }
 
 void Sema::resolveParamCommandIndexes(const FullComment *FC) {
-  if (!isFunctionDecl()) {
+  if (!involvesFunctionType()) {
 // We already warned that \\param commands are not attached to a function
 // decl.
 return;
@@ -816,6 +816,14 @@
   }
 }
 
+bool Sema::involvesFunctionType() {
+  if (!ThisDeclInfo)
+return false;
+  if (!ThisDeclInfo->IsFilled)
+inspectThisDecl();
+  return ThisDeclInfo->involvesFunctionType();
+}
+
 bool Sema::isFunctionDecl() {
   if (!ThisDeclInfo)
 return false;
Index: clang/lib/AST/Comment.cpp
===
--- clang/lib/AST/Comment.cpp
+++ clang/lib/AST/Comment.cpp
@@ -250,6 +250,7 @@
   IsClassMethod = !IsInstanceMethod;
 }
 IsVariadic = FD->isVariadic();
+assert(involvesFunctionType());
 break;
   }
   case Decl::ObjCMethod: {
@@ -261,6 +262,7 @@
 IsInstanceMethod = MD->isInstanceMethod();
 IsClassMethod = !IsInstanceMethod;
 IsVariadic = MD->isVariadic();
+assert(involvesFunctionType());
 break;
   }
   case Decl::FunctionTemplate: {
@@ -272,6 +274,7 @@
 ReturnType = FD->getReturnType();
 TemplateParameters = FTD->getTemplateParameters();
 IsVariadic = FD->isVariadic();
+assert(involvesFunctionType());
 break;
   }
   case Decl::ClassTemplate: {
@@ -352,11 +355,11 @@
 TypeLoc TL = TSI->getTypeLoc().getUnqualifiedLoc();
 FunctionTypeLoc FTL;
 if (getFunctionTypeLoc(TL, FTL)) {
-  Kind = FunctionKind;
   ParamVars = FTL.getParams();
   ReturnType = FTL.getReturnLoc().getType();
   if (const auto *FPT = dyn_cast(FTL.getTypePtr()))
 IsVariadic = FPT->isVariadic();
+  assert(involvesFunctionType());
 }
   }
 
Index: clang/include/clang/AST/CommentSema.h
===
--- clang/include/clang/AST/CommentSema.h
+++ clang/include/clang/AST/CommentSema.h
@@ -201,6 +201,10 @@
   /// Emit diagnostics about unknown parametrs.
   void resolveParamCommandIndexes(const FullComment *FC);
 
+  /// \returns \c true if the declaration that this comment is attached to
+  /// is a pointer to function/method/block type or has such a type.
+  bool involvesFunctionType();
+
   bool isFunctionDecl();
   bool isAnyFunctionDecl();
 
Index: clang/include/clang/AST/Comment.h
===
--- clang/include/clang/AST/Comment.h
+++ clang/include/clang/AST/Comment.h
@@ -1019,9 +1019,6 @@
 /// \li member function template,
 /// \li member function template specialization,
 /// \li ObjC method,
-/// \li variable of function pointer, member function pointer or block type,
-/// \li a typedef for a function pointer, member function pointer,
-/// ObjC block.
 FunctionKind,
 
 /// Something that we consider a "class":
@@ -1089,6 +1086,8 @@
   TemplateDeclKind getTemplateKind() const LLVM_READONLY {
 return static_cast(TemplateKind);
   }
+
+  bool involvesFunctionType() const { return !ReturnType.isNull(); }
 };
 
 /// A full comment attached to a declaration, contains block content.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D113690: Comment AST: Find out if function is variadic in DeclInfo::fill

2021-11-12 Thread Aaron Puchert via Phabricator via cfe-commits

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG4e7df1ef7b67: Comment AST: Find out if function is variadic 
in DeclInfo::fill (authored by aaronpuchert).

Changed prior to commit:
  https://reviews.llvm.org/D113690?vs=386571=386910#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113690

Files:
  clang/include/clang/AST/Comment.h
  clang/lib/AST/Comment.cpp
  clang/lib/AST/CommentSema.cpp
  clang/test/Sema/warn-documentation.cpp

Index: clang/test/Sema/warn-documentation.cpp
===
--- clang/test/Sema/warn-documentation.cpp
+++ clang/test/Sema/warn-documentation.cpp
@@ -1448,6 +1448,17 @@
  */
 using VariadicFnType2 = void (*)(int a, ...);
 
+/*!
+ * Function pointer type variable.
+ *
+ * @param a
+ * works
+ *
+ * @param ...
+ * now should work too.
+ */
+void (*variadicFnVar)(int a, ...);
+
 // expected-warning@+2 {{empty paragraph passed to '@note' command}}
 /**
 @note
Index: clang/lib/AST/CommentSema.cpp
===
--- clang/lib/AST/CommentSema.cpp
+++ clang/lib/AST/CommentSema.cpp
@@ -830,26 +830,11 @@
 }
 
 bool Sema::isFunctionOrMethodVariadic() {
-  if (!isFunctionDecl() || !ThisDeclInfo->CurrentDecl)
+  if (!ThisDeclInfo)
 return false;
-  if (const FunctionDecl *FD =
-dyn_cast(ThisDeclInfo->CurrentDecl))
-return FD->isVariadic();
-  if (const FunctionTemplateDecl *FTD =
-dyn_cast(ThisDeclInfo->CurrentDecl))
-return FTD->getTemplatedDecl()->isVariadic();
-  if (const ObjCMethodDecl *MD =
-dyn_cast(ThisDeclInfo->CurrentDecl))
-return MD->isVariadic();
-  if (const TypedefNameDecl *TD =
-  dyn_cast(ThisDeclInfo->CurrentDecl)) {
-QualType Type = TD->getUnderlyingType();
-if (Type->isFunctionPointerType() || Type->isBlockPointerType())
-  Type = Type->getPointeeType();
-if (const auto *FT = Type->getAs())
-  return FT->isVariadic();
-  }
-  return false;
+  if (!ThisDeclInfo->IsFilled)
+inspectThisDecl();
+  return ThisDeclInfo->IsVariadic;
 }
 
 bool Sema::isObjCMethodDecl() {
Index: clang/lib/AST/Comment.cpp
===
--- clang/lib/AST/Comment.cpp
+++ clang/lib/AST/Comment.cpp
@@ -210,6 +210,7 @@
   IsObjCMethod = false;
   IsInstanceMethod = false;
   IsClassMethod = false;
+  IsVariadic = false;
   ParamVars = None;
   TemplateParameters = nullptr;
 
@@ -248,6 +249,7 @@
   IsInstanceMethod = MD->isInstance();
   IsClassMethod = !IsInstanceMethod;
 }
+IsVariadic = FD->isVariadic();
 break;
   }
   case Decl::ObjCMethod: {
@@ -258,6 +260,7 @@
 IsObjCMethod = true;
 IsInstanceMethod = MD->isInstanceMethod();
 IsClassMethod = !IsInstanceMethod;
+IsVariadic = MD->isVariadic();
 break;
   }
   case Decl::FunctionTemplate: {
@@ -268,6 +271,7 @@
 ParamVars = FD->parameters();
 ReturnType = FD->getReturnType();
 TemplateParameters = FTD->getTemplateParameters();
+IsVariadic = FD->isVariadic();
 break;
   }
   case Decl::ClassTemplate: {
@@ -351,6 +355,8 @@
   Kind = FunctionKind;
   ParamVars = FTL.getParams();
   ReturnType = FTL.getReturnLoc().getType();
+  if (const auto *FPT = dyn_cast(FTL.getTypePtr()))
+IsVariadic = FPT->isVariadic();
 }
   }
 
Index: clang/include/clang/AST/Comment.h
===
--- clang/include/clang/AST/Comment.h
+++ clang/include/clang/AST/Comment.h
@@ -1077,6 +1077,9 @@
   /// Can be true only if \c IsFunctionDecl is true.
   unsigned IsClassMethod : 1;
 
+  /// Is \c CommentDecl something we consider a "function" that's variadic.
+  unsigned IsVariadic : 1;
+
   void fill();
 
   DeclKind getKind() const LLVM_READONLY {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 3010883 - Comment AST: Recognize function-like objects via return type (NFC)

2021-11-12 Thread Aaron Puchert via cfe-commits


Author: Aaron Puchert
Date: 2021-11-12T21:11:11+01:00
New Revision: 3010883fc296619def051e0a2f97d40fb15960d7

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

LOG: Comment AST: Recognize function-like objects via return type (NFC)

Instead of pretending that function pointer type aliases or variables
are functions, and thereby losing the information that they are type
aliases or variables, respectively, we use the existence of a return
type in the DeclInfo to signify a "function-like" object.

That seems pretty natural, since it's also the return type (or parameter
list) from the DeclInfo that we compare the documentation with.

Addresses a concern voiced in D111264#3115104.

Reviewed By: gribozavr2

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

Added: 


Modified: 
clang/include/clang/AST/Comment.h
clang/include/clang/AST/CommentSema.h
clang/lib/AST/Comment.cpp
clang/lib/AST/CommentSema.cpp

Removed: 




diff  --git a/clang/include/clang/AST/Comment.h 
b/clang/include/clang/AST/Comment.h
index fff0a985028a..4184e103206d 100644
--- a/clang/include/clang/AST/Comment.h
+++ b/clang/include/clang/AST/Comment.h
@@ -1019,9 +1019,6 @@ struct DeclInfo {
 /// \li member function template,
 /// \li member function template specialization,
 /// \li ObjC method,
-/// \li variable of function pointer, member function pointer or block 
type,
-/// \li a typedef for a function pointer, member function pointer,
-/// ObjC block.
 FunctionKind,
 
 /// Something that we consider a "class":
@@ -1089,6 +1086,8 @@ struct DeclInfo {
   TemplateDeclKind getTemplateKind() const LLVM_READONLY {
 return static_cast(TemplateKind);
   }
+
+  bool involvesFunctionType() const { return !ReturnType.isNull(); }
 };
 
 /// A full comment attached to a declaration, contains block content.

diff  --git a/clang/include/clang/AST/CommentSema.h 
b/clang/include/clang/AST/CommentSema.h
index 4a5174e08bff..b4e564e81185 100644
--- a/clang/include/clang/AST/CommentSema.h
+++ b/clang/include/clang/AST/CommentSema.h
@@ -201,6 +201,10 @@ class Sema {
   /// Emit diagnostics about unknown parametrs.
   void resolveParamCommandIndexes(const FullComment *FC);
 
+  /// \returns \c true if the declaration that this comment is attached to
+  /// is a pointer to function/method/block type or has such a type.
+  bool involvesFunctionType();
+
   bool isFunctionDecl();
   bool isAnyFunctionDecl();
 

diff  --git a/clang/lib/AST/Comment.cpp b/clang/lib/AST/Comment.cpp
index aac0591b057e..fae3640d5ff7 100644
--- a/clang/lib/AST/Comment.cpp
+++ b/clang/lib/AST/Comment.cpp
@@ -250,6 +250,7 @@ void DeclInfo::fill() {
   IsClassMethod = !IsInstanceMethod;
 }
 IsVariadic = FD->isVariadic();
+assert(involvesFunctionType());
 break;
   }
   case Decl::ObjCMethod: {
@@ -261,6 +262,7 @@ void DeclInfo::fill() {
 IsInstanceMethod = MD->isInstanceMethod();
 IsClassMethod = !IsInstanceMethod;
 IsVariadic = MD->isVariadic();
+assert(involvesFunctionType());
 break;
   }
   case Decl::FunctionTemplate: {
@@ -272,6 +274,7 @@ void DeclInfo::fill() {
 ReturnType = FD->getReturnType();
 TemplateParameters = FTD->getTemplateParameters();
 IsVariadic = FD->isVariadic();
+assert(involvesFunctionType());
 break;
   }
   case Decl::ClassTemplate: {
@@ -352,11 +355,11 @@ void DeclInfo::fill() {
 TypeLoc TL = TSI->getTypeLoc().getUnqualifiedLoc();
 FunctionTypeLoc FTL;
 if (getFunctionTypeLoc(TL, FTL)) {
-  Kind = FunctionKind;
   ParamVars = FTL.getParams();
   ReturnType = FTL.getReturnLoc().getType();
   if (const auto *FPT = dyn_cast(FTL.getTypePtr()))
 IsVariadic = FPT->isVariadic();
+  assert(involvesFunctionType());
 }
   }
 

diff  --git a/clang/lib/AST/CommentSema.cpp b/clang/lib/AST/CommentSema.cpp
index 8251802482a0..087f103e4931 100644
--- a/clang/lib/AST/CommentSema.cpp
+++ b/clang/lib/AST/CommentSema.cpp
@@ -86,7 +86,7 @@ ParamCommandComment *Sema::actOnParamCommandStart(
   new (Allocator) ParamCommandComment(LocBegin, LocEnd, CommandID,
   CommandMarker);
 
-  if (!isFunctionDecl())
+  if (!involvesFunctionType())
 Diag(Command->getLocation(),
  diag::warn_doc_param_not_attached_to_a_function_decl)
   << CommandMarker
@@ -588,7 +588,7 @@ void Sema::checkReturnsCommand(const BlockCommandComment 
*Command) {
   // to document the value that the property getter returns.
   if (isObjCPropertyDecl())
 return;
-  if (isFunctionDecl()) {
+  if (involvesFunctionType()) {
 assert(!ThisDeclInfo->ReturnType.isNull() &&
"should have a valid return type");
 if (ThisDeclInfo->ReturnType->isVoidType()) {
@@

[clang] 59b1e98 - Comment Sema: Make most of CommentSema private (NFC)

2021-11-12 Thread Aaron Puchert via cfe-commits


Author: Aaron Puchert
Date: 2021-11-12T21:11:52+01:00
New Revision: 59b1e98137e961a61ffaeb609b6790999c461bec

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

LOG: Comment Sema: Make most of CommentSema private (NFC)

We only need to expose setDecl, copyArray and the actOn* methods.

Added: 


Modified: 
clang/include/clang/AST/CommentSema.h

Removed: 




diff  --git a/clang/include/clang/AST/CommentSema.h 
b/clang/include/clang/AST/CommentSema.h
index b4e564e81185..015ce8f8652a 100644
--- a/clang/include/clang/AST/CommentSema.h
+++ b/clang/include/clang/AST/CommentSema.h
@@ -181,6 +181,7 @@ class Sema {
 
   FullComment *actOnFullComment(ArrayRef Blocks);
 
+private:
   void checkBlockCommandEmptyParagraph(BlockCommandComment *Command);
 
   void checkReturnsCommand(const BlockCommandComment *Command);



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

[clang] 4e7df1e - Comment AST: Find out if function is variadic in DeclInfo::fill

2021-11-12 Thread Aaron Puchert via cfe-commits


Author: Aaron Puchert
Date: 2021-11-12T21:10:56+01:00
New Revision: 4e7df1ef7b679953c1501177539166876c4cbda4

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

LOG: Comment AST: Find out if function is variadic in DeclInfo::fill

Then we don't have to look into the declaration again. Also it's only
natural to collect this information alongside parameters and return
type, as it's also just a parameter in some sense.

Reviewed By: gribozavr2

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

Added: 


Modified: 
clang/include/clang/AST/Comment.h
clang/lib/AST/Comment.cpp
clang/lib/AST/CommentSema.cpp
clang/test/Sema/warn-documentation.cpp

Removed: 




diff  --git a/clang/include/clang/AST/Comment.h 
b/clang/include/clang/AST/Comment.h
index 50ed7eec82080..fff0a985028a8 100644
--- a/clang/include/clang/AST/Comment.h
+++ b/clang/include/clang/AST/Comment.h
@@ -1077,6 +1077,9 @@ struct DeclInfo {
   /// Can be true only if \c IsFunctionDecl is true.
   unsigned IsClassMethod : 1;
 
+  /// Is \c CommentDecl something we consider a "function" that's variadic.
+  unsigned IsVariadic : 1;
+
   void fill();
 
   DeclKind getKind() const LLVM_READONLY {

diff  --git a/clang/lib/AST/Comment.cpp b/clang/lib/AST/Comment.cpp
index 5e6a7de5b5638..aac0591b057ec 100644
--- a/clang/lib/AST/Comment.cpp
+++ b/clang/lib/AST/Comment.cpp
@@ -210,6 +210,7 @@ void DeclInfo::fill() {
   IsObjCMethod = false;
   IsInstanceMethod = false;
   IsClassMethod = false;
+  IsVariadic = false;
   ParamVars = None;
   TemplateParameters = nullptr;
 
@@ -248,6 +249,7 @@ void DeclInfo::fill() {
   IsInstanceMethod = MD->isInstance();
   IsClassMethod = !IsInstanceMethod;
 }
+IsVariadic = FD->isVariadic();
 break;
   }
   case Decl::ObjCMethod: {
@@ -258,6 +260,7 @@ void DeclInfo::fill() {
 IsObjCMethod = true;
 IsInstanceMethod = MD->isInstanceMethod();
 IsClassMethod = !IsInstanceMethod;
+IsVariadic = MD->isVariadic();
 break;
   }
   case Decl::FunctionTemplate: {
@@ -268,6 +271,7 @@ void DeclInfo::fill() {
 ParamVars = FD->parameters();
 ReturnType = FD->getReturnType();
 TemplateParameters = FTD->getTemplateParameters();
+IsVariadic = FD->isVariadic();
 break;
   }
   case Decl::ClassTemplate: {
@@ -351,6 +355,8 @@ void DeclInfo::fill() {
   Kind = FunctionKind;
   ParamVars = FTL.getParams();
   ReturnType = FTL.getReturnLoc().getType();
+  if (const auto *FPT = dyn_cast(FTL.getTypePtr()))
+IsVariadic = FPT->isVariadic();
 }
   }
 

diff  --git a/clang/lib/AST/CommentSema.cpp b/clang/lib/AST/CommentSema.cpp
index 3977e6c20a88a..8251802482a0c 100644
--- a/clang/lib/AST/CommentSema.cpp
+++ b/clang/lib/AST/CommentSema.cpp
@@ -830,26 +830,11 @@ bool Sema::isAnyFunctionDecl() {
 }
 
 bool Sema::isFunctionOrMethodVariadic() {
-  if (!isFunctionDecl() || !ThisDeclInfo->CurrentDecl)
+  if (!ThisDeclInfo)
 return false;
-  if (const FunctionDecl *FD =
-dyn_cast(ThisDeclInfo->CurrentDecl))
-return FD->isVariadic();
-  if (const FunctionTemplateDecl *FTD =
-dyn_cast(ThisDeclInfo->CurrentDecl))
-return FTD->getTemplatedDecl()->isVariadic();
-  if (const ObjCMethodDecl *MD =
-dyn_cast(ThisDeclInfo->CurrentDecl))
-return MD->isVariadic();
-  if (const TypedefNameDecl *TD =
-  dyn_cast(ThisDeclInfo->CurrentDecl)) {
-QualType Type = TD->getUnderlyingType();
-if (Type->isFunctionPointerType() || Type->isBlockPointerType())
-  Type = Type->getPointeeType();
-if (const auto *FT = Type->getAs())
-  return FT->isVariadic();
-  }
-  return false;
+  if (!ThisDeclInfo->IsFilled)
+inspectThisDecl();
+  return ThisDeclInfo->IsVariadic;
 }
 
 bool Sema::isObjCMethodDecl() {

diff  --git a/clang/test/Sema/warn-documentation.cpp 
b/clang/test/Sema/warn-documentation.cpp
index 3a25c31f76f59..7243e791bba60 100644
--- a/clang/test/Sema/warn-documentation.cpp
+++ b/clang/test/Sema/warn-documentation.cpp
@@ -1448,6 +1448,17 @@ typedef void (*VariadicFnType)(int a, ...);
  */
 using VariadicFnType2 = void (*)(int a, ...);
 
+/*!
+ * Function pointer type variable.
+ *
+ * @param a
+ * works
+ *
+ * @param ...
+ * now should work too.
+ */
+void (*variadicFnVar)(int a, ...);
+
 // expected-warning@+2 {{empty paragraph passed to '@note' command}}
 /**
 @note



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

[PATCH] D113636: format_arg attribute does not support nullable instancetype return type

2021-11-12 Thread Akira Hatanaka via Phabricator via cfe-commits

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113636

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

[PATCH] D109557: Adds a BlockIndent option to AlignAfterOpenBracket

2021-11-12 Thread Björn Schäpers via Phabricator via cfe-commits

HazardyKnusperkeks added a comment.

In D109557#3066854 , @csmulhern wrote:

> In D109557#3063681 , 
> @MyDeveloperDay wrote:
>
>> Personally I'm not convinced there wouldn't be people who want this for 
>> function declarations and definitions
>>
>>   Function(
>>   param1,
>>   param2,
>>   param3
>>   );
>>
>> but don't want this
>>
>>   if (
>>  foo()
>>   )
>>   
>
> To be clear, the if styling above would only occur if `foo()` was long enough 
> to line wrap. Not all instances of `if`. However, I agree that could be true, 
> and the existing clang-format code clearly treats indentation inside if / for 
> / while differently than e.g. function calls. The existing 
> AlignAfterOpenBracket options for example do not apply to the former for 
> instance, which is why we see the weird indentation in the current revision 
> where the opening brace is not followed by a line break, but the closing 
> brace is preceded by one.

So we would still have the chance for a breaking if condition? There should be 
a test to document that! But I'm also not sure this should happen, but lets 
see, nobody is forced to chose `BlockIndent`.

Otherwise this is fine for me.




Comment at: clang/include/clang/Format/Format.h:100
+///
+/// Note: This currently only applies to parentheses.
+BAS_BlockIndent,

Maybe highlight this with the warning block?
Or correct it. ;)



Comment at: clang/lib/Format/ContinuationIndenter.cpp:951
 
+  if (PreviousNonComment && PreviousNonComment->is(tok::l_paren)) {
+State.Stack.back().BreakBeforeClosingParen =

Remove the braces



Comment at: clang/unittests/Format/FormatTest.cpp:22453
+  Style.AllowAllArgumentsOnNextLine = false;
+  Style.AllowAllConstructorInitializersOnNextLine = false;
+  Style.AllowAllParametersOfDeclarationOnNextLine = false;

How does it behave without these?



Comment at: clang/unittests/Format/FormatTest.cpp:22560
+
+  verifyFormat("if (quitelongarg !=\n"
+   "(alsolongarg - 1)) { // ABC is a very long "

I think you should add a test without the comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109557

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

[clang] 46a68c8 - Sema: const-qualify ParsedAttr::iterator::operator*()

2021-11-12 Thread Duncan P. N. Exon Smith via cfe-commits


Author: Duncan P. N. Exon Smith
Date: 2021-11-12T11:47:16-08:00
New Revision: 46a68c85bfc99f0bc651e5096a641d5d4051e99d

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

LOG: Sema: const-qualify ParsedAttr::iterator::operator*()

`const`-qualify ParsedAttr::iterator::operator*(), clearing up confusion
about the two meanings of const for pointers/iterators. Helps unblock
removal of (non-const) iterator_facade_base::operator->().

Added: 


Modified: 
clang/include/clang/Sema/ParsedAttr.h

Removed: 




diff  --git a/clang/include/clang/Sema/ParsedAttr.h 
b/clang/include/clang/Sema/ParsedAttr.h
index 52b2c0d963fce..ff2303c84bd21 100644
--- a/clang/include/clang/Sema/ParsedAttr.h
+++ b/clang/include/clang/Sema/ParsedAttr.h
@@ -894,7 +894,7 @@ class ParsedAttributesView {
 ParsedAttr> {
 iterator() : iterator_adaptor_base(nullptr) {}
 iterator(VecTy::iterator I) : iterator_adaptor_base(I) {}
-reference operator*() { return **I; }
+reference operator*() const { return **I; }
 friend class ParsedAttributesView;
   };
   struct const_iterator



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

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

2021-11-12 Thread Louis Dionne via Phabricator via cfe-commits

ldionne requested changes to this revision.
ldionne added a comment.

Regarding the back-deployment target for Apple, I believe the first OSes where 
`libc++.dylib` had sized deallocation are `macOS 10.12`, `iOS 10.0`, `watchOS 
3.0`, and `tvOS 10.0`. We should be able to enable reliance on sized 
deallocation functions automatically whenever the deployment target is greater 
than or equal to these versions.

The comments I left in the test file apply to both test files.




Comment at: 
libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/sized_delete_array14.pass.cpp:12-13
 // UNSUPPORTED: sanitizer-new-delete, c++03, c++11
+// XFAIL: clang-12, clang-13
+// XFAIL: apple-clang-13
 

The comment you removed is still relevant and should be kept for these 
compilers, since it applies to them.



Comment at: 
libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/sized_delete_array14.pass.cpp:15-16
 
-// NOTE: Clang does not enable sized-deallocation in C++14 and beyond by
-// default. It is only enabled when -fsized-deallocation is given.
-// XFAIL: clang, apple-clang
+// REQUIRES: -fsized-deallocation
+// ADDITIONAL_COMPILE_FLAGS: -fsized-deallocation
+

This shouldn't be necessary anymore if it is enabled by default in Clang trunk, 
no? And in fact we definitely don't want to add these flags here, since we're 
trying to test the default Clang behavior. I think that's why CI is failing 
with a bunch of XPASSes.



Comment at: 
libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/sized_delete_array14.pass.cpp:17
+// ADDITIONAL_COMPILE_FLAGS: -fsized-deallocation
+
+#if !defined(__cpp_sized_deallocation)

We will also need to add the following annotation to express that the test is 
expected to fail when run on targets where sized deallocation wasn't available 
yet:

```
// Sized deallocation was added in macOS 10.12 and aligned OSes.
// XFAIL: use_system_cxx_lib && target={{.+}}-apple-macosx10.{{9|10|11}}
```



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112921

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

[PATCH] D113148: Add new clang-tidy check for string_view(nullptr)

2021-11-12 Thread CJ Johnson via Phabricator via cfe-commits

CJ-Johnson updated this revision to Diff 386904.
CJ-Johnson marked 3 inline comments as done.
CJ-Johnson added a comment.

Address review comments from Yitzie


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113148

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/StringviewNullptrCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/StringviewNullptrCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone-stringview-nullptr.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/bugprone-stringview-nullptr.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-stringview-nullptr.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-stringview-nullptr.cpp
@@ -0,0 +1,1417 @@
+// RUN: %check_clang_tidy %s bugprone-stringview-nullptr -std=c++17 %t
+
+namespace std {
+
+using nullptr_t = decltype(nullptr);
+
+template 
+T &();
+
+template 
+struct type_identity { using type = T; };
+template 
+using type_identity_t = typename type_identity::type;
+
+template 
+class basic_string_view {
+public:
+  basic_string_view();
+  basic_string_view(const C *);
+  basic_string_view(const basic_string_view &);
+  basic_string_view =(const basic_string_view &);
+};
+
+template 
+bool operator<(basic_string_view, basic_string_view);
+template 
+bool operator<(type_identity_t>, basic_string_view);
+template 
+bool operator<(basic_string_view, type_identity_t>);
+
+template 
+bool operator<=(basic_string_view, basic_string_view);
+template 
+bool operator<=(type_identity_t>, basic_string_view);
+template 
+bool operator<=(basic_string_view, type_identity_t>);
+
+template 
+bool operator>(basic_string_view, basic_string_view);
+template 
+bool operator>(type_identity_t>, basic_string_view);
+template 
+bool operator>(basic_string_view, type_identity_t>);
+
+template 
+bool operator>=(basic_string_view, basic_string_view);
+template 
+bool operator>=(type_identity_t>, basic_string_view);
+template 
+bool operator>=(basic_string_view, type_identity_t>);
+
+template 
+bool operator==(basic_string_view, basic_string_view);
+template 
+bool operator==(type_identity_t>, basic_string_view);
+template 
+bool operator==(basic_string_view, type_identity_t>);
+
+template 
+bool operator!=(basic_string_view, basic_string_view);
+template 
+bool operator!=(type_identity_t>, basic_string_view);
+template 
+bool operator!=(basic_string_view, type_identity_t>);
+
+using string_view = basic_string_view;
+
+} // namespace std
+
+void function(std::string_view);
+void function(std::string_view, std::string_view);
+
+void temporary_construction() /* a */ {
+  // Functional Cast
+  {
+(void)(std::string_view(nullptr)) /* a1 */;
+// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: constructing basic_string_view from null is undefined; replace with the default constructor
+// CHECK-FIXES: {{^}}(void)(std::string_view()) /* a1 */;
+
+(void)(std::string_view((nullptr))) /* a2 */;
+// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: constructing{{.*}}default
+// CHECK-FIXES: {{^}}(void)(std::string_view()) /* a2 */;
+
+(void)(std::string_view({nullptr})) /* a3 */;
+// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: constructing{{.*}}default
+// CHECK-FIXES: {{^}}(void)(std::string_view()) /* a3 */;
+
+(void)(std::string_view({(nullptr)})) /* a4 */;
+// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: constructing{{.*}}default
+// CHECK-FIXES: {{^}}(void)(std::string_view()) /* a4 */;
+
+(void)(std::string_view({})) /* a5 */; // Default `const CharT*`
+// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: constructing{{.*}}default
+// CHECK-FIXES: {{^}}(void)(std::string_view()) /* a5 */;
+
+// (void)(const std::string_view(nullptr)) /* a6 */;
+// CV qualifiers do not compile in this context
+
+// (void)(const std::string_view((nullptr))) /* a7 */;
+// CV qualifiers do not compile in this context
+
+// (void)(const std::string_view({nullptr})) /* a8 */;
+// CV qualifiers do not compile in this context
+
+// (void)(const std::string_view({(nullptr)})) /* a9 */;
+// CV qualifiers do not compile in this context
+
+// (void)(const std::string_view({})) /* a10 */; // Default `const CharT*`
+// CV qualifiers do not compile in this context
+  }
+
+  // Temporary Object
+  {
+(void)(std::string_view{nullptr}) /* a11 */;
+// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: constructing{{.*}}default
+// CHECK-FIXES: {{^}}(void)(std::string_view{}) /* a11 */;
+
+(void)(std::string_view{(nullptr)}) /* a12 */;
+// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: constructing{{.*}}default
+//

[PATCH] D110216: [clang] retain type sugar in auto / template argument deduction

2021-11-12 Thread Louis Dionne via Phabricator via cfe-commits

ldionne added a comment.

I think this should do it:

  diff --git 
a/libcxx/test/std/utilities/any/any.nonmembers/any.cast/const_correctness.fail.cpp
 
b/libcxx/test/std/utilities/any/any.nonmembers/any.cast/const_correctness.verify.cpp
  similarity index 81%
  rename from 
libcxx/test/std/utilities/any/any.nonmembers/any.cast/const_correctness.fail.cpp
  rename to 
libcxx/test/std/utilities/any/any.nonmembers/any.cast/const_correctness.verify.cpp
  index 234efc83423b..a28e3fc9d1d7 100644
  --- 
a/libcxx/test/std/utilities/any/any.nonmembers/any.cast/const_correctness.fail.cpp
  +++ 
b/libcxx/test/std/utilities/any/any.nonmembers/any.cast/const_correctness.verify.cpp
  @@ -18,6 +18,13 @@
   
   // Try and cast away const.
   
  +// This test only checks that we static_assert in any_cast when the
  +// constraints are not respected, however Clang will sometimes emit
  +// additional errors while trying to instantiate the rest of any_cast
  +// following the static_assert. We ignore unexpected errors in
  +// clang-verify to make the test more robust to changes in Clang.
  +// ADDITIONAL_COMPILE_FLAGS: -Xclang -verify-ignore-unexpected=error
  +
   #include 
   
   struct TestType {};
  @@ -30,19 +37,15 @@ int main(int, char**)
   
   any a;
   
  -// expected-error@any:* {{drops 'const' qualifier}}
   // expected-error-re@any:* {{static_assert failed{{.*}} "ValueType is 
required to be a const lvalue reference or a CopyConstructible type"}}
   any_cast(static_cast(a)); // expected-note 
{{requested here}}
   
  -// expected-error@any:* {{cannot cast from lvalue of type 'const 
TestType' to rvalue reference type 'TestType &&'; types are not compatible}}
   // expected-error-re@any:* {{static_assert failed{{.*}} "ValueType is 
required to be a const lvalue reference or a CopyConstructible type"}}
   any_cast(static_cast(a)); // expected-note 
{{requested here}}
   
  -// expected-error@any:* {{drops 'const' qualifier}}
   // expected-error-re@any:* {{static_assert failed{{.*}} "ValueType is 
required to be a const lvalue reference or a CopyConstructible type"}}
   any_cast(static_cast(a)); // expected-note 
{{requested here}}
   
  -// expected-error@any:* {{cannot cast from lvalue of type 'const 
TestType2' to rvalue reference type 'TestType2 &&'; types are not compatible}}
   // expected-error-re@any:* {{static_assert failed{{.*}} "ValueType is 
required to be a const lvalue reference or a CopyConstructible type"}}
   any_cast(static_cast(a)); // expected-note 
{{requested here}}
   
  diff --git 
a/libcxx/test/std/utilities/any/any.nonmembers/any.cast/not_copy_constructible.fail.cpp
 
b/libcxx/test/std/utilities/any/any.nonmembers/any.cast/not_copy_constructible.verify.cpp
  similarity index 81%
  rename from 
libcxx/test/std/utilities/any/any.nonmembers/any.cast/not_copy_constructible.fail.cpp
  rename to 
libcxx/test/std/utilities/any/any.nonmembers/any.cast/not_copy_constructible.verify.cpp
  index 44a67f7aa03d..ec40c11b 100644
  --- 
a/libcxx/test/std/utilities/any/any.nonmembers/any.cast/not_copy_constructible.fail.cpp
  +++ 
b/libcxx/test/std/utilities/any/any.nonmembers/any.cast/not_copy_constructible.verify.cpp
  @@ -24,6 +24,13 @@
   
   // Test instantiating the any_cast with a non-copyable type.
   
  +// This test only checks that we static_assert in any_cast when the
  +// constraints are not respected, however Clang will sometimes emit
  +// additional errors while trying to instantiate the rest of any_cast
  +// following the static_assert. We ignore unexpected errors in
  +// clang-verify to make the test more robust to changes in Clang.
  +// ADDITIONAL_COMPILE_FLAGS: -Xclang -verify-ignore-unexpected=error
  +
   #include 
   
   using std::any;
  @@ -45,17 +52,14 @@ struct no_move {
   int main(int, char**) {
   any a;
   // expected-error-re@any:* {{static_assert failed{{.*}} "ValueType is 
required to be an lvalue reference or a CopyConstructible type"}}
  -// expected-error@any:* {{static_cast from 'no_copy' to 'no_copy' uses 
deleted function}}
   any_cast(static_cast(a)); // expected-note {{requested 
here}}
   
   // expected-error-re@any:* {{static_assert failed{{.*}} "ValueType is 
required to be a const lvalue reference or a CopyConstructible type"}}
  -// expected-error@any:* {{static_cast from 'const no_copy' to 'no_copy' 
uses deleted function}}
   any_cast(static_cast(a)); // expected-note 
{{requested here}}
   
   any_cast(static_cast(a)); // OK
   
   // expected-error-re@any:* {{static_assert failed{{.*}} "ValueType is 
required to be an rvalue reference or a CopyConstructible type"}}
  -// expected-error@any:* {{static_cast from 'typename 
remove_reference::type' (aka 'no_move') to 'no_move' uses deleted 
function}}
   any_cast(static_cast(a));
   
 return 0;

Use `git apply` -- in particular mind the renaming of the files.


Repository:
  rG LLVM Github Monorepo

[PATCH] D113148: Add new clang-tidy check for string_view(nullptr)

2021-11-12 Thread CJ Johnson via Phabricator via cfe-commits

CJ-Johnson marked 5 inline comments as done.
CJ-Johnson added inline comments.



Comment at: clang-tools-extra/clang-tidy/bugprone/StringviewNullptrCheck.cpp:75
+  compoundLiteralExpr(has(StringViewConstructingFromNullExpr)),
+  changeTo(node("null_argument_expr"), cat("")), construction_warning);
+

ymandel wrote:
> And more below...
Updated all 9 of them. Thanks!



Comment at: clang-tools-extra/clang-tidy/bugprone/StringviewNullptrCheck.cpp:90
+  declStmt(
+  has(varDecl(has(StringViewConstructingFromNullExpr),
+  unless(has(cxxConstructExpr(isListInitialization()

ymandel wrote:
> Here and throughout the matchers: in general, `has` is a fallback when you 
> don't have a more specific matcher. In this case, It think you mean to be 
> testing the initialization expression, in which case you should query that 
> directly: `hasInitializer`.  I'd recommend you revisit the uses of `has` and 
> check in each case if there's a more specific query. That's both cheaper 
> (doesn't need to iterate through all children) and more readable (because it 
> expresses the intent).
Thanks for the suggestion! I went back through all of the has calls and 
replaced the ones I could with something more specific.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/bugprone-stringview-nullptr.cpp:134-147
+// (void)(const std::string_view{nullptr}) /* a16 */;
+// CV qualifiers do not compile in this context
+
+// (void)(const std::string_view{(nullptr)}) /* a17 */;
+// CV qualifiers do not compile in this context
+
+// (void)(const std::string_view{{nullptr}}) /* a18 */;

ymandel wrote:
> what are these lines doing?
These lines are not "tests" themselves but they clearly document that all of 
the various permutations have been considered. If someone reads the test file 
and thinks "what about this other case?", these demonstrate that such other 
cases have been considered but are not valid :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113148

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

[PATCH] D110216: [clang] retain type sugar in auto / template argument deduction

2021-11-12 Thread Louis Dionne via Phabricator via cfe-commits

ldionne added a comment.

In D110216#3127530 , @mizvekov wrote:

> Exactly what Aaron said :-)
>
> Though might be a complication here if libcxx tests this with both ToT and 
> stable clang, so could be perhaps this needs to be changed in a way that 
> works in both.
>
> I will check this much later today, having a busy day.
>
> Adding Louis for the libcxx expertise.

Thanks for the heads up, I'm thinking about a fix right now. I'll paste the 
diff here and you can incorporate it to this review, which should trigger 
libc++ CI and make sure everything works.

We should probably start running the libc++ CI on Clang changes systematically, 
but I'm somewhat concerned about the bandwidth of our CI.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110216

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

[PATCH] D113738: [LTO] Allow passing -Os/-Oz as the optimization level

2021-11-12 Thread Peter Collingbourne via Phabricator via cfe-commits

pcc added a comment.

Please see D63976  where we rejected a similar 
change in favor of just letting this be controllable at compile time.

To the extent that the pass pipeline is affected by the size optimization 
level, I think we should change the passes so that they respect the 
`optsize`/`minsize` attributes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113738

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

[PATCH] D112024: [clang] diagnose_as attribute for Fortify diagnosing like builtins.

2021-11-12 Thread Michael Benfield via Phabricator via cfe-commits

mbenfield added inline comments.



Comment at: clang/test/Sema/attr-diagnose-as-builtin.c:62
+
+#ifdef __cplusplus
+template 

george.burgess.iv wrote:
> nit: can we also add a non-templated overload check in here?
> 
> if the diag isn't beautiful, that's fine IMO. just having a test-case to show 
> the expected behavior would be nice
Sorry, not sure what is being requested. By a "non-templated overload check" do 
you mean something different from `memcpy2` above?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112024

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

[PATCH] D113738: [LTO] Allow passing -Os/-Oz as the optimization level

2021-11-12 Thread Arthur Eubanks via Phabricator via cfe-commits

aeubanks updated this revision to Diff 386882.
aeubanks added a comment.

add more tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113738

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  lld/COFF/Config.h
  lld/COFF/Driver.cpp
  lld/Common/Args.cpp
  lld/Common/CMakeLists.txt
  lld/ELF/Config.h
  lld/ELF/Driver.cpp
  lld/ELF/LTO.cpp
  lld/MachO/Config.h
  lld/MachO/Driver.cpp
  lld/include/lld/Common/Args.h
  lld/test/COFF/lto-opt-level.ll
  lld/test/ELF/lto/opt-level.ll
  lld/test/MachO/lto-opt-level.ll
  lld/test/wasm/lto/opt-level.ll
  lld/wasm/Config.h
  lld/wasm/Driver.cpp
  llvm/include/llvm/LTO/Config.h
  llvm/include/llvm/LTO/legacy/LTOCodeGenerator.h
  llvm/include/llvm/LTO/legacy/ThinLTOCodeGenerator.h
  llvm/include/llvm/Passes/OptimizationLevel.h
  llvm/lib/LTO/LTO.cpp
  llvm/lib/LTO/LTOBackend.cpp
  llvm/lib/LTO/LTOCodeGenerator.cpp
  llvm/lib/LTO/ThinLTOCodeGenerator.cpp
  llvm/test/tools/llvm-lto2/X86/opt-levels.ll
  llvm/test/tools/llvm-lto2/X86/slp-vectorize-pm.ll
  llvm/test/tools/lto/opt-level.ll
  llvm/tools/llvm-lto/llvm-lto.cpp
  llvm/tools/llvm-lto2/llvm-lto2.cpp
  llvm/tools/lto/lto.cpp
  llvm/utils/gn/secondary/lld/Common/BUILD.gn

Index: llvm/utils/gn/secondary/lld/Common/BUILD.gn
===
--- llvm/utils/gn/secondary/lld/Common/BUILD.gn
+++ llvm/utils/gn/secondary/lld/Common/BUILD.gn
@@ -27,6 +27,7 @@
 "//llvm/lib/IR",
 "//llvm/lib/MC",
 "//llvm/lib/Option",
+"//llvm/lib/Passes",
 "//llvm/lib/Support",
 "//llvm/lib/Target",
   ]
Index: llvm/tools/lto/lto.cpp
===
--- llvm/tools/lto/lto.cpp
+++ llvm/tools/lto/lto.cpp
@@ -23,6 +23,7 @@
 #include "llvm/LTO/legacy/LTOCodeGenerator.h"
 #include "llvm/LTO/legacy/LTOModule.h"
 #include "llvm/LTO/legacy/ThinLTOCodeGenerator.h"
+#include "llvm/Passes/OptimizationLevel.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/TargetSelect.h"
@@ -34,12 +35,10 @@
 
 // extra command-line flags needed for LTOCodeGenerator
 static cl::opt
-OptLevel("O",
- cl::desc("Optimization level. [-O0, -O1, -O2, or -O3] "
-  "(default = '-O2')"),
- cl::Prefix,
- cl::ZeroOrMore,
- cl::init('2'));
+OptLevel("O",
+ cl::desc("Optimization level. [-O0, -O1, -O2, -O3, -Os, or -Oz] "
+  "(default = '-O2')"),
+ cl::Prefix, cl::ZeroOrMore, cl::init('2'));
 
 static cl::opt EnableFreestanding(
 "lto-freestanding", cl::init(false),
@@ -152,9 +151,10 @@
   LTOCodeGenerator *CG = unwrap(cg);
   CG->setAttrs(codegen::getMAttrs());
 
-  if (OptLevel < '0' || OptLevel > '3')
-report_fatal_error("Optimization level must be between 0 and 3");
-  CG->setOptLevel(OptLevel - '0');
+  Optional OL = OptimizationLevel::forChar(OptLevel);
+  if (!OL)
+report_fatal_error("invalid optimization level: -O" + Twine(OptLevel));
+  CG->setOptLevel(*OL);
   CG->setFreestanding(EnableFreestanding);
   CG->setDisableVerify(DisableVerify);
 }
@@ -525,20 +525,21 @@
   CodeGen->setFreestanding(EnableFreestanding);
 
   if (OptLevel.getNumOccurrences()) {
-if (OptLevel < '0' || OptLevel > '3')
-  report_fatal_error("Optimization level must be between 0 and 3");
-CodeGen->setOptLevel(OptLevel - '0');
-switch (OptLevel) {
-case '0':
+Optional OL = OptimizationLevel::forChar(OptLevel);
+if (!OL)
+  report_fatal_error("Invalid optimization level: " + Twine(OptLevel));
+CodeGen->setOptLevel(*OL);
+switch (OL->getSpeedupLevel()) {
+case 0:
   CodeGen->setCodeGenOptLevel(CodeGenOpt::None);
   break;
-case '1':
+case 1:
   CodeGen->setCodeGenOptLevel(CodeGenOpt::Less);
   break;
-case '2':
+case 2:
   CodeGen->setCodeGenOptLevel(CodeGenOpt::Default);
   break;
-case '3':
+case 3:
   CodeGen->setCodeGenOptLevel(CodeGenOpt::Aggressive);
   break;
 }
Index: llvm/tools/llvm-lto2/llvm-lto2.cpp
===
--- llvm/tools/llvm-lto2/llvm-lto2.cpp
+++ llvm/tools/llvm-lto2/llvm-lto2.cpp
@@ -20,6 +20,7 @@
 #include "llvm/Config/llvm-config.h"
 #include "llvm/IR/DiagnosticPrinter.h"
 #include "llvm/LTO/LTO.h"
+#include "llvm/Passes/OptimizationLevel.h"
 #include "llvm/Passes/PassPlugin.h"
 #include "llvm/Remarks/HotnessThresholdParser.h"
 #include "llvm/Support/Caching.h"
@@ -36,8 +37,9 @@
 static codegen::RegisterCodeGenFlags CGF;
 
 static cl::opt
-OptLevel("O", cl::desc("Optimization level. [-O0, -O1, -O2, or -O3] "
-   "(default = '-O2')"),
+OptLevel("O",
+ cl::desc("Optimization level. [-O0, -O1, -O2, -O3, -Os, or -Oz] "
+  "(default = '-O2')"),
  cl::Prefix, cl::ZeroOrMore, cl::init('2'));

[PATCH] D112024: [clang] diagnose_as attribute for Fortify diagnosing like builtins.

2021-11-12 Thread George Burgess IV via Phabricator via cfe-commits

george.burgess.iv accepted this revision.
george.burgess.iv added a comment.
This revision is now accepted and ready to land.

LGTM % 2 nits. Please feel free to commit after it LGT @aaron.ballman too. :)

Thanks again!




Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:2961
+  "%0 attribute references function %1, which %plural{0:takes no 
arguments|1:takes one argument|"
+  ":requires exactly %2 arguments}2">;
+def err_attribute_bounds_for_function : Error<

tiny nit: for consistency with the other options here



Comment at: clang/test/Sema/attr-diagnose-as-builtin.c:62
+
+#ifdef __cplusplus
+template 

nit: can we also add a non-templated overload check in here?

if the diag isn't beautiful, that's fine IMO. just having a test-case to show 
the expected behavior would be nice


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112024

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

[PATCH] D113707: [clang] Make -masm=intel affect inline asm style

2021-11-12 Thread Nico Weber via Phabricator via cfe-commits

thakis added a comment.

Thanks for pointing out built-in headers, that's a great point. I started 
adding .att_syntax lines where needed, but the I realized that that doens't 
work: it's too late for things taking memory, since %0 replacement is done with 
asm writer setting before getting to reading the snippet. So %0 is replaced 
with (say) `[eax]` before things get to the asm parser, and then `.att_syntax` 
switches it to att syntax, and then it says "can't do `[eax]` in att syntax".

The official solution for this according to "Multiple assembler dialects in asm 
templates" in gcc docs->Extensions->Inline Assembly->Extended Asm is to write 
every inline asm snippet twice: `bt{l %[Offset],%[Base] | %[Base],%[Offset]}`. 
clang doesn't yet support {|}. So I think I have to update this patch to make 
clang support {|} and use that in the intrinsic headers where needed.


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

https://reviews.llvm.org/D113707

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

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

2021-11-12 Thread John McCall via Phabricator via cfe-commits

rjmccall added a comment.

In D112921#3127767 , @rnk wrote:

> Let's not bring back the weak function thunk approach. That approach caused 
> problems described in my commit, D8467 , 
> which is why the code was removed.

The weak function that intercedes if the strong function isn't found statically 
was, yeah, a poorly thought out idea.  Weak *imports* work reliably, at least 
on Darwin, but they do require SDK support.

> The next steps are to sort out right defaults for Apple and understand the 
> libc++ test failures. Would it be reasonable to take a shortcut here, leave 
> the feature disabled for Apple targets, and defer those details to those that 
> own the target?

Yes, I think that would be acceptable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112921

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

[clang-tools-extra] 4fb62e1 - [clangd] Mark completions as plain-text when there's no snippet part

2021-11-12 Thread Sam McCall via cfe-commits


Author: Sam McCall
Date: 2021-11-12T18:44:20+01:00
New Revision: 4fb62e138398263dbefaa499c712929562bdf3bd

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

LOG: [clangd] Mark completions as plain-text when there's no snippet part

This helps nvim support the "repeat" action

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

Added: 


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

Removed: 




diff  --git a/clang-tools-extra/clangd/CodeComplete.cpp 
b/clang-tools-extra/clangd/CodeComplete.cpp
index 8c4b292d..f1f1cae690bc 100644
--- a/clang-tools-extra/clangd/CodeComplete.cpp
+++ b/clang-tools-extra/clangd/CodeComplete.cpp
@@ -2113,8 +2113,12 @@ CompletionItem CodeCompletion::render(const 
CodeCompleteOptions ) const {
   // FIXME(kadircet): Do not even fill insertText after making sure textEdit is
   // compatible with most of the editors.
   LSP.insertText = LSP.textEdit->newText;
-  LSP.insertTextFormat = Opts.EnableSnippets ? InsertTextFormat::Snippet
- : InsertTextFormat::PlainText;
+  // Some clients support snippets but work better with plaintext.
+  // So if the snippet is trivial, let the client know.
+  // https://github.com/clangd/clangd/issues/922
+  LSP.insertTextFormat = (Opts.EnableSnippets && !SnippetSuffix.empty())
+ ? InsertTextFormat::Snippet
+ : InsertTextFormat::PlainText;
   if (InsertInclude && InsertInclude->Insertion)
 LSP.additionalTextEdits.push_back(*InsertInclude->Insertion);
 

diff  --git a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp 
b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
index c63eecbe5091..aff6f6cf25ff 100644
--- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -1887,6 +1887,11 @@ TEST(CompletionTest, Render) {
   EXPECT_EQ(R.insertText, "Foo::x(${0:bool})");
   EXPECT_EQ(R.insertTextFormat, InsertTextFormat::Snippet);
 
+  C.SnippetSuffix = "";
+  R = C.render(Opts);
+  EXPECT_EQ(R.insertText, "Foo::x");
+  EXPECT_EQ(R.insertTextFormat, InsertTextFormat::PlainText);
+
   Include.Insertion.emplace();
   R = C.render(Opts);
   EXPECT_EQ(R.label, "^Foo::x(bool) const");



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

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

2021-11-12 Thread Reid Kleckner via Phabricator via cfe-commits

rnk added a comment.

Let's not bring back the weak function thunk approach. That approach caused 
problems described in my commit, D8467 , which 
is why the code was removed.

The next steps are to sort out right defaults for Apple and understand the 
libc++ test failures. Would it be reasonable to take a shortcut here, leave the 
feature disabled for Apple targets, and defer those details to those that own 
the target?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112921

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

[PATCH] D113765: [clangd] Fix function-arg-placeholder suppression with macros.

2021-11-12 Thread Sam McCall via Phabricator via cfe-commits

sammccall updated this revision to Diff 386873.
sammccall marked 2 inline comments as done.
sammccall added a comment.

Address review comments.
While here, condense some long comments that repeated the code and seemed 
confusing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113765

Files:
  clang-tools-extra/clangd/CodeComplete.cpp
  clang-tools-extra/clangd/tool/ClangdMain.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
@@ -1662,6 +1662,9 @@
   // Overload with bool
   int a(bool);
   int b(float);
+
+  X(int);
+  X(float);
 };
 int GFuncC(int);
 int GFuncD(int);
@@ -1671,6 +1674,10 @@
   EXPECT_THAT(completions(Context + "int y = X().^", {}, Opts).Completions,
   UnorderedElementsAre(Labeled("a(â¦)"), Labeled("b(float)")));
 
+  // Constructor completions are bundled.
+  EXPECT_THAT(completions(Context + "X z = X^", {}, Opts).Completions,
+  UnorderedElementsAre(Labeled("X"), Labeled("X(â¦)")));
+
   // Non-member completions are bundled, including index+sema.
   Symbol NoArgsGFunc = func("GFuncC");
   EXPECT_THAT(
@@ -2318,6 +2325,15 @@
 UnorderedElementsAre(AllOf(Named("foo_class"), SnippetSuffix("<$0>")),
  AllOf(Named("foo_alias"), SnippetSuffix("<$0>";
   }
+  {
+auto Results = completions(
+R"cpp(
+  #define FOO(x, y) x##f
+  FO^ )cpp",
+{}, Opts);
+EXPECT_THAT(Results.Completions, UnorderedElementsAre(AllOf(
+ Named("FOO"), SnippetSuffix("($0)";
+  }
 }
 
 TEST(CompletionTest, SuggestOverrides) {
@@ -3165,6 +3181,7 @@
   clangd::CodeCompleteOptions Opts;
   Opts.EnableSnippets = true;
   std::string Context = R"cpp(
+#define MACRO(x)
 int foo(int A);
 int bar();
 struct Object {
@@ -3212,6 +3229,9 @@
   Contains(AllOf(Labeled("Container(int Size)"),
  SnippetSuffix(""),
  Kind(CompletionItemKind::Constructor;
+  EXPECT_THAT(completions(Context + "MAC^(2)", {}, Opts).Completions,
+  Contains(AllOf(Labeled("MACRO(x)"), SnippetSuffix(""),
+ Kind(CompletionItemKind::Text;
 }
 
 TEST(CompletionTest, NoCrashDueToMacroOrdering) {
Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -233,7 +233,6 @@
  "function calls. When enabled, completions also contain "
  "placeholders for method parameters"),
 init(CodeCompleteOptions().EnableFunctionArgSnippets),
-Hidden,
 };
 
 opt HeaderInsertion{
Index: clang-tools-extra/clangd/CodeComplete.cpp
===
--- clang-tools-extra/clangd/CodeComplete.cpp
+++ clang-tools-extra/clangd/CodeComplete.cpp
@@ -466,32 +466,27 @@
   // FIXME(ibiryukov): sometimes add template arguments to a snippet, e.g.
   // we need to complete 'forward<$1>($0)'.
   return "($0)";
-// Suppress function argument snippets cursor is followed by left
-// parenthesis (and potentially arguments) or if there are potentially
-// template arguments. There are cases where it would be wrong (e.g. next
-// '<' token is a comparison rather than template argument list start) but
-// it is less common and suppressing snippet provides better UX.
-if (Completion.Kind == CompletionItemKind::Function ||
-Completion.Kind == CompletionItemKind::Method ||
-Completion.Kind == CompletionItemKind::Constructor) {
-  // If there is a potential template argument list, drop snippet and just
-  // complete symbol name. Ideally, this could generate an edit that would
-  // paste function arguments after template argument list but it would be
-  // complicated. Example:
-  //
-  // fu^ -> function
+
+bool MayHaveArgList = Completion.Kind == CompletionItemKind::Function ||
+  Completion.Kind == CompletionItemKind::Method ||
+  Completion.Kind == CompletionItemKind::Constructor ||
+  Completion.Kind == CompletionItemKind::Text /*Macro*/;
+// If likely arg list already exists, don't add new parens & placeholders.
+//   Snippet: function(int x, int y)
+//   func^(1,2) -> function(1, 2)
+// NOT function(int x, int y)(1, 2)
+if (MayHaveArgList) {
+  // Check for a template argument list in the code.
+  //   Snippet: function(int x)
+  //   fu^(1) ->

[PATCH] D113765: [clangd] Fix function-arg-placeholder suppression with macros.

2021-11-12 Thread Sam McCall via Phabricator via cfe-commits

sammccall added inline comments.



Comment at: clang-tools-extra/clangd/CodeComplete.cpp:517
+Completion.Kind == CompletionItemKind::Method ||
+Completion.Kind == CompletionItemKind::Text /*Macro*/) {
   // Functions snippets can be of 2 types:

kadircet wrote:
> again while here, maybe introduce constructors into this condition. (Even 
> better, what about a `bool isFunctionLikeCompletion(const CompletionItem&)` 
> that we can use both here and above?)
Done, though I didn't want to extract the variable too far away as there are 
some wrinkles (macros may not be function-like, template args...) that we can 
get away without resolving here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113765

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

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

2021-11-12 Thread John McCall via Phabricator via cfe-commits

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

Yeah, I think this either needs deployment restriction on Apple platforms or it 
needs the thunk / weak-linking approach from the original patch when the target 
OS isn't recent enough.  @ldionne should know the right OS versions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112921

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

[PATCH] D113779: [Clang] Add mfp16, mfp16fml and mdotprod flags for ARM target features.

2021-11-12 Thread Florian Hahn via Phabricator via cfe-commits

fhahn created this revision.
fhahn added reviewers: aemerson, ab, dmgreen, SjoerdMeijer, scanon.
Herald added subscribers: dang, kristof.beyls.
fhahn requested review of this revision.
Herald added a project: clang.

This patch adds support for `-m[no]fpf16`, ` `-m[no]fpf16fml` and
`-m[no]dotprod` feature flags for ARM.

The flags behave similar to specifying an -march flag with
+/-{fp16,fp16fml,dotprod}, but is more convenient for users that do not
want to provide a specific architecture version. There are similar flags
alread, like `-mcrc`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D113779

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Arch/AArch64.cpp
  clang/lib/Driver/ToolChains/Arch/ARM.cpp
  clang/test/Preprocessor/aarch64-target-features.c
  clang/test/Preprocessor/arm-target-features.c

Index: clang/test/Preprocessor/arm-target-features.c
===
--- clang/test/Preprocessor/arm-target-features.c
+++ clang/test/Preprocessor/arm-target-features.c
@@ -25,6 +25,7 @@
 // CHECK-V8A-ALLOW-FP-INSTR-V8A-NOT: #define __ARM_FEATURE_DOTPROD
 
 // RUN: %clang -target arm-none-linux-gnueabi -march=armv8.2-a+nofp16fml+fp16 -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-FULLFP16-VECTOR-SCALAR %s
+// RUN: %clang -target arm-none-linux-gnueabi -march=armv8.2-a -mnofp16fml -mfp16 -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-FULLFP16-VECTOR-SCALAR %s
 // RUN: %clang -target arm-none-linux-gnueabi -march=armv8.2-a+nofp16+fp16fml -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-FULLFP16-VECTOR-SCALAR %s
 // RUN: %clang -target arm-none-linux-gnueabi -march=armv8.2-a+fp16+nofp16fml -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-FULLFP16-VECTOR-SCALAR %s
 // RUN: %clang -target arm-none-linux-gnueabi -march=armv8-a+fp16fml -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-FULLFP16-VECTOR-SCALAR %s
@@ -77,6 +78,7 @@
 // CHECK-FULLFP16-SOFT-NOT: #define __ARM_FEATURE_FP16_VECTOR_ARITHMETIC
 
 // RUN: %clang -target arm-none-linux-gnueabi -march=armv8.2a+dotprod -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-DOTPROD %s
+// RUN: %clang -target arm-none-linux-gnueabi -march=armv8.2a -mdotprod -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-DOTPROD %s
 // CHECK-DOTPROD: #define __ARM_FEATURE_DOTPROD 1
 
 // RUN: %clang -target armv8r-none-linux-gnu -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-V8R %s
Index: clang/test/Preprocessor/aarch64-target-features.c
===
--- clang/test/Preprocessor/aarch64-target-features.c
+++ clang/test/Preprocessor/aarch64-target-features.c
@@ -199,20 +199,29 @@
 // CHECK-SVE2BITPERM: __ARM_FEATURE_SVE2_BITPERM 1
 
 // RUN: %clang -target aarch64-none-linux-gnu -march=armv8.2a+dotprod -x c -E -dM %s -o - | FileCheck --check-prefix=CHECK-DOTPROD %s
+// RUN: %clang -target aarch64-none-linux-gnu -march=armv8.2a -mdotprod -x c -E -dM %s -o - | FileCheck --check-prefix=CHECK-DOTPROD %s
 // CHECK-DOTPROD: __ARM_FEATURE_DOTPROD 1
 
 // On ARMv8.2-A and above, +fp16fml implies +fp16.
 // On ARMv8.4-A and above, +fp16 implies +fp16fml.
 // RUN: %clang -target aarch64-none-linux-gnueabi -march=armv8.2-a+nofp16fml+fp16 -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-FULLFP16-NOFML --check-prefix=CHECK-FULLFP16-VECTOR-SCALAR %s
+// RUN: %clang -target aarch64-none-linux-gnueabi -march=armv8.2-a -mnofp16fml -mfp16 -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-FULLFP16-NOFML --check-prefix=CHECK-FULLFP16-VECTOR-SCALAR %s
 // RUN: %clang -target aarch64-none-linux-gnueabi -march=armv8.2-a+nofp16+fp16fml -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-FULLFP16-FML --check-prefix=CHECK-FULLFP16-VECTOR-SCALAR %s
+// RUN: %clang -target aarch64-none-linux-gnueabi -march=armv8.2-a -mnofp16 -mfp16fml -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-FULLFP16-FML --check-prefix=CHECK-FULLFP16-VECTOR-SCALAR %s
 // RUN: %clang -target aarch64-none-linux-gnueabi -march=armv8.2-a+fp16+nofp16fml -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-FULLFP16-NOFML --check-prefix=CHECK-FULLFP16-VECTOR-SCALAR %s
+// RUN: %clang -target aarch64-none-linux-gnueabi -march=armv8.2-a -mfp16 -mnofp16fml -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-FULLFP16-NOFML --check-prefix=CHECK-FULLFP16-VECTOR-SCALAR %s
 // RUN: %clang -target aarch64-none-linux-gnueabi -march=armv8-a+fp16fml -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-FULLFP16-FML --check-prefix=CHECK-FULLFP16-VECTOR-SCALAR %s
+// RUN: %clang -target aarch64-none-linux-gnueabi -march=armv8-a -mfp16fml -x c -E -dM %s -o - | FileCheck -match-full-lines

[PATCH] D113148: Add new clang-tidy check for string_view(nullptr)

2021-11-12 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits

ymandel added a comment.

This looks really good. An impressive effort! Just a few changes.  Also, please 
ping Alex to get his opinion on the high level issue.




Comment at: clang-tools-extra/clang-tidy/bugprone/StringviewNullptrCheck.cpp:62
+  hasSourceExpression(StringViewConstructingFromNullExpr)),
+  changeTo(node("null_argument_expr"), cat("")), construction_warning);
+





Comment at: clang-tools-extra/clang-tidy/bugprone/StringviewNullptrCheck.cpp:67
+ unless(hasParent(compoundLiteralExpr(,
+  changeTo(node("null_argument_expr"), cat("")), construction_warning);
+





Comment at: clang-tools-extra/clang-tidy/bugprone/StringviewNullptrCheck.cpp:75
+  compoundLiteralExpr(has(StringViewConstructingFromNullExpr)),
+  changeTo(node("null_argument_expr"), cat("")), construction_warning);
+

And more below...



Comment at: clang-tools-extra/clang-tidy/bugprone/StringviewNullptrCheck.cpp:90
+  declStmt(
+  has(varDecl(has(StringViewConstructingFromNullExpr),
+  unless(has(cxxConstructExpr(isListInitialization()

Here and throughout the matchers: in general, `has` is a fallback when you 
don't have a more specific matcher. In this case, It think you mean to be 
testing the initialization expression, in which case you should query that 
directly: `hasInitializer`.  I'd recommend you revisit the uses of `has` and 
check in each case if there's a more specific query. That's both cheaper 
(doesn't need to iterate through all children) and more readable (because it 
expresses the intent).



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/bugprone-stringview-nullptr.cpp:134-147
+// (void)(const std::string_view{nullptr}) /* a16 */;
+// CV qualifiers do not compile in this context
+
+// (void)(const std::string_view{(nullptr)}) /* a17 */;
+// CV qualifiers do not compile in this context
+
+// (void)(const std::string_view{{nullptr}}) /* a18 */;

what are these lines doing?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113148

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

[PATCH] D113776: [Clang][SVE] Properly enable/disable dependant SVE target features based upon +(no)sve.* options

2021-11-12 Thread Thorsten via Phabricator via cfe-commits

tschuett added inline comments.



Comment at: clang/lib/Driver/ToolChains/Arch/AArch64.cpp:66
 
+typedef enum { Unset, True, False } OptState;
+bool isUnset(OptState State) { return State == OptState::Unset; }

This is almost an enum class.
```
enum class OptState { Unset, True, False };
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113776

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

[PATCH] D103511: [clang-tidy] Special member functions check should allow sole dtors by default

2021-11-12 Thread Balázs Benics via Phabricator via cfe-commits

steakhal abandoned this revision.
steakhal added a comment.
Herald added a subscriber: carlosgalvezp.

I forgot to abandon this earlier.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103511

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

[PATCH] D113690: Comment AST: Find out if function is variadic in DeclInfo::fill

2021-11-12 Thread Dmitri Gribenko via Phabricator via cfe-commits

gribozavr2 added a comment.

Thank you for the cleanup!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113690

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

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

2021-11-12 Thread Mark de Wever via Phabricator via cfe-commits

Mordante added a comment.

In D112921#3126492 , @zixuan-wu wrote:

> Is this going to be reviewed again or committed?

This patch still requires approval by the libc++ group.
The last build failed for multiple libc++ CI jobs, these should be fixed.
(I haven't looked at the code.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112921

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

[PATCH] D113575: Add `isInitCapture` and `forEachLambdaCapture` matchers.

2021-11-12 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits

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



Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:4235
+  for (const auto  : Node.captures()) {
+if (Finder->isTraversalIgnoringImplicitNodes() && Capture.isImplicit())
+  continue;

nice catch! i would have overlooked that detail.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113575

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

[PATCH] D113676: WIP: [clang][lex] Fix search path usage remark with modules

2021-11-12 Thread Jan Svoboda via Phabricator via cfe-commits

jansvoboda11 added inline comments.



Comment at: clang/lib/Lex/HeaderSearch.cpp:94
+  // Module map parsing initiated by header search.
+  if (HS.CurrentSearchPathIdx != ~0U)
+HS.ModuleToSearchDirIdx[M] = HS.CurrentSearchPathIdx;

ahoppen wrote:
> jansvoboda11 wrote:
> > ahoppen wrote:
> > > When would the `moduleMapModuleCreated` be called while 
> > > `CurrentSearchPathIdx == ~0U`? Could this be an `assert` instead?
> > This happens whenever any of the `ModuleMap` member functions that create 
> > new `Module` instances are called outside of `HeaderSearch`.
> > 
> > The `MMCallback` callback is basically "global" (present for the whole 
> > lifetime of `ModuleMap`), so that we don't have to repeatedly 
> > register/deregister it in `HeaderSearch::lookupModule`.
> Is there any reasonable case where module maps would be created without 
> `HeaderSearch` triggering the creation?
I think parsing of module maps (and therefore creation of contained modules) 
should be triggered through `HeaderSearch`.

However, there are also C++20 modules and explicit Clang modules that are not 
discovered by the header search mechanisms or modulemap parsing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113676

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

[PATCH] D51650: Implement target_clones multiversioning

2021-11-12 Thread Daniel Kiss via Phabricator via cfe-commits

danielkiss added inline comments.



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:3290-3296
+  // Ensure we don't combine these with themselves, since that causes some
+  // confusing behavior.
+  if (const auto *Other = D->getAttr()) {
+S.Diag(AL.getLoc(), diag::err_disallowed_duplicate_attribute) << AL;
+S.Diag(Other->getLocation(), diag::note_conflicting_attribute);
+return;
+  }

This caused the breakage. This check could be dropped for the reland.
target attributes seems can be combined, others shouldn't.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D51650

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

[PATCH] D113691: Comment AST: Recognize function-like objects via return type (NFC)

2021-11-12 Thread Dmitri Gribenko via Phabricator via cfe-commits

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

Nice, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113691

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

[PATCH] D113776: [Clang][SVE] Properly enable/disable dependant SVE target features based upon +(no)sve.* options

2021-11-12 Thread Bradley Smith via Phabricator via cfe-commits

bsmith created this revision.
bsmith added reviewers: paulwalker-arm, peterwaller-arm, sdesmalen.
Herald added subscribers: psnobl, tschuett.
Herald added a reviewer: efriedma.
bsmith requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D113776

Files:
  clang/lib/Driver/ToolChains/Arch/AArch64.cpp
  clang/test/Driver/aarch64-cpus.c
  clang/test/Driver/aarch64-implied-sve-features.c
  llvm/include/llvm/Support/AArch64TargetParser.def
  llvm/unittests/Support/TargetParserTest.cpp

Index: llvm/unittests/Support/TargetParserTest.cpp
===
--- llvm/unittests/Support/TargetParserTest.cpp
+++ llvm/unittests/Support/TargetParserTest.cpp
@@ -904,7 +904,8 @@
  AArch64::AEK_LSE | AArch64::AEK_RDM |
  AArch64::AEK_RCPC | AArch64::AEK_DOTPROD |
  AArch64::AEK_SVE2 | AArch64::AEK_BF16 |
- AArch64::AEK_I8MM | AArch64::AEK_SVE2BITPERM |
+ AArch64::AEK_I8MM | AArch64::AEK_SVE |
+ AArch64::AEK_SVE2 | AArch64::AEK_SVE2BITPERM |
  AArch64::AEK_PAUTH | AArch64::AEK_MTE |
  AArch64::AEK_SSBS | AArch64::AEK_FP16FML |
  AArch64::AEK_SB,
@@ -1012,12 +1013,12 @@
  AArch64::AEK_CRC | AArch64::AEK_FP |
  AArch64::AEK_SIMD | AArch64::AEK_RAS |
  AArch64::AEK_LSE | AArch64::AEK_RDM |
- AArch64::AEK_RCPC | AArch64::AEK_SVE2 |
- AArch64::AEK_DOTPROD | AArch64::AEK_MTE |
- AArch64::AEK_PAUTH | AArch64::AEK_I8MM |
- AArch64::AEK_BF16 | AArch64::AEK_SVE2BITPERM |
- AArch64::AEK_SSBS | AArch64::AEK_SB |
- AArch64::AEK_FP16FML,
+ AArch64::AEK_RCPC | AArch64::AEK_DOTPROD |
+ AArch64::AEK_MTE | AArch64::AEK_PAUTH |
+ AArch64::AEK_I8MM | AArch64::AEK_BF16 |
+ AArch64::AEK_SVE | AArch64::AEK_SVE2 |
+ AArch64::AEK_SVE2BITPERM | AArch64::AEK_SSBS |
+ AArch64::AEK_SB | AArch64::AEK_FP16FML,
  "9-A"),
 ARMCPUTestParams("cyclone", "armv8-a", "crypto-neon-fp-armv8",
  AArch64::AEK_NONE | AArch64::AEK_CRYPTO |
Index: llvm/include/llvm/Support/AArch64TargetParser.def
===
--- llvm/include/llvm/Support/AArch64TargetParser.def
+++ llvm/include/llvm/Support/AArch64TargetParser.def
@@ -145,9 +145,10 @@
 AARCH64_CPU_NAME("cortex-a55", ARMV8_2A, FK_CRYPTO_NEON_FP_ARMV8, false,
  (AArch64::AEK_FP16 | AArch64::AEK_DOTPROD | AArch64::AEK_RCPC))
 AARCH64_CPU_NAME("cortex-a510", ARMV9A, FK_NEON_FP_ARMV8, false,
- (AArch64::AEK_BF16 | AArch64::AEK_I8MM | AArch64::AEK_SVE2BITPERM |
+ (AArch64::AEK_BF16 | AArch64::AEK_I8MM | AArch64::AEK_SB |
   AArch64::AEK_PAUTH | AArch64::AEK_MTE | AArch64::AEK_SSBS |
-  AArch64::AEK_SB | AArch64::AEK_FP16FML))
+  AArch64::AEK_SVE | AArch64::AEK_SVE2 | AArch64::AEK_SVE2BITPERM |
+  AArch64::AEK_FP16FML))
 AARCH64_CPU_NAME("cortex-a57", ARMV8A, FK_CRYPTO_NEON_FP_ARMV8, false,
  (AArch64::AEK_CRC))
 AARCH64_CPU_NAME("cortex-a65", ARMV8_2A, FK_CRYPTO_NEON_FP_ARMV8, false,
@@ -184,8 +185,9 @@
   AArch64::AEK_SSBS))
 AARCH64_CPU_NAME("cortex-x2", ARMV9A, FK_NEON_FP_ARMV8, false,
  (AArch64::AEK_MTE | AArch64::AEK_BF16 | AArch64::AEK_I8MM |
-  AArch64::AEK_PAUTH | AArch64::AEK_SSBS | AArch64::AEK_SVE2BITPERM |
-  AArch64::AEK_SB | AArch64::AEK_FP16FML))
+  AArch64::AEK_PAUTH | AArch64::AEK_SSBS | AArch64::AEK_SB |
+  AArch64::AEK_SVE | AArch64::AEK_SVE2 | AArch64::AEK_SVE2BITPERM |
+  AArch64::AEK_FP16FML))
 AARCH64_CPU_NAME("neoverse-e1", ARMV8_2A, FK_CRYPTO_NEON_FP_ARMV8, false,
  (AArch64::AEK_DOTPROD | AArch64::AEK_FP16 | AArch64::AEK_RAS |
   AArch64::AEK_RCPC | AArch64::AEK_SSBS))
Index: clang/test/Driver/aarch64-implied-sve-features.c
===
--- /dev/null
+++ clang/test/Driver/aarch64-implied-sve-features.c
@@ -0,0 +1,65 @@
+// RUN: %clang -target aarch64-linux-gnu -march=armv8-a+sve %s -### |& FileCheck %s --check-prefix=SVE-ONLY
+// SVE-ONLY: "-target-feature" "+sve"
+
+// RUN: %clang -target aarch64-linux-gnu -march=armv8-a+nosve %s

[PATCH] D113676: WIP: [clang][lex] Fix search path usage remark with modules

2021-11-12 Thread Alex Hoppen via Phabricator via cfe-commits

ahoppen added inline comments.



Comment at: clang/lib/Lex/HeaderSearch.cpp:94
+  // Module map parsing initiated by header search.
+  if (HS.CurrentSearchPathIdx != ~0U)
+HS.ModuleToSearchDirIdx[M] = HS.CurrentSearchPathIdx;

jansvoboda11 wrote:
> ahoppen wrote:
> > When would the `moduleMapModuleCreated` be called while 
> > `CurrentSearchPathIdx == ~0U`? Could this be an `assert` instead?
> This happens whenever any of the `ModuleMap` member functions that create new 
> `Module` instances are called outside of `HeaderSearch`.
> 
> The `MMCallback` callback is basically "global" (present for the whole 
> lifetime of `ModuleMap`), so that we don't have to repeatedly 
> register/deregister it in `HeaderSearch::lookupModule`.
Is there any reasonable case where module maps would be created without 
`HeaderSearch` triggering the creation?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113676

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

[PATCH] D110216: [clang] retain type sugar in auto / template argument deduction

2021-11-12 Thread Matheus Izvekov via Phabricator via cfe-commits

mizvekov added a subscriber: ldionne.
mizvekov added a comment.

Exactly what Aaron said :-)

Though might be a complication here if libcxx tests this with both ToT and 
stable clang, so could be perhaps this needs to be changed in a way that works 
in both.

I will check this much later today, having a busy day.

Adding Louis for the libcxx expertise.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110216

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

[clang] 05f34ff - [clang] Inclusive language: change instances of blacklist/whitelist to allowlist/ignorelist

2021-11-12 Thread Zarko Todorovski via cfe-commits


Author: Zarko Todorovski
Date: 2021-11-12T15:46:16Z
New Revision: 05f34ffa216975f132fa1bd4cbf8424053a19147

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

LOG: [clang] Inclusive language: change instances of blacklist/whitelist to 
allowlist/ignorelist

Change the error message to use ignorelist, and changed some variable and 
function
names in related code and test.

Reviewed By: thakis

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

Added: 


Modified: 
clang/include/clang/AST/CommentHTMLTags.td
clang/include/clang/Basic/DiagnosticASTKinds.td
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Frontend/HeaderIncludeGen.cpp
clang/test/CodeGen/sanitize-address-field-padding.cpp

Removed: 




diff  --git a/clang/include/clang/AST/CommentHTMLTags.td 
b/clang/include/clang/AST/CommentHTMLTags.td
index 251490094940..a1ce8c6da96c 100644
--- a/clang/include/clang/AST/CommentHTMLTags.td
+++ b/clang/include/clang/AST/CommentHTMLTags.td
@@ -52,11 +52,11 @@ def Tr  : Tag<"tr"> { let EndTagOptional = 1; }
 def Th  : Tag<"th"> { let EndTagOptional = 1; }
 def Td  : Tag<"td"> { let EndTagOptional = 1; }
 
-// Define a blacklist of attributes that are not safe to pass through to HTML
+// Define a list of attributes that are not safe to pass through to HTML
 // output if the input is untrusted.
 //
-// FIXME: this should be a whitelist.  When changing this to a whitelist, don't
-// forget to change the default in the TableGen backend.
+// FIXME: This should be a list of attributes that _are_ safe.  When changing
+// this change, don't forget to change the default in the TableGen backend.
 class Attribute {
   string Spelling = spelling;
   bit IsSafeToPassThrough = 1;

diff  --git a/clang/include/clang/Basic/DiagnosticASTKinds.td 
b/clang/include/clang/Basic/DiagnosticASTKinds.td
index 496d86ee2fe7..d788c8517914 100644
--- a/clang/include/clang/Basic/DiagnosticASTKinds.td
+++ b/clang/include/clang/Basic/DiagnosticASTKinds.td
@@ -567,8 +567,8 @@ def remark_sanitize_address_insert_extra_padding_accepted : 
Remark<
 def remark_sanitize_address_insert_extra_padding_rejected : Remark<
 "-fsanitize-address-field-padding ignored for %0 because it "
 "%select{is not C++|is packed|is a union|is trivially copyable|"
-"has trivial destructor|is standard layout|is in a blacklisted file|"
-"is blacklisted}1">, ShowInSystemHeader,
+"has trivial destructor|is standard layout|is in a ignorelisted file|"
+"is ignorelisted}1">, ShowInSystemHeader,
 InGroup;
 
 def warn_npot_ms_struct : Warning<

diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index a3023f1c95e5..6e1891aa85de 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -4621,7 +4621,7 @@ void Clang::ConstructJob(Compilation , const JobAction 
,
 
 // reject options that shouldn't be supported in bitcode
 // also reject kernel/kext
-static const constexpr unsigned kBitcodeOptionBlacklist[] = {
+static const constexpr unsigned kBitcodeOptionIgnorelist[] = {
 options::OPT_mkernel,
 options::OPT_fapple_kext,
 options::OPT_ffunction_sections,
@@ -4665,7 +4665,7 @@ void Clang::ConstructJob(Compilation , const JobAction 
,
 options::OPT_mllvm,
 };
 for (const auto  : Args)
-  if (llvm::is_contained(kBitcodeOptionBlacklist, A->getOption().getID()))
+  if (llvm::is_contained(kBitcodeOptionIgnorelist, A->getOption().getID()))
 D.Diag(diag::err_drv_unsupported_embed_bitcode) << A->getSpelling();
 
 // Render the CodeGen options that need to be passed.

diff  --git a/clang/lib/Frontend/HeaderIncludeGen.cpp 
b/clang/lib/Frontend/HeaderIncludeGen.cpp
index 1ee47d8d2480..5db8792bf420 100644
--- a/clang/lib/Frontend/HeaderIncludeGen.cpp
+++ b/clang/lib/Frontend/HeaderIncludeGen.cpp
@@ -119,7 +119,7 @@ void clang::AttachHeaderIncludeGen(Preprocessor ,
   // Print header info for extra headers, pretending they were discovered by
   // the regular preprocessor. The primary use case is to support proper
   // generation of Make / Ninja file dependencies for implicit includes, such
-  // as sanitizer blacklists. It's only important for cl.exe compatibility,
+  // as sanitizer ignorelists. It's only important for cl.exe compatibility,
   // the GNU way to generate rules is -M / -MM / -MD / -MMD.
   for (const auto  : DepOpts.ExtraDeps)
 PrintHeaderInfo(OutputFile, Header.first, ShowDepth, 2, MSStyle);

diff  --git a/clang/test/CodeGen/sanitize-address-field-padding.cpp 
b/clang/test/CodeGen/sanitize-address-field-padding.cpp
index fbbabe96be9b..67316bd97412 100644
--- a/clang/test/CodeGen/sanitize-address-field-padding.cpp
+++

[PATCH] D113189: [clang] Inclusive language: change instances of blacklist/whitelist to allowlist/ignorelist

2021-11-12 Thread Zarko Todorovski via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG05f34ffa2169: [clang] Inclusive language: change instances 
of blacklist/whitelist to… (authored by ZarkoCA).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113189

Files:
  clang/include/clang/AST/CommentHTMLTags.td
  clang/include/clang/Basic/DiagnosticASTKinds.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/HeaderIncludeGen.cpp
  clang/test/CodeGen/sanitize-address-field-padding.cpp

Index: clang/test/CodeGen/sanitize-address-field-padding.cpp
===
--- clang/test/CodeGen/sanitize-address-field-padding.cpp
+++ clang/test/CodeGen/sanitize-address-field-padding.cpp
@@ -1,9 +1,9 @@
 // Test -fsanitize-address-field-padding
-// RUN: echo 'type:SomeNamespace::BlacklistedByName=field-padding' > %t.type.blacklist
-// RUN: echo 'src:*sanitize-address-field-padding.cpp=field-padding' > %t.file.blacklist
-// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fsanitize=address -fsanitize-address-field-padding=1 -fsanitize-blacklist=%t.type.blacklist -Rsanitize-address -emit-llvm -o - %s 2>&1 | FileCheck %s
-// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fsanitize=address -fsanitize-address-field-padding=1 -fsanitize-blacklist=%t.type.blacklist -Rsanitize-address -emit-llvm -o - %s -O1 -fno-experimental-new-pass-manager -mconstructor-aliases 2>&1 | FileCheck %s --check-prefix=WITH_CTOR_ALIASES
-// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fsanitize=address -fsanitize-address-field-padding=1 -fsanitize-blacklist=%t.file.blacklist -Rsanitize-address -emit-llvm -o - %s 2>&1 | FileCheck %s --check-prefix=FILE_BLACKLIST
+// RUN: echo 'type:SomeNamespace::IgnorelistedByName=field-padding' > %t.type.ignorelist
+// RUN: echo 'src:*sanitize-address-field-padding.cpp=field-padding' > %t.file.ignorelist
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fsanitize=address -fsanitize-address-field-padding=1 -fsanitize-ignorelist=%t.type.ignorelist -Rsanitize-address -emit-llvm -o - %s 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fsanitize=address -fsanitize-address-field-padding=1 -fsanitize-ignorelist=%t.type.ignorelist -Rsanitize-address -emit-llvm -o - %s -O1 -fno-experimental-new-pass-manager -mconstructor-aliases 2>&1 | FileCheck %s --check-prefix=WITH_CTOR_ALIASES
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fsanitize=address -fsanitize-address-field-padding=1 -fsanitize-ignorelist=%t.file.ignorelist -Rsanitize-address -emit-llvm -o - %s 2>&1 | FileCheck %s --check-prefix=FILE_IGNORELIST
 // RUN: %clang_cc1 -fsanitize=address -emit-llvm -o - %s 2>&1 | FileCheck %s --check-prefix=NO_PADDING
 // Try to emulate -save-temps option and make sure -disable-llvm-passes will not run sanitize instrumentation.
 // RUN: %clang_cc1 -fsanitize=address -emit-llvm -disable-llvm-passes -o - %s | %clang_cc1 -fsanitize=address -emit-llvm -o - -x ir | FileCheck %s --check-prefix=NO_PADDING
@@ -17,11 +17,11 @@
 // CHECK: -fsanitize-address-field-padding ignored for Negative3 because it is a union
 // CHECK: -fsanitize-address-field-padding ignored for Negative4 because it is trivially copyable
 // CHECK: -fsanitize-address-field-padding ignored for Negative5 because it is packed
-// CHECK: -fsanitize-address-field-padding ignored for SomeNamespace::BlacklistedByName because it is blacklisted
+// CHECK: -fsanitize-address-field-padding ignored for SomeNamespace::IgnorelistedByName because it is ignorelisted
 // CHECK: -fsanitize-address-field-padding ignored for ExternCStruct because it is not C++
 //
-// FILE_BLACKLIST: -fsanitize-address-field-padding ignored for Positive1 because it is in a blacklisted file
-// FILE_BLACKLIST-NOT: __asan_poison_intra_object_redzone
+// FILE_IGNORELIST: -fsanitize-address-field-padding ignored for Positive1 because it is in a ignorelisted file
+// FILE_IGNORELIST-NOT: __asan_poison_intra_object_redzone
 // NO_PADDING-NOT: __asan_poison_intra_object_redzone
 
 
@@ -141,10 +141,10 @@
 
 
 namespace SomeNamespace {
-class BlacklistedByName {
+class IgnorelistedByName {
  public:
-  BlacklistedByName() {}
-  ~BlacklistedByName() {}
+  IgnorelistedByName() {}
+  ~IgnorelistedByName() {}
   int make_it_non_standard_layout;
  private:
   char private1;
@@ -152,7 +152,7 @@
 };
 }  // SomeNamespace
 
-SomeNamespace::BlacklistedByName blacklisted_by_name;
+SomeNamespace::IgnorelistedByName ignorelisted_by_name;
 
 extern "C" {
 class ExternCStruct {
Index: clang/lib/Frontend/HeaderIncludeGen.cpp
===
--- clang/lib/Frontend/HeaderIncludeGen.cpp
+++ clang/lib/Frontend/HeaderIncludeGen.cpp
@@ -119,7 +119,7 @@
   // Print header info for extra headers, pretending they were discovered by
   // the regular preprocessor. The primary use case is to support proper
   //

[PATCH] D110927: [analyzer] Access stored value of a constant array through a pointer to another type

2021-11-12 Thread Balázs Benics via Phabricator via cfe-commits

steakhal added inline comments.



Comment at: clang/test/Analysis/initialization.cpp:295-299
+void glob_cast_opposite_sign1() {
+  auto *ptr = (unsigned int *)glob_arr2;
+  auto x1 = ptr[0]; // no-warning
+  auto x2 = ptr[1]; // expected-warning{{garbage or undefined}}
+}

ASDenysPetrov wrote:
> steakhal wrote:
> > ASDenysPetrov wrote:
> > > steakhal wrote:
> > > > ASDenysPetrov wrote:
> > > > > steakhal wrote:
> > > > > > I think it's not correct.
> > > > > > 
> > > > > > `glob_arr2` refers to an object of dynamic type `int[2]`.
> > > > > > And the pointer decayed from it is `int *`, which has //similar 
> > > > > > type// to `unsigned *` what you are using to access the memory.
> > > > > > Since they are //similar//, this operation should work for all the 
> > > > > > valid indices, thus `ptr[0]`, `ptr[1]`, `ptr[2]`, `ptr[3]` should 
> > > > > > all be valid.
> > > > > > 
> > > > > > 
> > > > > > glob_arr2 refers to an object of dynamic type int[2].
> > > > > `glob_arr2` has an extent of 4.
> > > > > > And the pointer decayed from it is int *, which has similar type to 
> > > > > > unsigned * what you are using to access the memory.
> > > > > Yes, they are the same and it perfectly suits to 
> > > > > http://eel.is/c++draft/basic.lval#11 . But still you can't access 
> > > > > other element then the first one according to 
> > > > > http://eel.is/c++draft/basic.compound#3 : //For purposes of pointer 
> > > > > arithmetic ([expr.add]) and comparison ([expr.rel], [expr.eq]), [...] 
> > > > > an object of type T that is not an array element is considered to 
> > > > > belong to an array with one element of type T. //
> > > > > `unsigned int` and `int` are different types according to 
> > > > > http://eel.is/c++draft/basic.fundamental#2 . The object of type 
> > > > > `unsigned int` is NOT an array, beacuse there is no array of type 
> > > > > `unsigned int`. Hence you can only only access the first and a single 
> > > > > element of type `unsigned int`.
> > > > > 
> > > > Yes, `glob_arr` has 4 elements, sorry for this typo.
> > > > 
> > > > ---
> > > > I disagree with the second part though. It seems like gcc disagrees as 
> > > > well: https://godbolt.org/z/5o7ozvPar
> > > > ```lang=C++
> > > > auto foo(unsigned ()[4], int ()[4]) {
> > > > p[0] = 2;
> > > > q[0] = 1;
> > > > return p[0]; // memory read! thus, p and q can alias according to 
> > > > g++.
> > > > }
> > > > ```
> > > > `gcc` still thinks that `p` and `q` can refer to the same memory region 
> > > > even if the `-fstrict-aliasing` flag is present.
> > > > In other circumstances, it would produce a `mov eax, 2` instead of a 
> > > > memory load if `p` and `q` cannot alias.
> > > >I disagree with the second part though. It seems like gcc disagrees as 
> > > >well: https://godbolt.org/z/5o7ozvPar
> > > I see how all the compilers handle this. I've switched several vendors on 
> > > Godbolt. They all have the similar behavior. You are right from compiler 
> > > perspective, but it's not quite the same in terms of C++ abstract machine.
> > > Your example is correct, it works OK and is consistent with the Standard:
> > > ```
> > > p[0] = 2;
> > > q[0] = 1;
> > > ```
> > > This one also works as expected but goes against the Standard:
> > > ```
> > > p[1] = 2;
> > > q[1] = 1;
> > > ```
> > > I want you watch this particular part about access via aliased pointers: 
> > > https://youtu.be/_qzMpk-22cc?t=2623 For me it seems correct, at least I 
> > > can't argue against of it now.
> > But in this example we have an array, thus pointer arithmetic should be 
> > fine according to the video.
> > Could you find the mentioned email discussion? I would be really curious.
> > BTW from the analyzer user's perspective, it would be 'better' to find 
> > strict aliasing violations which actually matter - now, and risking 
> > miscompilations.
> I'm not enough confident about that to debate now. As you can see we started 
> arguing as well. That means that the Standard really leaves the subject for 
> misinterpretation.
> OK, let's bring it to the form in which compilers operate. I mean, let's 
> legitimate indirections with a different offsets through aliased types: `auto 
> x = ((unsigened)arr)[42]; // OK`.
> I think it will be enough for this patch for now.
So, we agree that `glob_cast_opposite_sign1()` should have no warnings. At 
least for now. I would be okay with this direction.


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

https://reviews.llvm.org/D110927

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

[PATCH] D113775: [clang][modules] Umbrella with missing submodule: unify implicit & explicit

2021-11-12 Thread Jan Svoboda via Phabricator via cfe-commits

jansvoboda11 updated this revision to Diff 386847.
jansvoboda11 added a comment.

Re-run CI


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113775

Files:
  clang/include/clang/Frontend/CompilerInstance.h
  clang/lib/Frontend/CompilerInstance.cpp
  clang/test/Modules/missing-submodule.m

Index: clang/test/Modules/missing-submodule.m
===
--- clang/test/Modules/missing-submodule.m
+++ clang/test/Modules/missing-submodule.m
@@ -1,5 +1,6 @@
 // RUN: rm -rf %t
 // RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -F %S/Inputs %s -verify
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -F %S/Inputs %s -verify
 #include  // expected-warning{{missing submodule 'Module.NotInModule'}}
 
 int getNotInModule() {
Index: clang/lib/Frontend/CompilerInstance.cpp
===
--- clang/lib/Frontend/CompilerInstance.cpp
+++ clang/lib/Frontend/CompilerInstance.cpp
@@ -1772,9 +1772,24 @@
   return MS_ModuleNotFound;
 }
 
+static ModuleLoadResult handleMissingSubmodule(DiagnosticsEngine ,
+   Module *SubM,
+   SourceLocation ImportLoc,
+   ModuleIdPath Path) {
+  // We have an umbrella header or directory that doesn't actually include all
+  // the headers within the directory it covers. Complain about this missing
+  // submodule and recover by forgetting that we ever saw this submodule.
+  Diags.Report(ImportLoc, diag::warn_missing_submodule)
+  << SubM->getFullModuleName()
+  << SourceRange(Path.front().second, Path.back().second);
+
+  return ModuleLoadResult::MissingExpected;
+}
+
 ModuleLoadResult CompilerInstance::findOrCompileModuleAndReadAST(
 StringRef ModuleName, SourceLocation ImportLoc,
-SourceLocation ModuleNameLoc, bool IsInclusionDirective) {
+SourceLocation ModuleNameLoc, ModuleIdPath Path,
+Module::NameVisibilityKind Visibility, bool IsInclusionDirective) {
   // Search for a module with the given name.
   HeaderSearch  = PP->getHeaderSearchInfo();
   Module *M =
@@ -1797,6 +1812,16 @@
   return ModuleLoadResult::ConfigMismatch;
 }
 
+bool MapPrivateSubModToTopLevel = false;
+// FIXME: Should we detect this at module load time? It seems fairly
+// expensive (and rare).
+Module *SubM = getSubmoduleCorrespondingToPath(
+MapPrivateSubModToTopLevel, M, ImportLoc, Path, Visibility,
+IsInclusionDirective);
+
+if (SubM != M && !SubM->IsFromModuleFile && !MapPrivateSubModToTopLevel)
+  return handleMissingSubmodule(getDiagnostics(), SubM, ImportLoc, Path);
+
 getDiagnostics().Report(ModuleNameLoc, diag::err_module_build_disabled)
 << ModuleName;
 return nullptr;
@@ -2061,8 +2086,9 @@
 //}
 MM.cacheModuleLoad(*Path[0].first, Module);
   } else {
-ModuleLoadResult Result = findOrCompileModuleAndReadAST(
-ModuleName, ImportLoc, ModuleNameLoc, IsInclusionDirective);
+ModuleLoadResult Result =
+findOrCompileModuleAndReadAST(ModuleName, ImportLoc, ModuleNameLoc,
+  Path, Visibility, IsInclusionDirective);
 if (!Result.isNormal())
   return Result;
 if (!Result)
@@ -2077,6 +2103,9 @@
 return ModuleLoadResult();
 
   bool MapPrivateSubModToTopLevel = false;
+  class Module *TopLevelModule = Module;
+  // FIXME: Should we detect this at module load time? It seems fairly
+  // expensive (and rare).
   Module = getSubmoduleCorrespondingToPath(MapPrivateSubModToTopLevel, Module,
ImportLoc, Path, Visibility,
IsInclusionDirective);
@@ -2084,19 +2113,9 @@
   // Make the named module visible, if it's not already part of the module
   // we are parsing.
   if (ModuleName != getLangOpts().CurrentModule) {
-if (!Module->IsFromModuleFile && !MapPrivateSubModToTopLevel) {
-  // We have an umbrella header or directory that doesn't actually include
-  // all of the headers within the directory it covers. Complain about
-  // this missing submodule and recover by forgetting that we ever saw
-  // this submodule.
-  // FIXME: Should we detect this at module load time? It seems fairly
-  // expensive (and rare).
-  getDiagnostics().Report(ImportLoc, diag::warn_missing_submodule)
-<< Module->getFullModuleName()
-<< SourceRange(Path.front().second, Path.back().second);
-
-  return ModuleLoadResult::MissingExpected;
-}
+if (Module != TopLevelModule && !Module->IsFromModuleFile &&
+!MapPrivateSubModToTopLevel)
+  return handleMissingSubmodule(getDiagnostics(), Module, ImportLoc, Path);
 
 // Check whether this module is available.

[PATCH] D113775: [clang][modules] Umbrella with missing submodule: unify implicit & explicit

2021-11-12 Thread Jan Svoboda via Phabricator via cfe-commits

jansvoboda11 created this revision.
jansvoboda11 added reviewers: Bigcheese, dexonsmith, vsapsai.
jansvoboda11 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Submodules not covered by existing umbrella header are being imported as 
textual with implicit modules. Explicit modules error out, since they still 
expect to be provided a module file that covers such header.

Unify the behavior of explicit modules with implicit modules by allowing such 
cases and treating them as textual as well.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D113775

Files:
  clang/include/clang/Frontend/CompilerInstance.h
  clang/lib/Frontend/CompilerInstance.cpp
  clang/test/Modules/missing-submodule.m

Index: clang/test/Modules/missing-submodule.m
===
--- clang/test/Modules/missing-submodule.m
+++ clang/test/Modules/missing-submodule.m
@@ -1,5 +1,6 @@
 // RUN: rm -rf %t
 // RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -F %S/Inputs %s -verify
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -F %S/Inputs %s -verify
 #include  // expected-warning{{missing submodule 'Module.NotInModule'}}
 
 int getNotInModule() {
Index: clang/lib/Frontend/CompilerInstance.cpp
===
--- clang/lib/Frontend/CompilerInstance.cpp
+++ clang/lib/Frontend/CompilerInstance.cpp
@@ -1772,9 +1772,24 @@
   return MS_ModuleNotFound;
 }
 
+static ModuleLoadResult handleMissingSubmodule(DiagnosticsEngine ,
+   Module *SubM,
+   SourceLocation ImportLoc,
+   ModuleIdPath Path) {
+  // We have an umbrella header or directory that doesn't actually include all
+  // the headers within the directory it covers. Complain about this missing
+  // submodule and recover by forgetting that we ever saw this submodule.
+  Diags.Report(ImportLoc, diag::warn_missing_submodule)
+  << SubM->getFullModuleName()
+  << SourceRange(Path.front().second, Path.back().second);
+
+  return ModuleLoadResult::MissingExpected;
+}
+
 ModuleLoadResult CompilerInstance::findOrCompileModuleAndReadAST(
 StringRef ModuleName, SourceLocation ImportLoc,
-SourceLocation ModuleNameLoc, bool IsInclusionDirective) {
+SourceLocation ModuleNameLoc, ModuleIdPath Path,
+Module::NameVisibilityKind Visibility, bool IsInclusionDirective) {
   // Search for a module with the given name.
   HeaderSearch  = PP->getHeaderSearchInfo();
   Module *M =
@@ -1797,6 +1812,16 @@
   return ModuleLoadResult::ConfigMismatch;
 }
 
+bool MapPrivateSubModToTopLevel = false;
+// FIXME: Should we detect this at module load time? It seems fairly
+// expensive (and rare).
+Module *SubM = getSubmoduleCorrespondingToPath(
+MapPrivateSubModToTopLevel, M, ImportLoc, Path, Visibility,
+IsInclusionDirective);
+
+if (SubM != M && !SubM->IsFromModuleFile && !MapPrivateSubModToTopLevel)
+  return handleMissingSubmodule(getDiagnostics(), SubM, ImportLoc, Path);
+
 getDiagnostics().Report(ModuleNameLoc, diag::err_module_build_disabled)
 << ModuleName;
 return nullptr;
@@ -2061,8 +2086,9 @@
 //}
 MM.cacheModuleLoad(*Path[0].first, Module);
   } else {
-ModuleLoadResult Result = findOrCompileModuleAndReadAST(
-ModuleName, ImportLoc, ModuleNameLoc, IsInclusionDirective);
+ModuleLoadResult Result =
+findOrCompileModuleAndReadAST(ModuleName, ImportLoc, ModuleNameLoc,
+  Path, Visibility, IsInclusionDirective);
 if (!Result.isNormal())
   return Result;
 if (!Result)
@@ -2077,6 +2103,9 @@
 return ModuleLoadResult();
 
   bool MapPrivateSubModToTopLevel = false;
+  class Module *TopLevelModule = Module;
+  // FIXME: Should we detect this at module load time? It seems fairly
+  // expensive (and rare).
   Module = getSubmoduleCorrespondingToPath(MapPrivateSubModToTopLevel, Module,
ImportLoc, Path, Visibility,
IsInclusionDirective);
@@ -2084,19 +2113,9 @@
   // Make the named module visible, if it's not already part of the module
   // we are parsing.
   if (ModuleName != getLangOpts().CurrentModule) {
-if (!Module->IsFromModuleFile && !MapPrivateSubModToTopLevel) {
-  // We have an umbrella header or directory that doesn't actually include
-  // all of the headers within the directory it covers. Complain about
-  // this missing submodule and recover by forgetting that we ever saw
-  // this submodule.
-  // FIXME: Should we detect this at module load time? It seems fairly
-  // expensive (and rare).
-  getDiagnostics().Report(ImportLoc, diag::warn_missing_submodule)
-

[PATCH] D110927: [analyzer] Access stored value of a constant array through a pointer to another type

2021-11-12 Thread Denys Petrov via Phabricator via cfe-commits

ASDenysPetrov added inline comments.



Comment at: clang/test/Analysis/initialization.cpp:295-299
+void glob_cast_opposite_sign1() {
+  auto *ptr = (unsigned int *)glob_arr2;
+  auto x1 = ptr[0]; // no-warning
+  auto x2 = ptr[1]; // expected-warning{{garbage or undefined}}
+}

steakhal wrote:
> ASDenysPetrov wrote:
> > steakhal wrote:
> > > ASDenysPetrov wrote:
> > > > steakhal wrote:
> > > > > I think it's not correct.
> > > > > 
> > > > > `glob_arr2` refers to an object of dynamic type `int[2]`.
> > > > > And the pointer decayed from it is `int *`, which has //similar 
> > > > > type// to `unsigned *` what you are using to access the memory.
> > > > > Since they are //similar//, this operation should work for all the 
> > > > > valid indices, thus `ptr[0]`, `ptr[1]`, `ptr[2]`, `ptr[3]` should all 
> > > > > be valid.
> > > > > 
> > > > > 
> > > > > glob_arr2 refers to an object of dynamic type int[2].
> > > > `glob_arr2` has an extent of 4.
> > > > > And the pointer decayed from it is int *, which has similar type to 
> > > > > unsigned * what you are using to access the memory.
> > > > Yes, they are the same and it perfectly suits to 
> > > > http://eel.is/c++draft/basic.lval#11 . But still you can't access other 
> > > > element then the first one according to 
> > > > http://eel.is/c++draft/basic.compound#3 : //For purposes of pointer 
> > > > arithmetic ([expr.add]) and comparison ([expr.rel], [expr.eq]), [...] 
> > > > an object of type T that is not an array element is considered to 
> > > > belong to an array with one element of type T. //
> > > > `unsigned int` and `int` are different types according to 
> > > > http://eel.is/c++draft/basic.fundamental#2 . The object of type 
> > > > `unsigned int` is NOT an array, beacuse there is no array of type 
> > > > `unsigned int`. Hence you can only only access the first and a single 
> > > > element of type `unsigned int`.
> > > > 
> > > Yes, `glob_arr` has 4 elements, sorry for this typo.
> > > 
> > > ---
> > > I disagree with the second part though. It seems like gcc disagrees as 
> > > well: https://godbolt.org/z/5o7ozvPar
> > > ```lang=C++
> > > auto foo(unsigned ()[4], int ()[4]) {
> > > p[0] = 2;
> > > q[0] = 1;
> > > return p[0]; // memory read! thus, p and q can alias according to g++.
> > > }
> > > ```
> > > `gcc` still thinks that `p` and `q` can refer to the same memory region 
> > > even if the `-fstrict-aliasing` flag is present.
> > > In other circumstances, it would produce a `mov eax, 2` instead of a 
> > > memory load if `p` and `q` cannot alias.
> > >I disagree with the second part though. It seems like gcc disagrees as 
> > >well: https://godbolt.org/z/5o7ozvPar
> > I see how all the compilers handle this. I've switched several vendors on 
> > Godbolt. They all have the similar behavior. You are right from compiler 
> > perspective, but it's not quite the same in terms of C++ abstract machine.
> > Your example is correct, it works OK and is consistent with the Standard:
> > ```
> > p[0] = 2;
> > q[0] = 1;
> > ```
> > This one also works as expected but goes against the Standard:
> > ```
> > p[1] = 2;
> > q[1] = 1;
> > ```
> > I want you watch this particular part about access via aliased pointers: 
> > https://youtu.be/_qzMpk-22cc?t=2623 For me it seems correct, at least I 
> > can't argue against of it now.
> But in this example we have an array, thus pointer arithmetic should be fine 
> according to the video.
> Could you find the mentioned email discussion? I would be really curious.
> BTW from the analyzer user's perspective, it would be 'better' to find strict 
> aliasing violations which actually matter - now, and risking miscompilations.
I'm not enough confident about that to debate now. As you can see we started 
arguing as well. That means that the Standard really leaves the subject for 
misinterpretation.
OK, let's bring it to the form in which compilers operate. I mean, let's 
legitimate indirections with a different offsets through aliased types: `auto x 
= ((unsigened)arr)[42]; // OK`.
I think it will be enough for this patch for now.


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

https://reviews.llvm.org/D110927

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

[PATCH] D111434: [PowerPC] PPC backend optimization on conditional trap intrustions

2021-11-12 Thread Nemanja Ivanovic via Phabricator via cfe-commits

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

LGTM. There are some very minor nits that can be addressed on the commit.




Comment at: llvm/lib/Target/PowerPC/PPCMIPeephole.cpp:1018
+unsigned Opcode2 = LiMI2->getOpcode();
+bool isOperand2Immeidate = MI.getOperand(2).isImm();
+// We can only do the optimization for the "reg + reg" form.

amyk wrote:
> 
Nit: naming convention - variables start with uppercase letters.



Comment at: llvm/lib/Target/PowerPC/PPCMIPeephole.cpp:1019
+bool isOperand2Immeidate = MI.getOperand(2).isImm();
+// We can only do the optimization for the "reg + reg" form.
+if (!(LiMI1 && (Opcode1 == PPC::LI || Opcode1 == PPC::LI8)))

I don't understand this comment. You say we can only do the optimization for 
the "reg + reg" form but the second condition is actually for the "reg + imm" 
form.



Comment at: llvm/test/CodeGen/PowerPC/mi-peepholes-trap-opt.mir:5
+---
+name:conditional_trap_opt_reg_implicit_def
+alignment:   16

Please add a couple more tests:
1. Test case where we delete the instruction because it won't trap
2. Test case(s) where we do some combination of comparisons (NE(24): `<>`, 
LE(20): `<=`, etc.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111434

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

[PATCH] D110184: [OpenCL] Constructor address space test adjusted for C++ for OpenCL 2021

2021-11-12 Thread Justas Janickas via Phabricator via cfe-commits

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG388e8110db6f: [OpenCL] Constructor address space test 
adjusted for C++ for OpenCL 2021 (authored by Topotuna).
Herald added a subscriber: Naghasan.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110184

Files:
  clang/test/CodeGenOpenCLCXX/addrspace-constructors.clcpp


Index: clang/test/CodeGenOpenCLCXX/addrspace-constructors.clcpp
===
--- clang/test/CodeGenOpenCLCXX/addrspace-constructors.clcpp
+++ clang/test/CodeGenOpenCLCXX/addrspace-constructors.clcpp
@@ -1,4 +1,6 @@
-// RUN: %clang_cc1 %s -emit-llvm -o - -O0 -triple spir-unknown-unknown | 
FileCheck %s
+// RUN: %clang_cc1 %s -cl-std=clc++1.0 -DGENERIC -emit-llvm -o - -O0 -triple 
spir-unknown-unknown | FileCheck %s
+// RUN: %clang_cc1 %s -cl-std=clc++2021 -DGENERIC -emit-llvm -o - -O0 -triple 
spir-unknown-unknown | FileCheck %s
+// RUN: %clang_cc1 %s -cl-std=clc++2021 
-cl-ext=-__opencl_c_generic_address_space,-__opencl_c_pipes -emit-llvm -o - -O0 
-triple spir-unknown-unknown | FileCheck %s
 
 // CHECK: %struct.X = type { i32 }
 
@@ -11,8 +13,10 @@
   int x;
 
   // Local variables are handled in local_addrspace_init.clcpp
-  X() /*__generic*/ : x(0) {}
+  X() /*__generic or __private*/ : x(0) {}
+#if defined(GENERIC)
   X() __private : x(0) {}
+#endif
   X() __global : x(0) {}
   constexpr X() __constant : x(0) {}
   constexpr X(int x) __constant : x(x) {}


Index: clang/test/CodeGenOpenCLCXX/addrspace-constructors.clcpp
===
--- clang/test/CodeGenOpenCLCXX/addrspace-constructors.clcpp
+++ clang/test/CodeGenOpenCLCXX/addrspace-constructors.clcpp
@@ -1,4 +1,6 @@
-// RUN: %clang_cc1 %s -emit-llvm -o - -O0 -triple spir-unknown-unknown | FileCheck %s
+// RUN: %clang_cc1 %s -cl-std=clc++1.0 -DGENERIC -emit-llvm -o - -O0 -triple spir-unknown-unknown | FileCheck %s
+// RUN: %clang_cc1 %s -cl-std=clc++2021 -DGENERIC -emit-llvm -o - -O0 -triple spir-unknown-unknown | FileCheck %s
+// RUN: %clang_cc1 %s -cl-std=clc++2021 -cl-ext=-__opencl_c_generic_address_space,-__opencl_c_pipes -emit-llvm -o - -O0 -triple spir-unknown-unknown | FileCheck %s
 
 // CHECK: %struct.X = type { i32 }
 
@@ -11,8 +13,10 @@
   int x;
 
   // Local variables are handled in local_addrspace_init.clcpp
-  X() /*__generic*/ : x(0) {}
+  X() /*__generic or __private*/ : x(0) {}
+#if defined(GENERIC)
   X() __private : x(0) {}
+#endif
   X() __global : x(0) {}
   constexpr X() __constant : x(0) {}
   constexpr X(int x) __constant : x(x) {}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 388e811 - [OpenCL] Constructor address space test adjusted for C++ for OpenCL 2021

2021-11-12 Thread Justas Janickas via cfe-commits


Author: Justas Janickas
Date: 2021-11-12T14:31:50Z
New Revision: 388e8110db6f7b4ad9d655a70780f8698c4b9fd1

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

LOG: [OpenCL] Constructor address space test adjusted for C++ for OpenCL 2021

Reuses C++ for OpenCL constructor address space test so that it
supports optional generic address spaces in version 2021.

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

Added: 


Modified: 
clang/test/CodeGenOpenCLCXX/addrspace-constructors.clcpp

Removed: 




diff  --git a/clang/test/CodeGenOpenCLCXX/addrspace-constructors.clcpp 
b/clang/test/CodeGenOpenCLCXX/addrspace-constructors.clcpp
index c0913f484f5e3..9ad251d051c77 100644
--- a/clang/test/CodeGenOpenCLCXX/addrspace-constructors.clcpp
+++ b/clang/test/CodeGenOpenCLCXX/addrspace-constructors.clcpp
@@ -1,4 +1,6 @@
-// RUN: %clang_cc1 %s -emit-llvm -o - -O0 -triple spir-unknown-unknown | 
FileCheck %s
+// RUN: %clang_cc1 %s -cl-std=clc++1.0 -DGENERIC -emit-llvm -o - -O0 -triple 
spir-unknown-unknown | FileCheck %s
+// RUN: %clang_cc1 %s -cl-std=clc++2021 -DGENERIC -emit-llvm -o - -O0 -triple 
spir-unknown-unknown | FileCheck %s
+// RUN: %clang_cc1 %s -cl-std=clc++2021 
-cl-ext=-__opencl_c_generic_address_space,-__opencl_c_pipes -emit-llvm -o - -O0 
-triple spir-unknown-unknown | FileCheck %s
 
 // CHECK: %struct.X = type { i32 }
 
@@ -11,8 +13,10 @@ struct X {
   int x;
 
   // Local variables are handled in local_addrspace_init.clcpp
-  X() /*__generic*/ : x(0) {}
+  X() /*__generic or __private*/ : x(0) {}
+#if defined(GENERIC)
   X() __private : x(0) {}
+#endif
   X() __global : x(0) {}
   constexpr X() __constant : x(0) {}
   constexpr X(int x) __constant : x(x) {}



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

[PATCH] D113765: [clangd] Fix function-arg-placeholder suppression with macros.

2021-11-12 Thread Kadir Cetinkaya via Phabricator via cfe-commits

kadircet added inline comments.



Comment at: clang-tools-extra/clangd/CodeComplete.cpp:476
 Completion.Kind == CompletionItemKind::Method ||
 Completion.Kind == CompletionItemKind::Constructor) {
   // If there is a potential template argument list, drop snippet and just

while here, i suppose we should perform drop arguments and parentheses magic 
for macros as well?



Comment at: clang-tools-extra/clangd/CodeComplete.cpp:517
+Completion.Kind == CompletionItemKind::Method ||
+Completion.Kind == CompletionItemKind::Text /*Macro*/) {
   // Functions snippets can be of 2 types:

again while here, maybe introduce constructors into this condition. (Even 
better, what about a `bool isFunctionLikeCompletion(const CompletionItem&)` 
that we can use both here and above?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113765

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

[PATCH] D103317: [Analyzer][Core] Make SValBuilder to better simplify svals with 3 symbols in the tree

2021-11-12 Thread Balázs Benics via Phabricator via cfe-commits

steakhal added a comment.

To me at least, the patch looks good.
Please post some comparative measurements to demonstrate it won't introduce 
runtime regression.




Comment at: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:1144
+  : (SVal)SVB.makeIntVal(*Const);
+  return SVal();
+}

Let's be explicit about it.



Comment at: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:1148-1151
+  SVal Ret = getConst(Sym);
+  if (Ret.isUndef())
+Ret = Visit(Sym);
+  return Ret;





Comment at: clang/test/Analysis/svalbuilder-simplify-compound-svals.cpp:23
+  x = y = z = 1;
+  return 0;
+}

It feels odd that in some of your examples you return some value, but in the 
rest, you don't.



Comment at: clang/test/Analysis/svalbuilder-simplify-compound-svals.cpp:32
+  clang_analyzer_eval(x + y * z == 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(y * z == 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(x == 0); // expected-warning{{TRUE}}

You could additionally assert that `y == 0` and `z == 0`.



Comment at: clang/test/Analysis/svalbuilder-simplify-compound-svals.cpp:50
+void test_SymInt_constrained(int x, int y, int z) {
+  if (x * y * z != 4)
+return;

What if `z` were in the middle? Would it still pass?



Comment at: clang/test/Analysis/svalbuilder-simplify-compound-svals.cpp:65
+  x = 77 / (y + z);
+  if (y + z != 1)
+return;

Would the test pass if you were using `z + y != 1` here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103317

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

[PATCH] D113753: [Analyzer][Core] Better simplification in SimpleSValBuilder::evalBinOpNN

2021-11-12 Thread Balázs Benics via Phabricator via cfe-commits

steakhal added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:378
+  SVal simplifiedLhs = simplifySVal(state, lhs);
+  SVal simplifiedRhs = simplifySVal(state, rhs);
+  if (auto simplifiedLhsAsNonLoc = simplifiedLhs.getAs())

It seems like you simplify the `rhs` as well. Could we have a test for that?



Comment at: clang/test/Analysis/svalbuilder-simplify-in-evalbinop.cpp:13
+void test_evalBinOp_simplifies(int x, int y) {
+  x = y / 77;
+  if (y != 77)

Please declare `x` in this statement.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113753

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

[PATCH] D113765: [clangd] Fix function-arg-placeholder suppression with macros.

2021-11-12 Thread Sam McCall via Phabricator via cfe-commits

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

While here, unhide function-arg-placeholders flag. It's reasonable to want and
maybe we should consider making it default.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D113765

Files:
  clang-tools-extra/clangd/CodeComplete.cpp
  clang-tools-extra/clangd/tool/ClangdMain.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
@@ -2318,6 +2318,15 @@
 UnorderedElementsAre(AllOf(Named("foo_class"), SnippetSuffix("<$0>")),
  AllOf(Named("foo_alias"), 
SnippetSuffix("<$0>";
   }
+  {
+auto Results = completions(
+R"cpp(
+  #define FOO(x, y) x##f
+  FO^ )cpp",
+{}, Opts);
+EXPECT_THAT(Results.Completions, UnorderedElementsAre(AllOf(
+ Named("FOO"), 
SnippetSuffix("($0)";
+  }
 }
 
 TEST(CompletionTest, SuggestOverrides) {
Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -233,7 +233,6 @@
  "function calls. When enabled, completions also contain "
  "placeholders for method parameters"),
 init(CodeCompleteOptions().EnableFunctionArgSnippets),
-Hidden,
 };
 
 opt HeaderInsertion{
Index: clang-tools-extra/clangd/CodeComplete.cpp
===
--- clang-tools-extra/clangd/CodeComplete.cpp
+++ clang-tools-extra/clangd/CodeComplete.cpp
@@ -513,7 +513,8 @@
 if (Snippet->empty())
   return "";
 if (Completion.Kind == CompletionItemKind::Function ||
-Completion.Kind == CompletionItemKind::Method) {
+Completion.Kind == CompletionItemKind::Method ||
+Completion.Kind == CompletionItemKind::Text /*Macro*/) {
   // Functions snippets can be of 2 types:
   // - containing only function arguments, e.g.
   //   foo(${1:int p1}, ${2:int p2});


Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -2318,6 +2318,15 @@
 UnorderedElementsAre(AllOf(Named("foo_class"), SnippetSuffix("<$0>")),
  AllOf(Named("foo_alias"), SnippetSuffix("<$0>";
   }
+  {
+auto Results = completions(
+R"cpp(
+  #define FOO(x, y) x##f
+  FO^ )cpp",
+{}, Opts);
+EXPECT_THAT(Results.Completions, UnorderedElementsAre(AllOf(
+ Named("FOO"), SnippetSuffix("($0)";
+  }
 }
 
 TEST(CompletionTest, SuggestOverrides) {
Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -233,7 +233,6 @@
  "function calls. When enabled, completions also contain "
  "placeholders for method parameters"),
 init(CodeCompleteOptions().EnableFunctionArgSnippets),
-Hidden,
 };
 
 opt HeaderInsertion{
Index: clang-tools-extra/clangd/CodeComplete.cpp
===
--- clang-tools-extra/clangd/CodeComplete.cpp
+++ clang-tools-extra/clangd/CodeComplete.cpp
@@ -513,7 +513,8 @@
 if (Snippet->empty())
   return "";
 if (Completion.Kind == CompletionItemKind::Function ||
-Completion.Kind == CompletionItemKind::Method) {
+Completion.Kind == CompletionItemKind::Method ||
+Completion.Kind == CompletionItemKind::Text /*Macro*/) {
   // Functions snippets can be of 2 types:
   // - containing only function arguments, e.g.
   //   foo(${1:int p1}, ${2:int p2});
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D113754: [Analyzer][Core] Simplify IntSym in SValBuilder

2021-11-12 Thread Balázs Benics via Phabricator via cfe-commits

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

Great stuff. Have you checked the coverage?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113754

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

[clang-tools-extra] ebda5e1 - [clangd] Fix use-after-free in test

2021-11-12 Thread Kadir Cetinkaya via cfe-commits


Author: Kadir Cetinkaya
Date: 2021-11-12T14:50:23+01:00
New Revision: ebda5e1e521f4e7e47ccddb51a6a1d0d586b4265

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

LOG: [clangd] Fix use-after-free in test

Added: 


Modified: 
clang-tools-extra/clangd/unittests/FileIndexTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/unittests/FileIndexTests.cpp 
b/clang-tools-extra/clangd/unittests/FileIndexTests.cpp
index 5b429d61a9e4..a88900c47884 100644
--- a/clang-tools-extra/clangd/unittests/FileIndexTests.cpp
+++ b/clang-tools-extra/clangd/unittests/FileIndexTests.cpp
@@ -736,7 +736,8 @@ TEST(FileIndexTest, MacrosFromMainFile) {
   auto AST = TU.build();
   Idx.updateMain(testPath(TU.Filename), AST);
 
-  auto  = findSymbol(runFuzzyFind(Idx, ""), "FOO");
+  auto Slab = runFuzzyFind(Idx, "");
+  auto  = findSymbol(Slab, "FOO");
   EXPECT_TRUE(FooSymbol.Flags & Symbol::IndexedForCodeCompletion);
 }
 



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

[PATCH] D111434: [PowerPC] PPC backend optimization on conditional trap intrustions

2021-11-12 Thread Amy Kwan via Phabricator via cfe-commits

amyk added inline comments.



Comment at: llvm/lib/Target/PowerPC/PPCMIPeephole.cpp:1018
+unsigned Opcode2 = LiMI2->getOpcode();
+bool isOperand2Immeidate = MI.getOperand(2).isImm();
+// We can only do the optimization for the "reg + reg" form.





Comment at: llvm/lib/Target/PowerPC/PPCMIPeephole.cpp:1020
+// We can only do the optimization for the "reg + reg" form.
+if (!(LiMI1 && (Opcode1 == PPC::LI || Opcode1 == PPC::LI8)))
+  break;

Do we still need to take into account of the lis+ori that Nemanja mentioned?



Comment at: llvm/lib/Target/PowerPC/PPCMIPeephole.cpp:1022
+  break;
+if (!isOperand2Immeidate &&
+!(LiMI2 && (Opcode2 == PPC::LI || Opcode2 == PPC::LI8)))





Comment at: llvm/lib/Target/PowerPC/PPCMIPeephole.cpp:1028
+auto ImmOperand1 = LiMI1->getOperand(1).getImm();
+auto ImmOperand2 = isOperand2Immeidate ? MI.getOperand(2).getImm()
+   : LiMI2->getOperand(1).getImm();




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111434

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

[PATCH] D103395: PR45879: Keep evaluated expression in LValue object

2021-11-12 Thread Serge Pavlov via Phabricator via cfe-commits

sepavloff added a comment.

In D103395#3105677 , @tambre wrote:

> In D103395#3105668 , @sepavloff 
> wrote:
>
>> Strange, I see that it cannot be compiled neither by gcc nor by clang: 
>> https://godbolt.org/z/1dY9Gs6zM. Do I miss something?
>
> Sorry, should've been more specific. Try in C++20 mode: 
> https://godbolt.org/z/4v8b3nsET
> I think the difference might be due to P1331R2 
> , but 
> I'm not sure.

Thank you for this helpful test. I used a bit more accurate use of LHS 
expression and the issue seems resolved.

The new variant uses slightly more careful LHS expression tracking. Previously 
the expression was assigned only once. Now if the expression for LValue can be 
determined, it is updated. Only if the LValue starts to track non-expression 
entities, LHS expression is frozen to the last seen value. It does not affect 
the evaluation of active union member though.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103395

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

[PATCH] D103395: PR45879: Keep evaluated expression in LValue object

2021-11-12 Thread Serge Pavlov via Phabricator via cfe-commits

sepavloff updated this revision to Diff 386823.
sepavloff added a comment.

Update the patch

- Use more careful check for LHS expression,
- Make a bit more precise LHS expression tracking,
- Add comments,
- Add the test from discussion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103395

Files:
  clang/lib/AST/ExprConstant.cpp
  clang/test/SemaCXX/constant-expression-cxx2a.cpp

Index: clang/test/SemaCXX/constant-expression-cxx2a.cpp
===
--- clang/test/SemaCXX/constant-expression-cxx2a.cpp
+++ clang/test/SemaCXX/constant-expression-cxx2a.cpp
@@ -1447,3 +1447,22 @@
   constexpr bool b = [a = S(), b = S()] { return a.p == b.p; }();
   static_assert(!b);
 }
+
+namespace PR45879 {
+struct Base {
+  int m;
+};
+struct Derived : Base {};
+constexpr int k = ((Base{} = Derived{}), 0);
+
+struct sub {
+  char data;
+};
+struct main2 {
+  constexpr main2() {
+member = {};
+  }
+  sub member;
+};
+constexpr main2 a{};
+} // namespace PR45879
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -1546,6 +1546,7 @@
 APValue::LValueBase Base;
 CharUnits Offset;
 SubobjectDesignator Designator;
+const Expr *LExpr = nullptr;
 bool IsNullPtr : 1;
 bool InvalidBase : 1;
 
@@ -1554,6 +1555,7 @@
 const CharUnits () const { return Offset; }
 SubobjectDesignator () { return Designator; }
 const SubobjectDesignator () const { return Designator;}
+const Expr *getExpr() const { return LExpr; }
 bool isNullPointer() const { return IsNullPtr;}
 
 unsigned getLValueCallIndex() const { return Base.getCallIndex(); }
@@ -1591,6 +1593,8 @@
   Offset = CharUnits::fromQuantity(0);
   InvalidBase = BInvalid;
   Designator = SubobjectDesignator(getType(B));
+  if (const Expr *E = B.dyn_cast())
+LExpr = E;
   IsNullPtr = false;
 }
 
@@ -5893,11 +5897,17 @@
 /// operator whose left-hand side might involve a union member access. If it
 /// does, implicitly start the lifetime of any accessed union elements per
 /// C++20 [class.union]5.
+/// Returns true if the assignment is OK and it may be processed further.
 static bool HandleUnionActiveMemberChange(EvalInfo , const Expr *LHSExpr,
   const LValue ) {
   if (LHS.InvalidBase || LHS.Designator.Invalid)
 return false;
 
+  // If no LHS expression was specified, assume the LHS cannot be an assignment
+  // to a union member.
+  if (!LHSExpr)
+return true;
+
   llvm::SmallVector, 4> UnionPathLengths;
   // C++ [class.union]p5:
   //   define the set S(E) of subexpressions of E as follows:
@@ -6101,7 +6111,7 @@
MD->getParent()->isUnion()))
   return false;
 if (Info.getLangOpts().CPlusPlus20 && MD->isTrivial() &&
-!HandleUnionActiveMemberChange(Info, Args[0], *This))
+!HandleUnionActiveMemberChange(Info, This->getExpr(), *This))
   return false;
 if (!handleAssignment(Info, Args[0], *This, MD->getThisType(),
   RHSValue))
@@ -8058,10 +8068,20 @@
 namespace {
 class LValueExprEvaluator
   : public LValueExprEvaluatorBase {
+  friend class LValueExprEvaluatorBase;
+  friend class ExprEvaluatorBase;
+  friend class ConstStmtVisitor;
+  friend class StmtVisitorBase;
 public:
   LValueExprEvaluator(EvalInfo , LValue , bool InvalidBaseOK) :
 LValueExprEvaluatorBaseTy(Info, Result, InvalidBaseOK) {}
 
+  bool Evaluate(const Expr *E) {
+Result.LExpr = E;
+return Visit(E);
+  }
+
+protected:
   bool VisitVarDecl(const Expr *E, const VarDecl *VD);
   bool VisitUnaryPreIncDec(const UnaryOperator *UO);
 
@@ -8123,7 +8143,7 @@
   assert(!E->isValueDependent());
   assert(E->isGLValue() || E->getType()->isFunctionType() ||
  E->getType()->isVoidType() || isa(E));
-  return LValueExprEvaluator(Info, Result, InvalidBaseOK).Visit(E);
+  return LValueExprEvaluator(Info, Result, InvalidBaseOK).Evaluate(E);
 }
 
 bool LValueExprEvaluator::VisitDeclRefExpr(const DeclRefExpr *E) {
@@ -8424,7 +8444,7 @@
 
   // C++17 onwards require that we evaluate the RHS first.
   APValue RHS;
-  if (!Evaluate(RHS, this->Info, CAO->getRHS())) {
+  if (!::Evaluate(RHS, this->Info, CAO->getRHS())) {
 if (!Info.noteFailure())
   return false;
 Success = false;
@@ -8448,7 +8468,7 @@
 
   // C++17 onwards require that we evaluate the RHS first.
   APValue NewVal;
-  if (!Evaluate(NewVal, this->Info, E->getRHS())) {
+  if (!::Evaluate(NewVal, this->Info, E->getRHS())) {
 if (!Info.noteFailure())
   return false;
 Success = false;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D110216: [clang] retain type sugar in auto / template argument deduction

2021-11-12 Thread Aaron Puchert via Phabricator via cfe-commits

aaronpuchert added a comment.

Obviously the error message was just extended by the as-written type. Could 
have just adapted the expected results instead of reverting...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110216

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

[PATCH] D113489: [AArch64][SVE] Instcombine SVE LD1/ST1 to stock LLVM IR

2021-11-12 Thread Matt Devereau via Phabricator via cfe-commits

MattDevereau marked 9 inline comments as done.
MattDevereau added inline comments.



Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp:732
+  Type *VecTyPtr = II.getType()->getPointerTo();
+  IRBuilder<> Builder(II.getContext());
+  Builder.SetInsertPoint();

DavidTruby wrote:
> Nit: I think the default template arguments should just be picked without the 
> `<>`
I'm getting compiler errors when omitting `<>`



Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp:750
+  Value *PtrOp = II.getOperand(1);
+  IRBuilder<> Builder(II.getContext());
+  Builder.SetInsertPoint();

DavidTruby wrote:
> 
I'm getting compiler errors when omitting <>



Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp:765
+  Type *VecTyPtr = VecOp->getType()->getPointerTo();
+  IRBuilder<> Builder(II.getContext());
+  Builder.SetInsertPoint();

DavidTruby wrote:
> 
I'm getting compiler errors when omitting <>


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113489

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

[PATCH] D113761: [clang][modules] NFC: Factor out path-based submodule lookup

2021-11-12 Thread Jan Svoboda via Phabricator via cfe-commits

jansvoboda11 created this revision.
jansvoboda11 added reviewers: Bigcheese, dexonsmith, vsapsai.
jansvoboda11 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This is a prep patch for Dxx that factors out the path-based submodule 
lookup used in `CompilerInstance::loadModule`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D113761

Files:
  clang/include/clang/Frontend/CompilerInstance.h
  clang/lib/Frontend/CompilerInstance.cpp

Index: clang/lib/Frontend/CompilerInstance.cpp
===
--- clang/lib/Frontend/CompilerInstance.cpp
+++ clang/lib/Frontend/CompilerInstance.cpp
@@ -1931,65 +1931,12 @@
   return M;
 }
 
-ModuleLoadResult
-CompilerInstance::loadModule(SourceLocation ImportLoc,
- ModuleIdPath Path,
- Module::NameVisibilityKind Visibility,
- bool IsInclusionDirective) {
-  // Determine what file we're searching from.
-  StringRef ModuleName = Path[0].first->getName();
-  SourceLocation ModuleNameLoc = Path[0].second;
-
-  // If we've already handled this import, just return the cached result.
-  // This one-element cache is important to eliminate redundant diagnostics
-  // when both the preprocessor and parser see the same import declaration.
-  if (ImportLoc.isValid() && LastModuleImportLoc == ImportLoc) {
-// Make the named module visible.
-if (LastModuleImportResult && ModuleName != getLangOpts().CurrentModule)
-  TheASTReader->makeModuleVisible(LastModuleImportResult, Visibility,
-  ImportLoc);
-return LastModuleImportResult;
-  }
-
-  // If we don't already have information on this module, load the module now.
-  Module *Module = nullptr;
-  ModuleMap  = getPreprocessor().getHeaderSearchInfo().getModuleMap();
-  if (auto MaybeModule = MM.getCachedModuleLoad(*Path[0].first)) {
-// Use the cached result, which may be nullptr.
-Module = *MaybeModule;
-  } else if (ModuleName == getLangOpts().CurrentModule) {
-// This is the module we're building.
-Module = PP->getHeaderSearchInfo().lookupModule(
-ModuleName, ImportLoc, /*AllowSearch*/ true,
-/*AllowExtraModuleMapSearch*/ !IsInclusionDirective);
-/// FIXME: perhaps we should (a) look for a module using the module name
-//  to file map (PrebuiltModuleFiles) and (b) diagnose if still not found?
-//if (Module == nullptr) {
-//  getDiagnostics().Report(ModuleNameLoc, diag::err_module_not_found)
-//<< ModuleName;
-//  DisableGeneratingGlobalModuleIndex = true;
-//  return ModuleLoadResult();
-//}
-MM.cacheModuleLoad(*Path[0].first, Module);
-  } else {
-ModuleLoadResult Result = findOrCompileModuleAndReadAST(
-ModuleName, ImportLoc, ModuleNameLoc, IsInclusionDirective);
-if (!Result.isNormal())
-  return Result;
-if (!Result)
-  DisableGeneratingGlobalModuleIndex = true;
-Module = Result;
-MM.cacheModuleLoad(*Path[0].first, Module);
-  }
-
-  // If we never found the module, fail.  Otherwise, verify the module and link
-  // it up.
-  if (!Module)
-return ModuleLoadResult();
+Module *CompilerInstance::getSubmoduleCorrespondingToPath(
+bool , Module *Module, SourceLocation ImportLoc,
+ModuleIdPath Path, Module::NameVisibilityKind Visibility,
+bool IsInclusionDirective) {
+  MapPrivateSubModToTopLevel = false;
 
-  // Verify that the rest of the module path actually corresponds to
-  // a submodule.
-  bool MapPrivateSubModToTopLevel = false;
   for (unsigned I = 1, N = Path.size(); I != N; ++I) {
 StringRef Name = Path[I].first->getName();
 clang::Module *Sub = Module->findSubmodule(Name);
@@ -2070,6 +2017,70 @@
 Module = Sub;
   }
 
+  return Module;
+}
+
+ModuleLoadResult
+CompilerInstance::loadModule(SourceLocation ImportLoc,
+ ModuleIdPath Path,
+ Module::NameVisibilityKind Visibility,
+ bool IsInclusionDirective) {
+  // Determine what file we're searching from.
+  StringRef ModuleName = Path[0].first->getName();
+  SourceLocation ModuleNameLoc = Path[0].second;
+
+  // If we've already handled this import, just return the cached result.
+  // This one-element cache is important to eliminate redundant diagnostics
+  // when both the preprocessor and parser see the same import declaration.
+  if (ImportLoc.isValid() && LastModuleImportLoc == ImportLoc) {
+// Make the named module visible.
+if (LastModuleImportResult && ModuleName != getLangOpts().CurrentModule)
+  TheASTReader->makeModuleVisible(LastModuleImportResult, Visibility,
+  ImportLoc);
+return LastModuleImportResult;
+  }
+
+  // If we don't already have information on this module, load the module now.
+  Module *Module = nullptr;
+  ModuleMap  =

[clang] ab6ef58 - [clang] NFC: Format a loop in CompilerInstance

2021-11-12 Thread Jan Svoboda via cfe-commits


Author: Jan Svoboda
Date: 2021-11-12T14:16:06+01:00
New Revision: ab6ef5872763fd84e714c30467a49ad41d81bafc

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

LOG: [clang] NFC: Format a loop in CompilerInstance

This code will be moved to a separate function in a future patch. Reformatting 
now to prevent a bunch of clang-format complains on Phabricator.

Added: 


Modified: 
clang/lib/Frontend/CompilerInstance.cpp

Removed: 




diff  --git a/clang/lib/Frontend/CompilerInstance.cpp 
b/clang/lib/Frontend/CompilerInstance.cpp
index ee62d702a762..1432607204bd 100644
--- a/clang/lib/Frontend/CompilerInstance.cpp
+++ b/clang/lib/Frontend/CompilerInstance.cpp
@@ -2008,10 +2008,9 @@ CompilerInstance::loadModule(SourceLocation ImportLoc,
   PrivateModule, PP->getIdentifierInfo(Module->Name)->getTokenID());
   PrivPath.push_back(std::make_pair(, Path[0].second));
 
-  if (PP->getHeaderSearchInfo().lookupModule(PrivateModule, ImportLoc,
- true, !IsInclusionDirective))
-Sub =
-loadModule(ImportLoc, PrivPath, Visibility, IsInclusionDirective);
+  if (PP->getHeaderSearchInfo().lookupModule(PrivateModule, ImportLoc, 
true,
+ !IsInclusionDirective))
+Sub = loadModule(ImportLoc, PrivPath, Visibility, 
IsInclusionDirective);
   if (Sub) {
 MapPrivateSubModToTopLevel = true;
 if (!getDiagnostics().isIgnored(
@@ -2034,9 +2033,9 @@ CompilerInstance::loadModule(SourceLocation ImportLoc,
   unsigned BestEditDistance = (std::numeric_limits::max)();
 
   for (class Module *SubModule : Module->submodules()) {
-unsigned ED = Name.edit_distance(SubModule->Name,
- /*AllowReplacements=*/true,
- BestEditDistance);
+unsigned ED =
+Name.edit_distance(SubModule->Name,
+   /*AllowReplacements=*/true, BestEditDistance);
 if (ED <= BestEditDistance) {
   if (ED < BestEditDistance) {
 Best.clear();
@@ -2049,12 +2048,11 @@ CompilerInstance::loadModule(SourceLocation ImportLoc,
 
   // If there was a clear winner, user it.
   if (Best.size() == 1) {
-getDiagnostics().Report(Path[I].second,
-diag::err_no_submodule_suggest)
-  << Path[I].first << Module->getFullModuleName() << Best[0]
-  << SourceRange(Path[0].second, Path[I-1].second)
-  << FixItHint::CreateReplacement(SourceRange(Path[I].second),
-  Best[0]);
+getDiagnostics().Report(Path[I].second, diag::err_no_submodule_suggest)
+<< Path[I].first << Module->getFullModuleName() << Best[0]
+<< SourceRange(Path[0].second, Path[I - 1].second)
+<< FixItHint::CreateReplacement(SourceRange(Path[I].second),
+Best[0]);
 
 Sub = Module->findSubmodule(Best[0]);
   }
@@ -2064,8 +2062,8 @@ CompilerInstance::loadModule(SourceLocation ImportLoc,
   // No submodule by this name. Complain, and don't look for further
   // submodules.
   getDiagnostics().Report(Path[I].second, diag::err_no_submodule)
-<< Path[I].first << Module->getFullModuleName()
-<< SourceRange(Path[0].second, Path[I-1].second);
+  << Path[I].first << Module->getFullModuleName()
+  << SourceRange(Path[0].second, Path[I - 1].second);
   break;
 }
 



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

[PATCH] D113555: [clangd] Mark macros from preamble for code completion

2021-11-12 Thread Kadir Cetinkaya via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
kadircet marked an inline comment as done.
Closed by commit rG7d668ae38d2d: [clangd] Mark macros from preamble for code 
completion (authored by kadircet).

Changed prior to commit:
  https://reviews.llvm.org/D113555?vs=386107=386807#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113555

Files:
  clang-tools-extra/clangd/index/SymbolCollector.cpp
  clang-tools-extra/clangd/unittests/FileIndexTests.cpp


Index: clang-tools-extra/clangd/unittests/FileIndexTests.cpp
===
--- clang-tools-extra/clangd/unittests/FileIndexTests.cpp
+++ clang-tools-extra/clangd/unittests/FileIndexTests.cpp
@@ -727,6 +727,19 @@
   EXPECT_THAT(MT.child("f3").children(), ElementsAre(Pair("relations", _)));
   EXPECT_THAT(MT.child("f3").total(), Gt(0U));
 }
+
+TEST(FileIndexTest, MacrosFromMainFile) {
+  FileIndex Idx;
+  TestTU TU;
+  TU.Code = "#pragma once\n#define FOO";
+  TU.Filename = "foo.h";
+  auto AST = TU.build();
+  Idx.updateMain(testPath(TU.Filename), AST);
+
+  auto  = findSymbol(runFuzzyFind(Idx, ""), "FOO");
+  EXPECT_TRUE(FooSymbol.Flags & Symbol::IndexedForCodeCompletion);
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/index/SymbolCollector.cpp
===
--- clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -564,6 +564,12 @@
 S.SymInfo.Lang = index::SymbolLanguage::C;
 S.Origin = Opts.Origin;
 S.CanonicalDeclaration = R.Location;
+// Make the macro visible for code completion if main file is an
+// include-able header.
+if (!HeaderFileURIs->getIncludeHeader(SM.getMainFileID()).empty()) {
+  S.Flags |= Symbol::IndexedForCodeCompletion;
+  S.Flags |= Symbol::VisibleOutsideFile;
+}
 Symbols.insert(S);
   }
 }


Index: clang-tools-extra/clangd/unittests/FileIndexTests.cpp
===
--- clang-tools-extra/clangd/unittests/FileIndexTests.cpp
+++ clang-tools-extra/clangd/unittests/FileIndexTests.cpp
@@ -727,6 +727,19 @@
   EXPECT_THAT(MT.child("f3").children(), ElementsAre(Pair("relations", _)));
   EXPECT_THAT(MT.child("f3").total(), Gt(0U));
 }
+
+TEST(FileIndexTest, MacrosFromMainFile) {
+  FileIndex Idx;
+  TestTU TU;
+  TU.Code = "#pragma once\n#define FOO";
+  TU.Filename = "foo.h";
+  auto AST = TU.build();
+  Idx.updateMain(testPath(TU.Filename), AST);
+
+  auto  = findSymbol(runFuzzyFind(Idx, ""), "FOO");
+  EXPECT_TRUE(FooSymbol.Flags & Symbol::IndexedForCodeCompletion);
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/index/SymbolCollector.cpp
===
--- clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -564,6 +564,12 @@
 S.SymInfo.Lang = index::SymbolLanguage::C;
 S.Origin = Opts.Origin;
 S.CanonicalDeclaration = R.Location;
+// Make the macro visible for code completion if main file is an
+// include-able header.
+if (!HeaderFileURIs->getIncludeHeader(SM.getMainFileID()).empty()) {
+  S.Flags |= Symbol::IndexedForCodeCompletion;
+  S.Flags |= Symbol::VisibleOutsideFile;
+}
 Symbols.insert(S);
   }
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] 7d668ae - [clangd] Mark macros from preamble for code completion

2021-11-12 Thread Kadir Cetinkaya via cfe-commits


Author: Kadir Cetinkaya
Date: 2021-11-12T14:01:14+01:00
New Revision: 7d668ae38d2d54ebd5eca5f66a8cf353c3526dc3

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

LOG: [clangd] Mark macros from preamble for code completion

If the main file is a header, mark the marcos defined in its preamble
section as code-completion ready.

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

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

Added: 


Modified: 
clang-tools-extra/clangd/index/SymbolCollector.cpp
clang-tools-extra/clangd/unittests/FileIndexTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/index/SymbolCollector.cpp 
b/clang-tools-extra/clangd/index/SymbolCollector.cpp
index 0591c5113a23a..dfcd4cb50a130 100644
--- a/clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ b/clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -564,6 +564,12 @@ void SymbolCollector::handleMacros(const MainFileMacros 
) {
 S.SymInfo.Lang = index::SymbolLanguage::C;
 S.Origin = Opts.Origin;
 S.CanonicalDeclaration = R.Location;
+// Make the macro visible for code completion if main file is an
+// include-able header.
+if (!HeaderFileURIs->getIncludeHeader(SM.getMainFileID()).empty()) {
+  S.Flags |= Symbol::IndexedForCodeCompletion;
+  S.Flags |= Symbol::VisibleOutsideFile;
+}
 Symbols.insert(S);
   }
 }

diff  --git a/clang-tools-extra/clangd/unittests/FileIndexTests.cpp 
b/clang-tools-extra/clangd/unittests/FileIndexTests.cpp
index 5f1998af3c21f..5b429d61a9e46 100644
--- a/clang-tools-extra/clangd/unittests/FileIndexTests.cpp
+++ b/clang-tools-extra/clangd/unittests/FileIndexTests.cpp
@@ -727,6 +727,19 @@ TEST(FileSymbolsTest, Profile) {
   EXPECT_THAT(MT.child("f3").children(), ElementsAre(Pair("relations", _)));
   EXPECT_THAT(MT.child("f3").total(), Gt(0U));
 }
+
+TEST(FileIndexTest, MacrosFromMainFile) {
+  FileIndex Idx;
+  TestTU TU;
+  TU.Code = "#pragma once\n#define FOO";
+  TU.Filename = "foo.h";
+  auto AST = TU.build();
+  Idx.updateMain(testPath(TU.Filename), AST);
+
+  auto  = findSymbol(runFuzzyFind(Idx, ""), "FOO");
+  EXPECT_TRUE(FooSymbol.Flags & Symbol::IndexedForCodeCompletion);
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang



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

[PATCH] D51650: Implement target_clones multiversioning

2021-11-12 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added a comment.

In D51650#3126569 , @akuegel wrote:

> Since it is not clear whether the semantic change was intended, I think it 
> makes sense to revert the patch for now. If it is intended, it might be good 
> to mention it in the change description, so that people are warned.

That looks like an unintended change to me, likely due to the new mutual 
exclusion checks. Thanks for letting us know!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D51650

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

[PATCH] D103317: [Analyzer][Core] Make SValBuilder to better simplify svals with 3 symbols in the tree

2021-11-12 Thread Gabor Marton via Phabricator via cfe-commits

martong updated this revision to Diff 386806.
martong edited the summary of this revision.
martong added a comment.

- Update in comments: `Unknown` -> `Undef`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103317

Files:
  clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  clang/test/Analysis/svalbuilder-simplify-compound-svals.cpp
  clang/test/Analysis/taint-tester.c

Index: clang/test/Analysis/taint-tester.c
===
--- clang/test/Analysis/taint-tester.c
+++ clang/test/Analysis/taint-tester.c
@@ -59,7 +59,7 @@
   int tty = xy.y; // expected-warning + {{tainted}}
 }
 
-void BitwiseOp(int in, char inn) {
+void BitwiseOp(int in, char inn, int zz) {
   // Taint on bitwise operations, integer to integer cast.
   int m;
   int x = 0;
@@ -67,11 +67,12 @@
   int y = (in << (x << in)) * 5;// expected-warning + {{tainted}}
   // The next line tests integer to integer cast.
   int z = y & inn; // expected-warning + {{tainted}}
-  if (y == 5) // expected-warning + {{tainted}}
+  if (y == zz) { // expected-warning + {{tainted}}
 m = z | z;// expected-warning + {{tainted}}
+  }
   else
 m = inn;
-  int mm = m; // expected-warning + {{tainted}}
+  int mm = m; // expected-warning 1 {{tainted}}
 }
 
 // Test getenv.
Index: clang/test/Analysis/svalbuilder-simplify-compound-svals.cpp
===
--- /dev/null
+++ clang/test/Analysis/svalbuilder-simplify-compound-svals.cpp
@@ -0,0 +1,69 @@
+// RUN: %clang_analyze_cc1 %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=debug.ExprInspection \
+// RUN:   -analyzer-config eagerly-assume=false \
+// RUN:   -verify
+
+// Here we test whether the SValBuilder is capable to simplify existing
+// compound SVals (where there are at leaset 3 symbols in the tree) based on
+// newly added constraints.
+
+void clang_analyzer_eval(bool);
+void clang_analyzer_warnIfReached();
+
+int test_left_tree_constrained(int x, int y, int z) {
+  if (x + y + z != 0)
+return 0;
+  if (x + y != 0)
+return 0;
+  clang_analyzer_eval(x + y + z == 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(x + y == 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(z == 0); // expected-warning{{TRUE}}
+  x = y = z = 1;
+  return 0;
+}
+
+int test_right_tree_constrained(int x, int y, int z) {
+  if (x + y * z != 0)
+return 0;
+  if (y * z != 0)
+return 0;
+  clang_analyzer_eval(x + y * z == 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(y * z == 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(x == 0); // expected-warning{{TRUE}}
+  return 0;
+}
+
+int test_left_tree_constrained_minus(int x, int y, int z) {
+  if (x - y - z != 0)
+return 0;
+  if (x - y != 0)
+return 0;
+  clang_analyzer_eval(x - y - z == 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(x - y == 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(z == 0); // expected-warning{{TRUE}}
+  x = y = z = 1;
+  return 0;
+}
+
+void test_SymInt_constrained(int x, int y, int z) {
+  if (x * y * z != 4)
+return;
+  if (z != 2)
+return;
+  if (x * y == 3) {
+clang_analyzer_warnIfReached(); // no-warning
+return;
+  }
+  (void)(x * y * z);
+}
+
+void test_SValBuilder_simplifies_IntSym(int x, int y, int z) {
+  // Most IntSym BinOps are transformed to SymInt in SimpleSValBuilder.
+  // Division is one exception.
+  x = 77 / (y + z);
+  if (y + z != 1)
+return;
+  clang_analyzer_eval(x == 77); // expected-warning{{TRUE}}
+  (void)(x * y * z);
+}
Index: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
===
--- clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
+++ clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
@@ -1102,7 +1102,6 @@
   if (SymbolRef Sym = V.getAsSymbol())
 return state->getConstraintManager().getSymVal(state, Sym);
 
-  // FIXME: Add support for SymExprs.
   return nullptr;
 }
 
@@ -1134,6 +1133,24 @@
   return cache(Sym, SVB.makeSymbolVal(Sym));
 }
 
+// Return the known const value for the Sym if available, or return Undef
+// otherwise.
+SVal getConst(SymbolRef Sym) {
+  const llvm::APSInt *Const =
+  State->getConstraintManager().getSymVal(State, Sym);
+  if (Const)
+return Loc::isLocType(Sym->getType()) ? (SVal)SVB.makeIntLocVal(*Const)
+  : (SVal)SVB.makeIntVal(*Const);
+  return SVal();
+}
+
+SVal getConstOrVisit(SymbolRef Sym) {
+  SVal Ret = getConst(Sym);
+  if (Ret.isUndef())
+Ret = Visit(Sym);
+  return Ret;
+}
+
   public:
 Simplifier(ProgramStateRef State)
 : State(State), SVB(State->getStateManager().getSValBuilder()) {}
@@ -1147,15 +1164,14 @@
   return

[PATCH] D113641: [llvm] Add a SFINAE template parameter to DenseMapInfo

2021-11-12 Thread Chris Lattner via Phabricator via cfe-commits

lattner added a comment.

Oh, are you concerned about staging this in?  If you want to stage it (add the 
includes now, remove them later or something), that is totally fine with me.  
Maybe I don't understand the impact correctly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113641

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

[PATCH] D113641: [llvm] Add a SFINAE template parameter to DenseMapInfo

2021-11-12 Thread Chris Lattner via Phabricator via cfe-commits

lattner added a comment.

I think a few void's probably won't hurt anyone?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113641

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

[PATCH] D113641: [llvm] Add a SFINAE template parameter to DenseMapInfo

2021-11-12 Thread River Riddle via Phabricator via cfe-commits

rriddle added inline comments.



Comment at: llvm/include/llvm/ADT/Hashing.h:59
 namespace llvm {
-template  struct DenseMapInfo;
 

lattner wrote:
> Is there a way to keep the forward declarations references here instead of 
> #include?  DenseMapInfo.h pulls in a ton of stuff including  and 
> 
I mentioned it in a comment. If we don't want the includes, we'll need to 
sprinkle `void` everywhere that we specialize the template. (I don't have a 
preference either way)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113641

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

[PATCH] D113641: [llvm] Add a SFINAE template parameter to DenseMapInfo

2021-11-12 Thread Chris Lattner via Phabricator via cfe-commits

lattner accepted this revision.
lattner added a comment.
Herald added a subscriber: sdasgup3.

Nice, I'm very excited about this - this will allow a lot of cleanups.  Thank 
you for doing this!




Comment at: llvm/include/llvm/ADT/Hashing.h:59
 namespace llvm {
-template  struct DenseMapInfo;
 

Is there a way to keep the forward declarations references here instead of 
#include?  DenseMapInfo.h pulls in a ton of stuff including  and 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113641

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

[PATCH] D113754: [Analyzer][Core] Simplify IntSym in SValBuilder

2021-11-12 Thread Gabor Marton via Phabricator via cfe-commits

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

Make the SimpleSValBuilder capable to simplify existing IntSym
expressions based on a newly added constraint on the sub-expression.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D113754

Files:
  clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  clang/test/Analysis/svalbuilder-simplify-intsym.cpp


Index: clang/test/Analysis/svalbuilder-simplify-intsym.cpp
===
--- /dev/null
+++ clang/test/Analysis/svalbuilder-simplify-intsym.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang_analyze_cc1 %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=debug.ExprInspection \
+// RUN:   -analyzer-config eagerly-assume=false \
+// RUN:   -verify
+
+// Here we test whether the SValBuilder is capable to simplify existing
+// IntSym expressions based on a newly added constraint on the sub-expression.
+
+void clang_analyzer_eval(bool);
+
+void test_SValBuilder_simplifies_IntSym(int x, int y) {
+  // Most IntSym BinOps are transformed to SymInt in SimpleSValBuilder.
+  // Division is one exception.
+  x = 77 / y;
+  if (y != 1)
+return;
+  clang_analyzer_eval(x == 77); // expected-warning{{TRUE}}
+  (void)(x * y);
+}
Index: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
===
--- clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
+++ clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
@@ -1183,6 +1183,19 @@
   S, SVB.evalBinOp(State, S->getOpcode(), LHS, RHS, S->getType()));
 }
 
+SVal VisitIntSymExpr(const IntSymExpr *S) {
+  auto I = Cached.find(S);
+  if (I != Cached.end())
+return I->second;
+
+  SVal RHS = Visit(S->getRHS());
+  if (isUnchanged(S->getRHS(), RHS))
+return skip(S);
+  SVal LHS = SVB.makeIntVal(S->getLHS());
+  return cache(
+  S, SVB.evalBinOp(State, S->getOpcode(), LHS, RHS, S->getType()));
+}
+
 SVal VisitSymSymExpr(const SymSymExpr *S) {
   auto I = Cached.find(S);
   if (I != Cached.end())


Index: clang/test/Analysis/svalbuilder-simplify-intsym.cpp
===
--- /dev/null
+++ clang/test/Analysis/svalbuilder-simplify-intsym.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang_analyze_cc1 %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=debug.ExprInspection \
+// RUN:   -analyzer-config eagerly-assume=false \
+// RUN:   -verify
+
+// Here we test whether the SValBuilder is capable to simplify existing
+// IntSym expressions based on a newly added constraint on the sub-expression.
+
+void clang_analyzer_eval(bool);
+
+void test_SValBuilder_simplifies_IntSym(int x, int y) {
+  // Most IntSym BinOps are transformed to SymInt in SimpleSValBuilder.
+  // Division is one exception.
+  x = 77 / y;
+  if (y != 1)
+return;
+  clang_analyzer_eval(x == 77); // expected-warning{{TRUE}}
+  (void)(x * y);
+}
Index: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
===
--- clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
+++ clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
@@ -1183,6 +1183,19 @@
   S, SVB.evalBinOp(State, S->getOpcode(), LHS, RHS, S->getType()));
 }
 
+SVal VisitIntSymExpr(const IntSymExpr *S) {
+  auto I = Cached.find(S);
+  if (I != Cached.end())
+return I->second;
+
+  SVal RHS = Visit(S->getRHS());
+  if (isUnchanged(S->getRHS(), RHS))
+return skip(S);
+  SVal LHS = SVB.makeIntVal(S->getLHS());
+  return cache(
+  S, SVB.evalBinOp(State, S->getOpcode(), LHS, RHS, S->getType()));
+}
+
 SVal VisitSymSymExpr(const SymSymExpr *S) {
   auto I = Cached.find(S);
   if (I != Cached.end())
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D113753: [Analyzer][Core] Better simplification in SimpleSValBuilder::evalBinOpNN

2021-11-12 Thread Gabor Marton via Phabricator via cfe-commits

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

Make the SValBuilder capable to simplify existing
SVals based on a newly added constraints when evaluating a BinOp.

Before this patch, we called `simplify` only in some edge cases.
However, we can and should investigate the constraints in all cases.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D113753

Files:
  clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  clang/test/Analysis/svalbuilder-simplify-in-evalbinop.cpp


Index: clang/test/Analysis/svalbuilder-simplify-in-evalbinop.cpp
===
--- /dev/null
+++ clang/test/Analysis/svalbuilder-simplify-in-evalbinop.cpp
@@ -0,0 +1,18 @@
+// RUN: %clang_analyze_cc1 %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=debug.ExprInspection \
+// RUN:   -analyzer-config eagerly-assume=false \
+// RUN:   -verify
+
+// Here we test whether the SValBuilder is capable to simplify existing
+// SVals based on a newly added constraints when evaluating a BinOp.
+
+void clang_analyzer_eval(bool);
+
+void test_evalBinOp_simplifies(int x, int y) {
+  x = y / 77;
+  if (y != 77)
+return;
+  clang_analyzer_eval(x == 1); // expected-warning{{TRUE}}
+  (void)(x * y);
+}
Index: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
===
--- clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
+++ clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
@@ -372,6 +372,15 @@
   NonLoc InputLHS = lhs;
   NonLoc InputRHS = rhs;
 
+  // Constraints may have changed since the creation of a bound SVal. Check if
+  // the values can be simplified based on those new constraints.
+  SVal simplifiedLhs = simplifySVal(state, lhs);
+  SVal simplifiedRhs = simplifySVal(state, rhs);
+  if (auto simplifiedLhsAsNonLoc = simplifiedLhs.getAs())
+lhs = *simplifiedLhsAsNonLoc;
+  if (auto simplifiedRhsAsNonLoc = simplifiedRhs.getAs())
+rhs = *simplifiedRhsAsNonLoc;
+
   // Handle trivial case where left-side and right-side are the same.
   if (lhs == rhs)
 switch (op) {
@@ -619,16 +628,6 @@
 }
   }
 
-  // Does the symbolic expression simplify to a constant?
-  // If so, "fold" the constant by setting 'lhs' to a ConcreteInt
-  // and try again.
-  SVal simplifiedLhs = simplifySVal(state, lhs);
-  if (simplifiedLhs != lhs)
-if (auto simplifiedLhsAsNonLoc = simplifiedLhs.getAs()) {
-  lhs = *simplifiedLhsAsNonLoc;
-  continue;
-}
-
   // Is the RHS a constant?
   if (const llvm::APSInt *RHSValue = getKnownValue(state, rhs))
 return MakeSymIntVal(Sym, op, *RHSValue, resultTy);


Index: clang/test/Analysis/svalbuilder-simplify-in-evalbinop.cpp
===
--- /dev/null
+++ clang/test/Analysis/svalbuilder-simplify-in-evalbinop.cpp
@@ -0,0 +1,18 @@
+// RUN: %clang_analyze_cc1 %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=debug.ExprInspection \
+// RUN:   -analyzer-config eagerly-assume=false \
+// RUN:   -verify
+
+// Here we test whether the SValBuilder is capable to simplify existing
+// SVals based on a newly added constraints when evaluating a BinOp.
+
+void clang_analyzer_eval(bool);
+
+void test_evalBinOp_simplifies(int x, int y) {
+  x = y / 77;
+  if (y != 77)
+return;
+  clang_analyzer_eval(x == 1); // expected-warning{{TRUE}}
+  (void)(x * y);
+}
Index: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
===
--- clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
+++ clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
@@ -372,6 +372,15 @@
   NonLoc InputLHS = lhs;
   NonLoc InputRHS = rhs;
 
+  // Constraints may have changed since the creation of a bound SVal. Check if
+  // the values can be simplified based on those new constraints.
+  SVal simplifiedLhs = simplifySVal(state, lhs);
+  SVal simplifiedRhs = simplifySVal(state, rhs);
+  if (auto simplifiedLhsAsNonLoc = simplifiedLhs.getAs())
+lhs = *simplifiedLhsAsNonLoc;
+  if (auto simplifiedRhsAsNonLoc = simplifiedRhs.getAs())
+rhs = *simplifiedRhsAsNonLoc;
+
   // Handle trivial case where left-side and right-side are the same.
   if (lhs == rhs)
 switch (op) {
@@ -619,16 +628,6 @@
 }
   }
 
-  // Does the symbolic expression simplify to a constant?
-  // If so, "fold" the constant by setting 'lhs' to a ConcreteInt
-  // and try again.
-  SVal simplifiedLhs = simplifySVal(state, lhs);
-  if (simplifiedLhs != lhs)
-if (auto

[PATCH] D113752: [Parse] Use empty RecoveryExpr when if/while/do/switch conditions fail to parse

2021-11-12 Thread Sam McCall via Phabricator via cfe-commits

sammccall updated this revision to Diff 386797.
sammccall added a comment.

More conservative in changes to ActOnIfStmt


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113752

Files:
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/ParseExprCXX.cpp
  clang/lib/Parse/ParseStmt.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/TreeTransform.h
  clang/test/AST/ast-dump-invalid.cpp
  clang/test/AST/loop-recovery.cpp
  clang/test/Parser/cxx0x-attributes.cpp
  clang/test/Sema/complex-int.c
  clang/test/SemaCXX/condition.cpp

Index: clang/test/SemaCXX/condition.cpp
===
--- clang/test/SemaCXX/condition.cpp
+++ clang/test/SemaCXX/condition.cpp
@@ -20,6 +20,8 @@
   while (struct S {} *x=0) ; // expected-error {{'S' cannot be defined in a condition}}
   while (struct {} *x=0) ; // expected-error-re {{'(unnamed struct at {{.*}})' cannot be defined in a condition}}
   switch (enum {E} x=0) ; // expected-error-re {{'(unnamed enum at {{.*}})' cannot be defined in a condition}}
+  // expected-warning@-1 {{switch statement has empty body}}
+  // expected-note@-2 {{put the semicolon on a separate line}}
 
   if (int x=0) { // expected-note 2 {{previous definition is here}}
 int x;  // expected-error {{redefinition of 'x'}}
Index: clang/test/Sema/complex-int.c
===
--- clang/test/Sema/complex-int.c
+++ clang/test/Sema/complex-int.c
@@ -18,8 +18,8 @@
 result = xx*yy;
 
 switch (arr) { // expected-error{{statement requires expression of integer type ('_Complex int' invalid)}}
-  case brr: ;
-  case xx: ;
+  case brr: ; // expected-error{{integer constant expression must have integer type}}
+  case xx: ; // expected-error{{integer constant expression must have integer type}}
 }
 switch (ii) {
   case brr: ; // expected-error{{integer constant expression must have integer type}}
Index: clang/test/Parser/cxx0x-attributes.cpp
===
--- clang/test/Parser/cxx0x-attributes.cpp
+++ clang/test/Parser/cxx0x-attributes.cpp
@@ -152,6 +152,7 @@
   [[ab]ab] ns::i); // expected-error {{an attribute list cannot appear here}}
   do {} while ( // expected-note {{to match this '('}}
   alignas(4 ns::i; // expected-note {{to match this '('}}
+   // expected-error@-1 {{expected ';' after do/while}}
 } // expected-error 2{{expected ')'}} expected-error {{expected expression}}
 
 [[]] using T = int; // expected-error {{an attribute list cannot appear here}}
Index: clang/test/AST/loop-recovery.cpp
===
--- /dev/null
+++ clang/test/AST/loop-recovery.cpp
@@ -0,0 +1,45 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: not %clang_cc1 -fsyntax-only -ast-dump %s | FileCheck %s
+
+void test() {
+  while(!!!) // expected-error {{expected expression}}
+int whileBody;
+  // CHECK: WhileStmt
+  // CHECK: RecoveryExpr {{.*}}  'bool'
+  // CHECK: whileBody 'int'
+
+  for(!!!) // expected-error {{expected expression}} expected-error {{expected ';'}}
+int forBody;
+  // CHECK: ForStmt
+  // FIXME: the AST should have a RecoveryExpr to distinguish from for(;;)
+  // CHECK-NOT: RecoveryExpr
+  // CHECK: forBody 'int'
+
+  for(auto c : !!!) // expected-error {{expected expression}}
+int forEachBody;
+  // FIXME: parse the foreach body
+  // CHECK-NOT: CXXForRangeStmt
+  // CHECK-NOT: forEachBody 'int'
+
+  do
+int doBody;
+  while(!!!); // expected-error {{expected expression}}
+  // CHECK: DoStmt
+  // CHECK: doBody 'int'
+  // CHECK: RecoveryExpr {{.*}}  'bool'
+
+  if(!!!) // expected-error {{expected expression}}
+int ifBody;
+  else
+int elseBody;
+  // CHECK: IfStmt
+  // CHECK: RecoveryExpr {{.*}}  'bool'
+  // CHECK: ifBody 'int'
+  // CHECK: elseBody 'int'
+
+  switch(!!!) // expected-error {{expected expression}}
+int switchBody;
+  // CHECK: SwitchStmt
+  // CHECK: RecoveryExpr {{.*}}  'int'
+  // CHECK: switchBody 'int'
+}
Index: clang/test/AST/ast-dump-invalid.cpp
===
--- clang/test/AST/ast-dump-invalid.cpp
+++ clang/test/AST/ast-dump-invalid.cpp
@@ -33,7 +33,7 @@
 // CHECK-NEXT:   |-ParmVarDecl
 // CHECK-NEXT:   `-CompoundStmt
 // CHECK-NEXT: `-IfStmt {{.*}} 
-// CHECK-NEXT:   |-OpaqueValueExpr {{.*}} <> 'bool'
+// CHECK-NEXT:   |-RecoveryExpr {{.*}}  'bool'
 // CHECK-NEXT:   |-ReturnStmt {{.*}} 
 // CHECK-NEXT:   | `-IntegerLiteral {{.*}}  'int' 4
 // CHECK-NEXT:   `-ReturnStmt {{.*}} 
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++

[PATCH] D103317: [Analyzer][engine][solver] Simplify complex constraints

2021-11-12 Thread Gabor Marton via Phabricator via cfe-commits

martong added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:1155-1156
 
-// TODO: Support SymbolCast. Support IntSymExpr when/if we actually
-// start producing them.
 

ASDenysPetrov wrote:
> ASDenysPetrov wrote:
> > vsavchenko wrote:
> > > ASDenysPetrov wrote:
> > > > vsavchenko wrote:
> > > > > Do we actually produce them?
> > > > You can met SymbolCast here if `evalCast` produced it.
> > > > See `SValBuilder::evalCastSubKind(nonloc::SymbolVal...)`:
> > > > ```
> > > > if (!Loc::isLocType(CastTy))
> > > >   if (!IsUnknownOriginalType || !CastTy->isFloatingType() ||
> > > >   T->isFloatingType())
> > > > return makeNonLoc(SE, T, CastTy);
> > > > ```
> > > > Also my patch will produce a lot of integral cast symbols D103096. You 
> > > > are welcome to examine it.
> > > In my comment I meant `IntSymExpr`
> > Oh, anyway :-)
> I think we should remove **ConcreteInt** and and as a consequence 
> **IntSymExpr**, **SymIntExpr** in the nearest future. SymbolVal should be 
> reworked to store a QualType, a RangeSet and an expression pointer itself. 
> IMO SymSymExpr and SymbolVal is sufficient to make any calculations. It may 
> help generalize a lot of them. **ConcreteInt** is just a special case for 
> SymbolVal. More than that **PointerToMember** can be removed as well, because 
> we would get an info being a pointer to member from QualType in this way.
> Do we actually produce them?

Yes we produce `IntSymExpr`s.



Comment at: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:1170-1172
+  SVal LHS = getConst(S->getLHS());
+  if (LHS.isUndef())
+LHS = Visit(S->getLHS());

vsavchenko wrote:
> It seems like a very common pattern now: 
> ```
> SVal Const = getConst(Expr);
> if (Const.isUndef())
>   return Visit(Expr);
> return Const;
> ```
> 
> At least we can make a function out of this, so we don't repeat this pattern 
> over and over.
> But I was thinking more radically to actually replace `Visit` with this, what 
> do you think?
Good idea!



Comment at: clang/test/Analysis/simplify-complex-constraints.cpp:25-27
+  if (x + (y + z) != 0)
+return 0;
+  if (y + z != 0)

vsavchenko wrote:
> Since it's not really specific to addition, maybe we can have tests for other 
> operators?
Ok, I've added a new test, and modified a bit this test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103317

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

1 2 >

1 - 100 of 123 matches

Mail list logo