[PATCH] D126694: [C++20][Modules] Implementation of GMF decl elision.

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a comment.

> Yes, that was the decision at the last time we looked - because removing 
> decls would degrade this - if we have new information that changes our 
> preferred design, then fine.

I remember the major reason for the last time to not remove the decls are that 
the design of AST doesn't support to remove decls. And my current idea is, we 
can refuse to write the discardable Decls into the BMIs.

> One solution is to place the elision behind a flag so that the user can 
> choose slower compilation with better diagnostics or faster compilation but 
> maybe harder-to-find errors?

I proposed to add a flag. But that was a helper for developers to find if we 
did anything wrong. I don't want to provide such a flag since on the one hand, 
there are already many flags  and on the other hand, form my user experience, 
it is not so hard to find the unexported decls. It is really much much more 
easier than template programming.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126694

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

[PATCH] D154368: [Clang] Fix constraint checking of non-generic lambdas.

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin updated this revision to Diff 536976.
cor3ntin added a comment.

- Move the check for lambda conversion operator in CheckFunmctionConstraints so 
that it is always performed.

- Add tests of constraints referring to parameters of the lambda


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154368

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaConcept.cpp
  clang/lib/Sema/SemaLambda.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/test/SemaTemplate/concepts.cpp

Index: clang/test/SemaTemplate/concepts.cpp
===
--- clang/test/SemaTemplate/concepts.cpp
+++ clang/test/SemaTemplate/concepts.cpp
@@ -410,8 +410,8 @@
 
 template 
 void SingleDepthReferencesTopLambda(U &&u) {
-  []()
-requires IsInt
+  []() // #SDRTL_OP
+requires IsInt // #SDRTL_REQ
   {}();
 }
 
@@ -434,8 +434,8 @@
 
 template 
 void DoubleDepthReferencesTopLambda(U &&u) {
-  []() { []()
-   requires IsInt
+  []() { []() // #DDRTL_OP
+   requires IsInt // #DDRTL_REQ
  {}(); }();
 }
 
@@ -459,10 +459,11 @@
 
 template 
 void DoubleDepthReferencesAllLambda(U &&u) {
-  [](U &&u2) {
-[](U && u3)
-  requires IsInt &&
-   IsInt && IsInt
+  [](U &&u2) { // #DDRAL_OP1
+[](U && u3) // #DDRAL_OP2
+  requires IsInt // #DDRAL_REQ
+&& IsInt
+&& IsInt
 {}(u2);
   }(u);
 }
@@ -484,8 +485,8 @@
 template 
 void ChecksLocalVar(T x) {
   T Local;
-  []()
-requires(IsInt)
+  []() // #CLV_OP
+requires(IsInt) // #CLV_REQ
   {}();
 }
 
@@ -529,6 +530,11 @@
   SingleDepthReferencesTopLambda(v);
   // FIXME: This should error on constraint failure! (Lambda!)
   SingleDepthReferencesTopLambda(will_fail);
+  // expected-note@-1{{in instantiation of function template specialization}}
+  // expected-error@#SDRTL_OP{{no matching function for call to object of type}}
+  // expected-note@#SDRTL_OP{{candidate function not viable: constraints not satisfied}}
+  // expected-note@#SDRTL_REQ{{because 'IsInt' evaluated to false}}
+
   DoubleDepthReferencesTop(v);
   DoubleDepthReferencesTop(will_fail);
   // expected-error@#DDRT_CALL{{no matching function for call to object of type 'lc2'}}
@@ -538,8 +544,12 @@
   // expected-note@#DDRT_REQ{{'IsInt' evaluated to false}}
 
   DoubleDepthReferencesTopLambda(v);
-  // FIXME: This should error on constraint failure! (Lambda!)
   DoubleDepthReferencesTopLambda(will_fail);
+  // expected-note@-1{{in instantiation of function template specialization}}
+  // expected-error@#DDRTL_OP{{no matching function for call to object of type}}
+  // expected-note@#DDRTL_OP{{candidate function not viable: constraints not satisfied}}
+  // expected-note@#DDRTL_OP{{while substituting into a lambda expression here}}
+  // expected-note@#DDRTL_REQ{{because 'IsInt' evaluated to false}}
   DoubleDepthReferencesAll(v);
   DoubleDepthReferencesAll(will_fail);
   // expected-error@#DDRA_CALL{{no matching function for call to object of type 'lc2'}}
@@ -549,8 +559,12 @@
   // expected-note@#DDRA_REQ{{'IsInt' evaluated to false}}
 
   DoubleDepthReferencesAllLambda(v);
-  // FIXME: This should error on constraint failure! (Lambda!)
   DoubleDepthReferencesAllLambda(will_fail);
+  // expected-note@-1{{in instantiation of function template specialization}}
+  // expected-note@#DDRAL_OP1{{while substituting into a lambda expression here}}
+  // expected-error@#DDRAL_OP2{{no matching function for call to object of type}}
+  // expected-note@#DDRAL_OP2{{candidate function not viable: constraints not satisfied}}
+  // expected-note@#DDRAL_REQ{{because 'IsInt' evaluated to false}}
 
   CausesFriendConstraint CFC;
   FriendFunc(CFC, 1);
@@ -565,6 +579,12 @@
   ChecksLocalVar(v);
   // FIXME: This should error on constraint failure! (Lambda!)
   ChecksLocalVar(will_fail);
+  // expected-note@-1{{in instantiation of function template specialization}}
+  // expected-error@#CLV_OP{{no matching function for call to object of type}}
+  // expected-note@#CLV_OP{{candidate function not viable: constraints not satisfied}}
+  // expected-note@#CLV_REQ{{because 'IsInt' evaluated to false}}
+
+
 
   LocalStructMemberVar(v);
   LocalStructMemberVar(will_fail);
@@ -701,6 +721,18 @@
 } // namespace SelfFriend
 
 
+namespace Surrogates {
+int f1(int);
+template 
+struct A {
+using F = int(int);
+operator F*() requires N { return f1; } // expected-note{{candidate surrogate function 'operator int (*)(int)' not viable: constraints not satisfied}}
+};
+int i = A{}(0);
+int j = A{}(0); // expected-error{{no matching function for call to object of type 'A'}}
+}
+
+
 namespace ConstrainedMemberVarTemplate {
 template  struct Container {
   static constexpr long arity = Size;
@@ -914,3 +946,53 @@
 
 static_assert(W0<0>::W1<1>::F::value == 1);
 } // TemplateInsideTemplateInsideTemplate

[PATCH] D154357: [Driver] Recognize powerpc-unknown-eabi as a bare-metal toolchain

[Christian Walther via Phabricator via cfe-commits]

cwalther added a comment.

Should I be worried about the build and test failures? At a glance, they look 
unrelated to my changes, so unless told otherwise I’m going to ignore them.




Comment at: clang/lib/Driver/ToolChains/BareMetal.cpp:168
+
+  if (Triple.getVendor() != llvm::Triple::UnknownVendor)
+return false;

jroelofs wrote:
> does this break your use cases re: having `indel` as the vendor? or will you 
> have a downstream patch to exempt `indel` from this check?
No, `indel` is recognized as `Triple::UnknownVendor`, so this works for us. It 
would indeed stop working if `Indel` were added to `enum VendorType` (and the 
triple parsing functions), but I don’t expect that to happen.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154357

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

[PATCH] D126694: [C++20][Modules] Implementation of GMF decl elision.

[Iain Sandoe via Phabricator via cfe-commits]

iains added a comment.

In D126694#4470297 , @ChuanqiXu wrote:

>> Yes, that was the decision at the last time we looked - because removing 
>> decls would degrade this - if we have new information that changes our 
>> preferred design, then fine.
>
> I remember the major reason for the last time to not remove the decls are 
> that the design of AST doesn't support to remove decls. And my current idea 
> is, we can refuse to write the discardable Decls into the BMIs.

@rsmith pointed out that the API was not intended for that purpose - so, yes, 
you are correct that the next place to look was in the serialisation [but ISTR 
that this also has some challenges because of the way in which the structures 
are interconnected].  It might be necessary to do a pass before the 
serialisation and actually prune the AST there. (in a similar manner we'd need 
to prune it to remove non-inline function bodies in some future version)

>> One solution is to place the elision behind a flag so that the user can 
>> choose slower compilation with better diagnostics or faster compilation but 
>> maybe harder-to-find errors?
>
> I proposed to add a flag. But that was a helper for developers to find if we 
> did anything wrong. I don't want to provide such a flag since on the one 
> hand, there are already many flags  and on the other hand, form my user 
> experience, it is not so hard to find the unexported decls. It is really much 
> much more easier than template programming.

Yeah, (as you know) I definitely prefer not to add more flags - there are too 
many - it was only an option in case there were many people against degrading 
diagnostic output.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126694

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

[PATCH] D126694: [C++20][Modules] Implementation of GMF decl elision.

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a comment.

In D126694#4470358 , @iains wrote:

> In D126694#4470297 , @ChuanqiXu 
> wrote:
>
>>> Yes, that was the decision at the last time we looked - because removing 
>>> decls would degrade this - if we have new information that changes our 
>>> preferred design, then fine.
>>
>> I remember the major reason for the last time to not remove the decls are 
>> that the design of AST doesn't support to remove decls. And my current idea 
>> is, we can refuse to write the discardable Decls into the BMIs.
>
> @rsmith pointed out that the API was not intended for that purpose - so, yes, 
> you are correct that the next place to look was in the serialisation [but 
> ISTR that this also has some challenges because of the way in which the 
> structures are interconnected].  It might be necessary to do a pass before 
> the serialisation and actually prune the AST there. (in a similar manner we'd 
> need to prune it to remove non-inline function bodies in some future version)

Yeah, I just feel it is implementable. But I need to give it a try.

>>> One solution is to place the elision behind a flag so that the user can 
>>> choose slower compilation with better diagnostics or faster compilation but 
>>> maybe harder-to-find errors?
>>
>> I proposed to add a flag. But that was a helper for developers to find if we 
>> did anything wrong. I don't want to provide such a flag since on the one 
>> hand, there are already many flags  and on the other hand, form my user 
>> experience, it is not so hard to find the unexported decls. It is really 
>> much much more easier than template programming.
>
> Yeah, (as you know) I definitely prefer not to add more flags - there are too 
> many - it was only an option in case there were many people against degrading 
> diagnostic output.

Got it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126694

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

[PATCH] D153296: [AST] Stop evaluate constant expression if the condition expression which in switch statement contains errors

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

Thanks for the fix. Sorry for being late (I was looking through the emails and 
found this patch).




Comment at: clang/lib/AST/ExprConstant.cpp:4921
+  // value is.
+  if (isa(E))
+return false;

The constant evaluator is not aware of the "error" concept, it is only aware of 
value-dependent -- the general idea behind is that we treat the 
dependent-on-error and dependent-on-template-parameters cases the same, they 
are potentially constant (if we see an expression contains errors, it could be 
constant depending on how the error is resolved), this will give us nice 
recovery and avoid bogus following diagnostics.

So, a `RecoveryExpr` should not result in failure when checking for a potential 
constant expression.

I think the right fix is to remove the conditional check `if 
(!EvaluateDependentExpr(SS->getCond(), Info))` in `EvaluateSwitch`, and return 
`ESR_Failed` unconditionally (we don't know its value, any switch-case anwser 
will be wrong in some cases). We already do this for return-statment, 
do-statement etc.





Comment at: clang/test/SemaCXX/constexpr-function-recovery-crash.cpp:91
+
+static_assert(test13(), "should not crash"); // expected-error {{static 
assertion expression is not an integral constant expression}}
+

nit: we can simplify it with the `TEST_EVALUATE` macro:

```
TEST_EVALUATE(SwitchErrorCond, switch(undef) { case 0: return 7; default: 
break;})
```



Comment at: clang/test/SemaCXX/constexpr-function-recovery-crash.cpp:93
+
+constexpr int test14() {
+int sum = 0;

Is this a new crash (and the tests below)?

They don't look like new crashes, I think the current constant evaluator should 
be able to handle them well. IIUC the only crash we have is the case where we 
have a error-dependent condition in `switch`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153296

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

[PATCH] D154417: [libc++] Disable tree invariant check in asserts mode

[Hans Wennborg via Phabricator via cfe-commits]

hans created this revision.
hans added reviewers: var-const, ldionne.
Herald added a project: All.
hans requested review of this revision.
Herald added a project: libc++.
Herald added a subscriber: libcxx-commits.
Herald added a reviewer: libc++.

This is a follow-up to D153672  which removed 
the old debug mode and moved many of those checks to the regular asserts mode.

The tree invariant check is too expensive for the regular asserts mode, making 
element removal `O(n)` instead of `O(log n)`, so disable it until there is a 
new debug assert category it can be put in.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D154417

Files:
  libcxx/include/__tree


Index: libcxx/include/__tree
===
--- libcxx/include/__tree
+++ libcxx/include/__tree
@@ -376,7 +376,8 @@
 {
 _LIBCPP_ASSERT_UNCATEGORIZED(__root != nullptr, "Root node should not be 
null");
 _LIBCPP_ASSERT_UNCATEGORIZED(__z != nullptr, "The node to remove should 
not be null");
-_LIBCPP_ASSERT_UNCATEGORIZED(std::__tree_invariant(__root), "The tree 
invariants should hold");
+// TODO: Use in the new debug mode:
+// _LIBCPP_DEBUG_ASSERT(std::__tree_invariant(__root), "The tree 
invariants should hold");
 // __z will be removed from the tree.  Client still needs to 
destruct/deallocate it
 // __y is either __z, or if __z has two children, __tree_next(__z).
 // __y will have at most one child.


Index: libcxx/include/__tree
===
--- libcxx/include/__tree
+++ libcxx/include/__tree
@@ -376,7 +376,8 @@
 {
 _LIBCPP_ASSERT_UNCATEGORIZED(__root != nullptr, "Root node should not be null");
 _LIBCPP_ASSERT_UNCATEGORIZED(__z != nullptr, "The node to remove should not be null");
-_LIBCPP_ASSERT_UNCATEGORIZED(std::__tree_invariant(__root), "The tree invariants should hold");
+// TODO: Use in the new debug mode:
+// _LIBCPP_DEBUG_ASSERT(std::__tree_invariant(__root), "The tree invariants should hold");
 // __z will be removed from the tree.  Client still needs to destruct/deallocate it
 // __y is either __z, or if __z has two children, __tree_next(__z).
 // __y will have at most one child.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D154420: [clang][dataflow] Model variables / fields / funcs used in default initializers.

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

mboehme created this revision.
Herald added subscribers: martong, xazax.hun.
Herald added a reviewer: NoQ.
Herald added a project: All.
mboehme requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The newly added test fails without the other changes in this patch.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D154420

Files:
  clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
  clang/unittests/Analysis/FlowSensitive/TransferTest.cpp


Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -5547,4 +5547,40 @@
   });
 }
 
+TEST(TransferTest, AnonymousStructWithReferenceField) {
+  std::string Code = R"(
+int global_i = 0;
+struct target {
+  target() {
+(void)0;
+// [[p]]
+  }
+  struct {
+int &i = global_i;
+  };
+};
+  )";
+  runDataflow(
+  Code,
+  [](const llvm::StringMap> &Results,
+ ASTContext &ASTCtx) {
+const Environment &Env = getEnvironmentAtAnnotation(Results, "p");
+const ValueDecl *GlobalIDecl = findValueDecl(ASTCtx, "global_i");
+const ValueDecl *IDecl = findValueDecl(ASTCtx, "i");
+const IndirectFieldDecl *IndirectField =
+findIndirectFieldDecl(ASTCtx, "i");
+
+auto *ThisLoc =
+
cast(Env.getThisPointeeStorageLocation());
+auto &AnonStruct = cast(ThisLoc->getChild(
+*cast(IndirectField->chain().front(;
+
+auto *RefVal =
+cast(Env.getValue(AnonStruct.getChild(*IDecl)));
+
+ASSERT_EQ(&RefVal->getReferentLoc(),
+  Env.getStorageLocation(*GlobalIDecl));
+  });
+}
+
 } // namespace
Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
===
--- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -194,6 +194,8 @@
   for (auto *Child : S.children())
 if (Child != nullptr)
   getFieldsGlobalsAndFuncs(*Child, Fields, Vars, Funcs);
+  if (const auto *DefaultInit = dyn_cast(&S))
+getFieldsGlobalsAndFuncs(*DefaultInit->getExpr(), Fields, Vars, Funcs);
 
   if (auto *DS = dyn_cast(&S)) {
 if (DS->isSingleDecl())


Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -5547,4 +5547,40 @@
   });
 }
 
+TEST(TransferTest, AnonymousStructWithReferenceField) {
+  std::string Code = R"(
+int global_i = 0;
+struct target {
+  target() {
+(void)0;
+// [[p]]
+  }
+  struct {
+int &i = global_i;
+  };
+};
+  )";
+  runDataflow(
+  Code,
+  [](const llvm::StringMap> &Results,
+ ASTContext &ASTCtx) {
+const Environment &Env = getEnvironmentAtAnnotation(Results, "p");
+const ValueDecl *GlobalIDecl = findValueDecl(ASTCtx, "global_i");
+const ValueDecl *IDecl = findValueDecl(ASTCtx, "i");
+const IndirectFieldDecl *IndirectField =
+findIndirectFieldDecl(ASTCtx, "i");
+
+auto *ThisLoc =
+cast(Env.getThisPointeeStorageLocation());
+auto &AnonStruct = cast(ThisLoc->getChild(
+*cast(IndirectField->chain().front(;
+
+auto *RefVal =
+cast(Env.getValue(AnonStruct.getChild(*IDecl)));
+
+ASSERT_EQ(&RefVal->getReferentLoc(),
+  Env.getStorageLocation(*GlobalIDecl));
+  });
+}
+
 } // namespace
Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
===
--- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -194,6 +194,8 @@
   for (auto *Child : S.children())
 if (Child != nullptr)
   getFieldsGlobalsAndFuncs(*Child, Fields, Vars, Funcs);
+  if (const auto *DefaultInit = dyn_cast(&S))
+getFieldsGlobalsAndFuncs(*DefaultInit->getExpr(), Fields, Vars, Funcs);
 
   if (auto *DS = dyn_cast(&S)) {
 if (DS->isSingleDecl())
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D154421: [clang][dataflow] Add a test for a struct that is directly self-referential through a reference.

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

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

The ongoing migration to strict handling of value
categories (see https://discourse.llvm.org/t/70086) will change the way we
handle fields of reference type, and I want to put a test in place that makes
sure we continue to handle this special case correctly.

Depends On D154420 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D154421

Files:
  clang/unittests/Analysis/FlowSensitive/TransferTest.cpp


Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -656,6 +656,28 @@
   });
 }
 
+TEST(TransferTest, DirectlySelfReferentialReference) {
+  std::string Code = R"(
+struct target {
+  target() {
+(void)0;
+// [[p]]
+  }
+  target &self = *this;
+};
+  )";
+  runDataflow(
+  Code,
+  [](const llvm::StringMap> &Results,
+ ASTContext &ASTCtx) {
+const Environment &Env = getEnvironmentAtAnnotation(Results, "p");
+const ValueDecl *SelfDecl = findValueDecl(ASTCtx, "self");
+
+auto *ThisLoc = Env.getThisPointeeStorageLocation();
+ASSERT_EQ(&ThisLoc->getChild(*SelfDecl), ThisLoc);
+  });
+}
+
 TEST(TransferTest, MultipleVarsDecl) {
   std::string Code = R"(
 void target() {


Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -656,6 +656,28 @@
   });
 }
 
+TEST(TransferTest, DirectlySelfReferentialReference) {
+  std::string Code = R"(
+struct target {
+  target() {
+(void)0;
+// [[p]]
+  }
+  target &self = *this;
+};
+  )";
+  runDataflow(
+  Code,
+  [](const llvm::StringMap> &Results,
+ ASTContext &ASTCtx) {
+const Environment &Env = getEnvironmentAtAnnotation(Results, "p");
+const ValueDecl *SelfDecl = findValueDecl(ASTCtx, "self");
+
+auto *ThisLoc = Env.getThisPointeeStorageLocation();
+ASSERT_EQ(&ThisLoc->getChild(*SelfDecl), ThisLoc);
+  });
+}
+
 TEST(TransferTest, MultipleVarsDecl) {
   std::string Code = R"(
 void target() {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D151696: [x86] Remove CPU_SPECIFIC* MACROs and add getCPUDispatchMangling

[Simon Pilgrim via Phabricator via cfe-commits]

RKSimon added inline comments.



Comment at: clang/test/CodeGen/attr-cpuspecific-cpus.c:40
 ATTR(cpu_specific(knm)) void CPU(void){}
+ATTR(cpu_specific(cascadelake)) void CPU(void){}
+ATTR(cpu_specific(cooperlake)) void CPU(void){}

FreddyYe wrote:
> In this patch, I additionally supported some intel new CPU's _cpu_specific 
> feature by creating a new mangling or copy some old ones (which means 
> aliasing certain cpu). Maybe I should do this in a following patch?
Yes please, that would great.



Comment at: llvm/lib/TargetParser/X86TargetParser.cpp:378
+  { {"core_3rd_gen_avx"}, CK_IvyBridge, FEATURE_AVX, FeaturesIvyBridge, 'S', 
true },
+  { {"core-avx-i"}, CK_IvyBridge, FEATURE_AVX, FeaturesIvyBridge, '\0', false 
},
   // Haswell microarchitecture based processors.

FreddyYe wrote:
> RKSimon wrote:
> > I'm still not clear on what determines the mangling mode and cpu dispatch 
> > flag for cpu targets are supposedly the same? For example, none of these 
> > ivybridge equivalent configs have the same values.
> I assign them by following orders:
> 1. Copy the mangling from the original CPU_SPEICIFC MACRO.
> 2. If there's no way to copy, assign to '\0' by default, which means doesn't 
> support __cpu_specific/dispatch feature.
> 3. If cpu name contain ''-', assign the mangling as '\0', too. Because '-' 
> cannot be correctly identified in _cpu_specific/dispatch().
> 4. set OnlyForCPUDispatch flag as `true` if this cpu name was not listed 
> here, which means it doesn't support -march, -mtune and so on. This flag 
> makes this cpu name only support __cpu_dispatch/specific feature. E.g. 
> core_3rd_gen_avx, core_4rd_gen_avx., ... And normally, these names are very 
> old. So supporting them with -march=, -mtune= is not easy for now. And notice 
> that new cpu names shouldn't set this flag as `true` since they should both 
> support -march= and __cpu_specific/dispatch feature by default.
> 
OK - please can you add that to a comment above line 311 for future reference?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151696

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

[PATCH] D153340: [include-cleaner] Add an IgnoreHeaders flag to the command-line tool.

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 536998.
hokein marked 10 inline comments as done.
hokein added a comment.

address review comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153340

Files:
  clang-tools-extra/clangd/IncludeCleaner.cpp
  clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
  clang-tools-extra/include-cleaner/include/clang-include-cleaner/Types.h
  clang-tools-extra/include-cleaner/lib/Analysis.cpp
  clang-tools-extra/include-cleaner/lib/Types.cpp
  clang-tools-extra/include-cleaner/test/tool.cpp
  clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp

Index: clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp
===
--- clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp
+++ clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp
@@ -14,10 +14,19 @@
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Tooling/CommonOptionsParser.h"
 #include "clang/Tooling/Tooling.h"
+#include "llvm/ADT/STLFunctionalExtras.h"
+#include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/CommandLine.h"
+#include "llvm/Support/FormatVariadic.h"
+#include "llvm/Support/Regex.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/raw_ostream.h"
+#include 
+#include 
+#include 
+#include 
+#include 
 
 namespace clang {
 namespace include_cleaner {
@@ -47,6 +56,14 @@
 cl::cat(IncludeCleaner),
 };
 
+cl::opt IgnoreHeaders{
+"ignore-headers",
+cl::desc("A comma-separated list of regexes to match against suffix of a "
+ "header, and disable analysis if matched."),
+cl::init(""),
+cl::cat(IncludeCleaner),
+};
+
 enum class PrintStyle { Changes, Final };
 cl::opt Print{
 "print",
@@ -91,9 +108,15 @@
 }
 
 class Action : public clang::ASTFrontendAction {
+public:
+  Action(llvm::function_ref HeaderFilter)
+  : HeaderFilter(HeaderFilter){};
+
+private:
   RecordedAST AST;
   RecordedPP PP;
   PragmaIncludes PI;
+  llvm::function_ref HeaderFilter;
 
   void ExecuteAction() override {
 auto &P = getCompilerInstance().getPreprocessor();
@@ -126,8 +149,8 @@
 assert(!Path.empty() && "Main file path not known?");
 llvm::StringRef Code = SM.getBufferData(SM.getMainFileID());
 
-auto Results =
-analyze(AST.Roots, PP.MacroReferences, PP.Includes, &PI, SM, HS);
+auto Results = analyze(AST.Roots, PP.MacroReferences, PP.Includes, &PI, SM,
+   HS, HeaderFilter);
 if (!Insert)
   Results.Missing.clear();
 if (!Remove)
@@ -176,6 +199,44 @@
 getCompilerInstance().getPreprocessor().getHeaderSearchInfo(), &PI, OS);
   }
 };
+class ActionFactory : public tooling::FrontendActionFactory {
+public:
+  ActionFactory(llvm::function_ref HeaderFilter)
+  : HeaderFilter(HeaderFilter) {}
+
+  std::unique_ptr create() override {
+return std::make_unique(HeaderFilter);
+  }
+
+private:
+  llvm::function_ref HeaderFilter;
+};
+
+std::function headerFilter() {
+  auto FilterRegs = std::make_shared>();
+
+  llvm::SmallVector Headers;
+  llvm::StringRef(IgnoreHeaders).split(Headers, ',', -1, /*KeepEmpty=*/false);
+  for (auto HeaderPattern : Headers) {
+std::string AnchoredPattern = "(" + HeaderPattern.str() + ")$";
+llvm::Regex CompiledRegex(AnchoredPattern);
+std::string RegexError;
+if (!CompiledRegex.isValid(RegexError)) {
+  llvm::errs() << llvm::formatv("Invalid regular expression '{0}': {1}\n",
+HeaderPattern, RegexError);
+  return nullptr;
+}
+FilterRegs->push_back(std::move(CompiledRegex));
+  }
+  return [FilterRegs](llvm::StringRef Path) {
+llvm::errs() << "Path: " << Path << "\n";
+for (const auto &F : *FilterRegs) {
+  if (F.match(Path))
+return true;
+}
+return false;
+  };
+}
 
 } // namespace
 } // namespace include_cleaner
@@ -191,19 +252,10 @@
 llvm::errs() << toString(OptionsParser.takeError());
 return 1;
   }
-
-  if (OptionsParser->getSourcePathList().size() != 1) {
-std::vector IncompatibleFlags = {&HTMLReportPath, &Print};
-for (const auto *Flag : IncompatibleFlags) {
-  if (Flag->getNumOccurrences()) {
-llvm::errs() << "-" << Flag->ArgStr << " requires a single input file";
-return 1;
-  }
-}
-  }
-  auto Factory = clang::tooling::newFrontendActionFactory();
+  auto HeaderFilter = headerFilter();
+  ActionFactory Factory(HeaderFilter);
   return clang::tooling::ClangTool(OptionsParser->getCompilations(),
OptionsParser->getSourcePathList())
- .run(Factory.get()) ||
+ .run(&Factory) ||
  Errors != 0;
 }
Index: clang-tools-extra/include-cleaner/test/tool.cpp
===
--- clang-tools-extra/include-cleaner/test/tool.cpp

[PATCH] D153340: [include-cleaner] Add an IgnoreHeaders flag to the command-line tool.

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



Comment at: clang-tools-extra/include-cleaner/test/tool.cpp:17
 
+//RUN: clang-include-cleaner -print=changes %s 
--ignore-headers="foobar\.h,foo\.h" -- -I%S/Inputs/ | FileCheck 
--match-full-lines --allow-empty --check-prefix=IGNORE %s
+// IGNORE-NOT: - "foobar.h"

kadircet wrote:
> can you ignore one but keep other?
> 
> it'd be useful to also test the regex behaviour
this tests aims to test filtering logic for both missing-includes and 
unused-includes cases.

added a new test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153340

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

[PATCH] D153612: [clang][analyzer] Add and change NoteTags in StdLibraryFunctionsChecker.

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

balazske updated this revision to Diff 537002.
balazske added a comment.

rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153612

Files:
  clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp
  clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.h
  clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
  clang/test/Analysis/errno-stdlibraryfunctions-notes.c
  clang/test/Analysis/std-c-library-functions-arg-constraints-note-tags.cpp
  clang/test/Analysis/std-c-library-functions-arg-constraints.c
  clang/test/Analysis/std-c-library-functions-path-notes.c
  clang/test/Analysis/stream-errno-note.c
  clang/test/Analysis/stream-note.c

Index: clang/test/Analysis/stream-note.c
===
--- clang/test/Analysis/stream-note.c
+++ clang/test/Analysis/stream-note.c
@@ -7,11 +7,13 @@
 
 void check_note_at_correct_open(void) {
   FILE *F1 = tmpfile(); // expected-note {{Stream opened here}}
+  // stdargs-note@-1 {{'tmpfile' is successful}}
   if (!F1)
 // expected-note@-1 {{'F1' is non-null}}
 // expected-note@-2 {{Taking false branch}}
 return;
   FILE *F2 = tmpfile();
+  // stdargs-note@-1 {{'tmpfile' is successful}}
   if (!F2) {
 // expected-note@-1 {{'F2' is non-null}}
 // expected-note@-2 {{Taking false branch}}
@@ -20,6 +22,7 @@
   }
   rewind(F2);
   fclose(F2);
+  // stdargs-note@-1 {{'fclose' fails}}
   rewind(F1);
 }
 // expected-warning@-1 {{Opened stream never closed. Potential resource leak}}
@@ -27,6 +30,7 @@
 
 void check_note_fopen(void) {
   FILE *F = fopen("file", "r"); // expected-note {{Stream opened here}}
+  // stdargs-note@-1 {{'fopen' is successful}}
   if (!F)
 // expected-note@-1 {{'F' is non-null}}
 // expected-note@-2 {{Taking false branch}}
@@ -37,11 +41,13 @@
 
 void check_note_freopen(void) {
   FILE *F = fopen("file", "r"); // expected-note {{Stream opened here}}
+  // stdargs-note@-1 {{'fopen' is successful}}
   if (!F)
 // expected-note@-1 {{'F' is non-null}}
 // expected-note@-2 {{Taking false branch}}
 return;
   F = freopen(0, "w", F); // expected-note {{Stream reopened here}}
+  // stdargs-note@-1 {{'freopen' is successful}}
   if (!F)
 // expected-note@-1 {{'F' is non-null}}
 // expected-note@-2 {{Taking false branch}}
@@ -52,6 +58,8 @@
 
 void check_note_leak_2(int c) {
   FILE *F1 = fopen("foo1.c", "r"); // expected-note {{Stream opened here}}
+  // stdargs-note@-1 {{'fopen' is successful}}
+  // stdargs-note@-2 {{'fopen' is successful}}
   if (!F1)
 // expected-note@-1 {{'F1' is non-null}}
 // expected-note@-2 {{Taking false branch}}
@@ -59,6 +67,8 @@
 // expected-note@-4 {{Taking false branch}}
 return;
   FILE *F2 = fopen("foo2.c", "r"); // expected-note {{Stream opened here}}
+  // stdargs-note@-1 {{'fopen' is successful}}
+  // stdargs-note@-2 {{'fopen' is successful}}
   if (!F2) {
 // expected-note@-1 {{'F2' is non-null}}
 // expected-note@-2 {{Taking false branch}}
@@ -84,6 +94,7 @@
 void check_track_null(void) {
   FILE *F;
   F = fopen("foo1.c", "r"); // expected-note {{Value assigned to 'F'}} expected-note {{Assuming pointer value is null}}
+  // stdargs-note@-1 {{'fopen' fails}}
   if (F != NULL) {  // expected-note {{Taking false branch}} expected-note {{'F' is equal to NULL}}
 fclose(F);
 return;
@@ -96,13 +107,16 @@
   FILE *F;
   char Buf[10];
   F = fopen("foo1.c", "r");
+  // stdargs-note@-1 {{'fopen' is successful}}
   if (F == NULL) { // expected-note {{Taking false branch}} expected-note {{'F' is not equal to NULL}}
 return;
   }
   fread(Buf, 1, 1, F);
+  // stdargs-note@-1 {{'fread' fails}}
   if (feof(F)) { // expected-note {{Taking true branch}}
 clearerr(F);
 fread(Buf, 1, 1, F);   // expected-note {{Assuming stream reaches end-of-file here}}
+// stdargs-note@-1 {{'fread' fails}}
 if (feof(F)) { // expected-note {{Taking true branch}}
   fread(Buf, 1, 1, F); // expected-warning {{Read function called when stream is in EOF state. Function has no effect}}
   // expected-note@-1 {{Read function called when stream is in EOF state. Function has no effect}}
@@ -115,10 +129,12 @@
   FILE *F;
   char Buf[10];
   F = fopen("foo1.c", "r");
+  // stdargs-note@-1 {{'fopen' is successful}}
   if (F == NULL) { // expected-note {{Taking false branch}} expected-note {{'F' is not equal to NULL}}
 return;
   }
   fread(Buf, 1, 1, F);
+  // stdargs-note@-1 {{'fread' is successful}}
   if (feof(F)) { // expected-note {{Taking false branch}}
 fclose(F);
 return;
@@ -127,6 +143,7 @@
 return;
   }
   fread(Buf, 1, 1, F);   // expected-note {{Assuming stream reaches end-of-file here}}
+  // stdargs-note@-1 {{'fread' fails}}
   if (feof(F)) { // expected-note {{Taking true branch}}
 fread(Buf, 1, 1, F); // expected-warning {{Read function called when stream is in EOF state.

[PATCH] D153776: [clang][analyzer] Display notes in StdLibraryFunctionsChecker only if interesting

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

balazske updated this revision to Diff 537003.
balazske added a comment.

rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153776

Files:
  clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
  clang/test/Analysis/errno-stdlibraryfunctions-notes.c
  clang/test/Analysis/std-c-library-functions-arg-constraints-note-tags.cpp
  clang/test/Analysis/std-c-library-functions-arg-constraints.c
  clang/test/Analysis/std-c-library-functions-path-notes.c
  clang/test/Analysis/stream-errno-note.c
  clang/test/Analysis/stream-note.c

Index: clang/test/Analysis/stream-note.c
===
--- clang/test/Analysis/stream-note.c
+++ clang/test/Analysis/stream-note.c
@@ -13,7 +13,6 @@
 // expected-note@-2 {{Taking false branch}}
 return;
   FILE *F2 = tmpfile();
-  // stdargs-note@-1 {{'tmpfile' is successful}}
   if (!F2) {
 // expected-note@-1 {{'F2' is non-null}}
 // expected-note@-2 {{Taking false branch}}
@@ -22,7 +21,6 @@
   }
   rewind(F2);
   fclose(F2);
-  // stdargs-note@-1 {{'fclose' fails}}
   rewind(F1);
 }
 // expected-warning@-1 {{Opened stream never closed. Potential resource leak}}
@@ -59,7 +57,6 @@
 void check_note_leak_2(int c) {
   FILE *F1 = fopen("foo1.c", "r"); // expected-note {{Stream opened here}}
   // stdargs-note@-1 {{'fopen' is successful}}
-  // stdargs-note@-2 {{'fopen' is successful}}
   if (!F1)
 // expected-note@-1 {{'F1' is non-null}}
 // expected-note@-2 {{Taking false branch}}
@@ -68,7 +65,6 @@
 return;
   FILE *F2 = fopen("foo2.c", "r"); // expected-note {{Stream opened here}}
   // stdargs-note@-1 {{'fopen' is successful}}
-  // stdargs-note@-2 {{'fopen' is successful}}
   if (!F2) {
 // expected-note@-1 {{'F2' is non-null}}
 // expected-note@-2 {{Taking false branch}}
@@ -107,16 +103,13 @@
   FILE *F;
   char Buf[10];
   F = fopen("foo1.c", "r");
-  // stdargs-note@-1 {{'fopen' is successful}}
   if (F == NULL) { // expected-note {{Taking false branch}} expected-note {{'F' is not equal to NULL}}
 return;
   }
   fread(Buf, 1, 1, F);
-  // stdargs-note@-1 {{'fread' fails}}
   if (feof(F)) { // expected-note {{Taking true branch}}
 clearerr(F);
 fread(Buf, 1, 1, F);   // expected-note {{Assuming stream reaches end-of-file here}}
-// stdargs-note@-1 {{'fread' fails}}
 if (feof(F)) { // expected-note {{Taking true branch}}
   fread(Buf, 1, 1, F); // expected-warning {{Read function called when stream is in EOF state. Function has no effect}}
   // expected-note@-1 {{Read function called when stream is in EOF state. Function has no effect}}
@@ -129,12 +122,10 @@
   FILE *F;
   char Buf[10];
   F = fopen("foo1.c", "r");
-  // stdargs-note@-1 {{'fopen' is successful}}
   if (F == NULL) { // expected-note {{Taking false branch}} expected-note {{'F' is not equal to NULL}}
 return;
   }
   fread(Buf, 1, 1, F);
-  // stdargs-note@-1 {{'fread' is successful}}
   if (feof(F)) { // expected-note {{Taking false branch}}
 fclose(F);
 return;
@@ -143,7 +134,6 @@
 return;
   }
   fread(Buf, 1, 1, F);   // expected-note {{Assuming stream reaches end-of-file here}}
-  // stdargs-note@-1 {{'fread' fails}}
   if (feof(F)) { // expected-note {{Taking true branch}}
 fread(Buf, 1, 1, F); // expected-warning {{Read function called when stream is in EOF state. Function has no effect}}
 // expected-note@-1 {{Read function called when stream is in EOF state. Function has no effect}}
@@ -155,11 +145,9 @@
   FILE *F;
   char Buf[10];
   F = fopen("foo1.c", "r");
-  // stdargs-note@-1 {{'fopen' is successful}}
   if (F == NULL) // expected-note {{Taking false branch}} expected-note {{'F' is not equal to NULL}}
 return;
   int RRet = fread(Buf, 1, 1, F); // expected-note {{Assuming stream reaches end-of-file here}}
-  // stdargs-note@-1 {{'fread' fails}}
   if (ferror(F)) {// expected-note {{Taking false branch}}
   } else {
 fread(Buf, 1, 1, F); // expected-warning {{Read function called when stream is in EOF state. Function has no effect}}
Index: clang/test/Analysis/stream-errno-note.c
===
--- clang/test/Analysis/stream-errno-note.c
+++ clang/test/Analysis/stream-errno-note.c
@@ -11,7 +11,6 @@
 void check_fopen(void) {
   FILE *F = fopen("xxx", "r");
   // expected-note@-1{{'errno' may be undefined after successful call to 'fopen'}}
-  // expected-note@-2{{'fopen' is successful}}
   // expected-note@+2{{'F' is non-null}}
   // expected-note@+1{{Taking false branch}}
   if (!F)
@@ -24,7 +23,6 @@
 void check_tmpfile(void) {
   FILE *F = tmpfile();
   // expected-note@-1{{'errno' may be undefined after successful call to 'tmpfile'}}
-  // expected-note@-2{{'tmpfile' is successful}}
   // expected-note@+2{{'F' is non-null}}
   // expected-note@+1{{Taking false branch}}
   if

[PATCH] D154423: [clang][analyzer] Add all success/failure messages to StdLibraryFunctionsChecker.

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

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

Success or failure messages are now shown at all checked functions, if the call
(return value) is interesting.
Additionally new functions are added: open, openat, socket, shutdown


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D154423

Files:
  clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
  clang/test/Analysis/Inputs/std-c-library-functions-POSIX.h
  clang/test/Analysis/std-c-library-functions-POSIX.c

Index: clang/test/Analysis/std-c-library-functions-POSIX.c
===
--- clang/test/Analysis/std-c-library-functions-POSIX.c
+++ clang/test/Analysis/std-c-library-functions-POSIX.c
@@ -1,3 +1,12 @@
+// RUN: %clang_analyze_cc1 %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=alpha.unix.StdCLibraryFunctions \
+// RUN:   -analyzer-config alpha.unix.StdCLibraryFunctions:ModelPOSIX=true \
+// RUN:   -analyzer-config alpha.unix.StdCLibraryFunctions:DisplayLoadedSummaries=true \
+// RUN:   -analyzer-checker=debug.ExprInspection \
+// RUN:   -analyzer-config eagerly-assume=false \
+// RUN:   -triple i686-unknown-linux -verify
+
 // RUN: %clang_analyze_cc1 %s \
 // RUN:   -analyzer-checker=core \
 // RUN:   -analyzer-checker=alpha.unix.StdCLibraryFunctions \
@@ -15,6 +24,8 @@
 // CHECK: Loaded summary for: int fileno(FILE *stream)
 // CHECK: Loaded summary for: long a64l(const char *str64)
 // CHECK: Loaded summary for: char *l64a(long value)
+// CHECK: Loaded summary for: int open(const char *path, int oflag, ...)
+// CHECK: Loaded summary for: int openat(int fd, const char *path, int oflag, ...)
 // CHECK: Loaded summary for: int access(const char *pathname, int amode)
 // CHECK: Loaded summary for: int faccessat(int dirfd, const char *pathname, int mode, int flags)
 // CHECK: Loaded summary for: int dup(int fildes)
@@ -82,6 +93,7 @@
 // CHECK: Loaded summary for: int execv(const char *path, char *const argv[])
 // CHECK: Loaded summary for: int execvp(const char *file, char *const argv[])
 // CHECK: Loaded summary for: int getopt(int argc, char *const argv[], const char *optstring)
+// CHECK: Loaded summary for: int socket(int domain, int type, int protocol)
 // CHECK: Loaded summary for: int accept(int socket, __SOCKADDR_ARG address, socklen_t *restrict address_len)
 // CHECK: Loaded summary for: int bind(int socket, __CONST_SOCKADDR_ARG address, socklen_t address_len)
 // CHECK: Loaded summary for: int getpeername(int socket, __SOCKADDR_ARG address, socklen_t *restrict address_len)
@@ -97,6 +109,7 @@
 // CHECK: Loaded summary for: int getsockopt(int socket, int level, int option_name, void *restrict option_value, socklen_t *restrict option_len)
 // CHECK: Loaded summary for: ssize_t send(int sockfd, const void *buf, size_t len, int flags)
 // CHECK: Loaded summary for: int socketpair(int domain, int type, int protocol, int sv[2])
+// CHECK: Loaded summary for: int shutdown(int socket, int how)
 // CHECK: Loaded summary for: int getnameinfo(const struct sockaddr *restrict sa, socklen_t salen, char *restrict node, socklen_t nodelen, char *restrict service, socklen_t servicelen, int flags)
 // CHECK: Loaded summary for: int utime(const char *filename, struct utimbuf *buf)
 // CHECK: Loaded summary for: int futimens(int fd, const struct timespec times[2])
@@ -128,8 +141,12 @@
 
 #include "Inputs/std-c-library-functions-POSIX.h"
 
-// Must have at least one call expression to initialize the summary map.
-int bar(void);
-void foo(void) {
-  bar();
+void test_open(void) {
+  open(0, 0); // \
+  // expected-warning{{The 1st argument to 'open' is NULL but should not be NULL}}
+}
+
+void test_open_additional_arg(void) {
+  open(0, 0, 0); // \
+  // expected-warning{{The 1st argument to 'open' is NULL but should not be NULL}}
 }
Index: clang/test/Analysis/Inputs/std-c-library-functions-POSIX.h
===
--- clang/test/Analysis/Inputs/std-c-library-functions-POSIX.h
+++ clang/test/Analysis/Inputs/std-c-library-functions-POSIX.h
@@ -47,6 +47,8 @@
 int fileno(FILE *stream);
 long a64l(const char *str64);
 char *l64a(long value);
+int open(const char *path, int oflag, ...);
+int openat(int fd, const char *path, int oflag, ...);
 int access(const char *pathname, int amode);
 int faccessat(int dirfd, const char *pathname, int mode, int flags);
 int dup(int fildes);
@@ -135,6 +137,7 @@
 } __CONST_SOCKADDR_ARG __attribute__((__transparent_union__));
 #undef __SOCKADDR_ONETYPE
 
+int socket(int domain, int type, int protocol);
 int accept(int soc

[PATCH] D153493: [dataflow] avoid more accidental copies of Environment

[Sam McCall via Phabricator via cfe-commits]

sammccall closed this revision.
sammccall added a comment.
Herald added a subscriber: wangpc.

This relanded as 1e010c5c4fae43c52d6f5f1c8e8920c26bcc6cc7 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153493

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

[PATCH] D148216: Add support for annotations in UpdateTestChecks (NFC)

[Henrik G Olsson via Phabricator via cfe-commits]

hnrklssn added a comment.

@nikic Are you happy with the current patchset?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148216

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

[PATCH] D154420: [clang][dataflow] Model variables / fields / funcs used in default initializers.

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

mboehme added a comment.

pre-merge failure looks unrelated


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154420

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

[PATCH] D154325: [analyzer][NFC] Move away from using raw-for loops inside StaticAnalyzer

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun accepted this revision.
xazax.hun added inline comments.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h:501
   public:
+const iterator &operator*() const { return *this; }
+

Is this just a dummy to make sure this satisfies some concepts? I think I 
comment might be helpful in that case, people might find a noop dereference 
operator confusing. Same for some other places.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h:749-750
 
   referenced_vars_iterator referenced_vars_begin() const;
   referenced_vars_iterator referenced_vars_end() const;
+  llvm::iterator_range referenced_vars() const;

Do we need these with the addition of `referenced_vars`?



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h:637-638
 
   region_iterator region_begin() const { return LiveRegionRoots.begin(); }
   region_iterator region_end() const { return LiveRegionRoots.end(); }
+  llvm::iterator_range regions() const {

Should we remove these?



Comment at: clang/lib/StaticAnalyzer/Checkers/MallocSizeofChecker.cpp:194-195
 
-  for (CallExpr::const_arg_iterator ai = i->AllocCall->arg_begin(),
-   ae = i->AllocCall->arg_end(); ai != ae; ++ai) {
-if (!(*ai)->getType()->isIntegralOrUnscopedEnumerationType())
+  for (const Expr *Arg : make_range(CallRec.AllocCall->arg_begin(),
+CallRec.AllocCall->arg_end())) {
+if (!Arg->getType()->isIntegralOrUnscopedEnumerationType())





Comment at: clang/lib/StaticAnalyzer/Core/AnalysisManager.cpp:54
+  for (PathDiagnosticConsumer *Consumer : PathConsumers) {
+delete Consumer;
   }

Hm, I wonder whether we actually want to use `unique_ptr`s to make the 
ownership explicit. Feel free to leave as is in this PR.



Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp:173-177
+} else {
 // The left-hand side may bind to a different value then the
 // computation type.
 LHSVal = svalBuilder.evalCast(Result, LTy, CTy);
+}

Might be an issue with Phab, but indent looks strange to me here.



Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp:1190
   CXXRecordDecl::field_iterator CurField = LE->getLambdaClass()->field_begin();
-  for (LambdaExpr::const_capture_init_iterator i = LE->capture_init_begin(),
-   e = LE->capture_init_end();
-   i != e; ++i, ++CurField, ++Idx) {
+  for (auto i = LE->capture_init_begin(), e = LE->capture_init_end(); i != e;
+   ++i, ++CurField, ++Idx) {

Ha about using `capture_inits`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154325

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

[PATCH] D153366: [dataflow] Add dedicated representation of boolean formulas

[Sam McCall via Phabricator via cfe-commits]

sammccall marked 11 inline comments as done.
sammccall added inline comments.
Herald added a subscriber: wangpc.



Comment at: clang/include/clang/Analysis/FlowSensitive/Formula.h:69
+
+  Atom atom() const {
+assert(kind() == AtomRef);

gribozavr2 wrote:
> Should it be called getAsAtom() or castAsAtom() ? For uniformity with other 
> names in Clang and LLVM.
> 
> 
This isn't quite a cast - the distinction between a variable's identity and a 
reference to it is real.
renamed to `getAtom()`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153366

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

[clang] 2fd614e - [dataflow] Add dedicated representation of boolean formulas

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2023-07-04T12:19:44+02:00
New Revision: 2fd614efc1bb9c27f1bc6c3096c60a7fe121e274

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

LOG: [dataflow] Add dedicated representation of boolean formulas

This is the first step in untangling the two current jobs of BoolValue.

=== Desired end-state: ===

- BoolValue will model C++ booleans e.g. held in StorageLocations.
  this includes describing uncertainty (e.g. "top" is a Value concern)
- Formula describes analysis-level assertions in terms of SAT atoms.

These can still be linked together: a BoolValue may have a corresponding
SAT atom which is constrained by formulas.

=== Done in this patch: ===

BoolValue is left intact, Formula is just the input type to the
SAT solver, and we build formulas as needed to invoke the solver.

=== Incidental changes to debug string printing: ===

- variables renamed from B0 etc to V0 etc
  B0 collides with the names of basic blocks, which is confusing when
  debugging flow conditions.
- debug printing of formulas (Formula and Atom) uses operator<<
  rather than debugString(), so works with gtest.
  Therefore moved out of DebugSupport.h
- Did the same to Solver::Result, and some helper changes to SolverTest,
  so that we get useful messages on unit test failures
- formulas are now printed as infix expressions on one line, rather than
  wrapped/indented S-exprs. My experience is that this is easier to scan
  FCs for small examples, and large ones are unreadable either way.
- most of the several debugString() functions for constraints/results
  are unused, so removed them rather than updating tests.
  Inlined the one that was actually used into its callsite.

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

Added: 
clang/include/clang/Analysis/FlowSensitive/Formula.h
clang/lib/Analysis/FlowSensitive/Formula.cpp

Modified: 
clang/include/clang/Analysis/FlowSensitive/Arena.h
clang/include/clang/Analysis/FlowSensitive/DebugSupport.h
clang/include/clang/Analysis/FlowSensitive/Solver.h
clang/include/clang/Analysis/FlowSensitive/WatchedLiteralsSolver.h
clang/lib/Analysis/FlowSensitive/Arena.cpp
clang/lib/Analysis/FlowSensitive/CMakeLists.txt
clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
clang/lib/Analysis/FlowSensitive/DebugSupport.cpp
clang/lib/Analysis/FlowSensitive/WatchedLiteralsSolver.cpp
clang/unittests/Analysis/FlowSensitive/ArenaTest.cpp
clang/unittests/Analysis/FlowSensitive/DebugSupportTest.cpp
clang/unittests/Analysis/FlowSensitive/SolverTest.cpp
clang/unittests/Analysis/FlowSensitive/TestingSupport.h

Removed: 




diff  --git a/clang/include/clang/Analysis/FlowSensitive/Arena.h 
b/clang/include/clang/Analysis/FlowSensitive/Arena.h
index 83b4ddeec02564..b6c62e76246254 100644
--- a/clang/include/clang/Analysis/FlowSensitive/Arena.h
+++ b/clang/include/clang/Analysis/FlowSensitive/Arena.h
@@ -8,6 +8,7 @@
 #ifndef LLVM_CLANG_ANALYSIS_FLOWSENSITIVE__ARENA_H
 #define LLVM_CLANG_ANALYSIS_FLOWSENSITIVE__ARENA_H
 
+#include "clang/Analysis/FlowSensitive/Formula.h"
 #include "clang/Analysis/FlowSensitive/StorageLocation.h"
 #include "clang/Analysis/FlowSensitive/Value.h"
 #include 
@@ -104,7 +105,17 @@ class Arena {
 return create();
   }
 
+  /// Gets the boolean formula equivalent of a BoolValue.
+  /// Each unique Top values is translated to an Atom.
+  /// TODO: migrate to storing Formula directly in Values instead.
+  const Formula &getFormula(const BoolValue &);
+
+  /// Returns a new atomic boolean variable, distinct from any other.
+  Atom makeAtom() { return static_cast(NextAtom++); };
+
 private:
+  llvm::BumpPtrAllocator Alloc;
+
   // Storage for the state of a program.
   std::vector> Locs;
   std::vector> Vals;
@@ -122,6 +133,9 @@ class Arena {
   llvm::DenseMap, BiconditionalValue *>
   BiconditionalVals;
 
+  llvm::DenseMap ValToFormula;
+  unsigned NextAtom = 0;
+
   AtomicBoolValue &TrueVal;
   AtomicBoolValue &FalseVal;
 };

diff  --git a/clang/include/clang/Analysis/FlowSensitive/DebugSupport.h 
b/clang/include/clang/Analysis/FlowSensitive/DebugSupport.h
index 360786b02729f2..6b9f3681490af1 100644
--- a/clang/include/clang/Analysis/FlowSensitive/DebugSupport.h
+++ b/clang/include/clang/Analysis/FlowSensitive/DebugSupport.h
@@ -19,7 +19,6 @@
 
 #include "clang/Analysis/FlowSensitive/Solver.h"
 #include "clang/Analysis/FlowSensitive/Value.h"
-#include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/StringRef.h"
 
 namespace clang {
@@ -28,52 +27,9 @@ namespace dataflow {
 /// Returns a string representation of a value kind.
 llvm::StringRef debugString(Value::Kind Kind);
 
-/// Returns a string representation of a boolean assignment to true or false.
-llvm::StringRef debugString(Solver::

[PATCH] D153366: [dataflow] Add dedicated representation of boolean formulas

[Sam McCall via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
sammccall marked an inline comment as done.
Closed by commit rG2fd614efc1bb: [dataflow] Add dedicated representation of 
boolean formulas (authored by sammccall).

Changed prior to commit:
  https://reviews.llvm.org/D153366?vs=535491&id=537027#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153366

Files:
  clang/include/clang/Analysis/FlowSensitive/Arena.h
  clang/include/clang/Analysis/FlowSensitive/DebugSupport.h
  clang/include/clang/Analysis/FlowSensitive/Formula.h
  clang/include/clang/Analysis/FlowSensitive/Solver.h
  clang/include/clang/Analysis/FlowSensitive/WatchedLiteralsSolver.h
  clang/lib/Analysis/FlowSensitive/Arena.cpp
  clang/lib/Analysis/FlowSensitive/CMakeLists.txt
  clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
  clang/lib/Analysis/FlowSensitive/DebugSupport.cpp
  clang/lib/Analysis/FlowSensitive/Formula.cpp
  clang/lib/Analysis/FlowSensitive/WatchedLiteralsSolver.cpp
  clang/unittests/Analysis/FlowSensitive/ArenaTest.cpp
  clang/unittests/Analysis/FlowSensitive/DebugSupportTest.cpp
  clang/unittests/Analysis/FlowSensitive/SolverTest.cpp
  clang/unittests/Analysis/FlowSensitive/TestingSupport.h

Index: clang/unittests/Analysis/FlowSensitive/TestingSupport.h
===
--- clang/unittests/Analysis/FlowSensitive/TestingSupport.h
+++ clang/unittests/Analysis/FlowSensitive/TestingSupport.h
@@ -43,6 +43,7 @@
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Allocator.h"
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Testing/Annotations/Annotations.h"
@@ -467,55 +468,44 @@
 
 /// Creates and owns constraints which are boolean values.
 class ConstraintContext {
-public:
-  // Creates an atomic boolean value.
-  BoolValue *atom() {
-Vals.push_back(std::make_unique());
-return Vals.back().get();
-  }
+  unsigned NextAtom = 0;
+  llvm::BumpPtrAllocator A;
 
-  // Creates an instance of the Top boolean value.
-  BoolValue *top() {
-Vals.push_back(std::make_unique());
-return Vals.back().get();
+  const Formula *make(Formula::Kind K,
+  llvm::ArrayRef Operands) {
+return &Formula::create(A, K, Operands);
   }
 
-  // Creates a boolean conjunction value.
-  BoolValue *conj(BoolValue *LeftSubVal, BoolValue *RightSubVal) {
-Vals.push_back(
-std::make_unique(*LeftSubVal, *RightSubVal));
-return Vals.back().get();
+public:
+  // Returns a reference to a fresh atomic variable.
+  const Formula *atom() {
+return &Formula::create(A, Formula::AtomRef, {}, NextAtom++);
   }
 
-  // Creates a boolean disjunction value.
-  BoolValue *disj(BoolValue *LeftSubVal, BoolValue *RightSubVal) {
-Vals.push_back(
-std::make_unique(*LeftSubVal, *RightSubVal));
-return Vals.back().get();
+  // Creates a boolean conjunction.
+  const Formula *conj(const Formula *LHS, const Formula *RHS) {
+return make(Formula::And, {LHS, RHS});
   }
 
-  // Creates a boolean negation value.
-  BoolValue *neg(BoolValue *SubVal) {
-Vals.push_back(std::make_unique(*SubVal));
-return Vals.back().get();
+  // Creates a boolean disjunction.
+  const Formula *disj(const Formula *LHS, const Formula *RHS) {
+return make(Formula::Or, {LHS, RHS});
   }
 
-  // Creates a boolean implication value.
-  BoolValue *impl(BoolValue *LeftSubVal, BoolValue *RightSubVal) {
-Vals.push_back(
-std::make_unique(*LeftSubVal, *RightSubVal));
-return Vals.back().get();
+  // Creates a boolean negation.
+  const Formula *neg(const Formula *Operand) {
+return make(Formula::Not, {Operand});
   }
 
-  // Creates a boolean biconditional value.
-  BoolValue *iff(BoolValue *LeftSubVal, BoolValue *RightSubVal) {
-Vals.push_back(
-std::make_unique(*LeftSubVal, *RightSubVal));
-return Vals.back().get();
+  // Creates a boolean implication.
+  const Formula *impl(const Formula *LHS, const Formula *RHS) {
+return make(Formula::Implies, {LHS, RHS});
   }
 
-private:
-  std::vector> Vals;
+  // Creates a boolean biconditional.
+  const Formula *iff(const Formula *LHS, const Formula *RHS) {
+return make(Formula::Equal, {LHS, RHS});
+  }
 };
 
 } // namespace test
Index: clang/unittests/Analysis/FlowSensitive/SolverTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/SolverTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/SolverTest.cpp
@@ -9,9 +9,10 @@
 #include 
 
 #include "TestingSupport.h"
+#include "clang/Analysis/FlowSensitive/Formula.h"
 #include "clang/Analysis/FlowSensitive/Solver.h"
-#include "clang/Analysis/FlowSensitive/Value.h"
 #include "clang/Analysis/FlowSensitive/WatchedLite

[PATCH] D151325: [analyzer] Differentiate lifetime extended temporaries

[Gábor Horváth via Phabricator via cfe-commits]

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

Overall looks good to me. I think the changes to the notes already make the 
analyzer more useful so there are some observable benefits to this patch.




Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h:1282
+  LLVM_ATTRIBUTE_RETURNS_NONNULL
+  const Expr *getExpr() const { return Ex; }
+  LLVM_ATTRIBUTE_RETURNS_NONNULL

Do we actually need this `Expr` in the memory region?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151325

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

[PATCH] D154421: [clang][dataflow] Add a test for a struct that is directly self-referential through a reference.

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

mboehme updated this revision to Diff 537029.
mboehme added a comment.

Actually make the test pass. It was missing an indirection.

No idea how I missed this on the initial upload...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154421

Files:
  clang/unittests/Analysis/FlowSensitive/TransferTest.cpp


Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -656,6 +656,30 @@
   });
 }
 
+TEST(TransferTest, DirectlySelfReferentialReference) {
+  std::string Code = R"(
+struct target {
+  target() {
+(void)0;
+// [[p]]
+  }
+  target &self = *this;
+};
+  )";
+  runDataflow(
+  Code,
+  [](const llvm::StringMap> &Results,
+ ASTContext &ASTCtx) {
+const Environment &Env = getEnvironmentAtAnnotation(Results, "p");
+const ValueDecl *SelfDecl = findValueDecl(ASTCtx, "self");
+
+auto *ThisLoc = Env.getThisPointeeStorageLocation();
+auto *RefVal =
+cast(Env.getValue(ThisLoc->getChild(*SelfDecl)));
+ASSERT_EQ(&RefVal->getReferentLoc(), ThisLoc);
+  });
+}
+
 TEST(TransferTest, MultipleVarsDecl) {
   std::string Code = R"(
 void target() {


Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -656,6 +656,30 @@
   });
 }
 
+TEST(TransferTest, DirectlySelfReferentialReference) {
+  std::string Code = R"(
+struct target {
+  target() {
+(void)0;
+// [[p]]
+  }
+  target &self = *this;
+};
+  )";
+  runDataflow(
+  Code,
+  [](const llvm::StringMap> &Results,
+ ASTContext &ASTCtx) {
+const Environment &Env = getEnvironmentAtAnnotation(Results, "p");
+const ValueDecl *SelfDecl = findValueDecl(ASTCtx, "self");
+
+auto *ThisLoc = Env.getThisPointeeStorageLocation();
+auto *RefVal =
+cast(Env.getValue(ThisLoc->getChild(*SelfDecl)));
+ASSERT_EQ(&RefVal->getReferentLoc(), ThisLoc);
+  });
+}
+
 TEST(TransferTest, MultipleVarsDecl) {
   std::string Code = R"(
 void target() {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D151325: [analyzer] Differentiate lifetime extended temporaries

[Tomasz Kamiński via Phabricator via cfe-commits]

tomasz-kaminski-sonarsource marked an inline comment as done.
tomasz-kaminski-sonarsource added inline comments.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h:1282
+  LLVM_ATTRIBUTE_RETURNS_NONNULL
+  const Expr *getExpr() const { return Ex; }
+  LLVM_ATTRIBUTE_RETURNS_NONNULL

xazax.hun wrote:
> Do we actually need this `Expr` in the memory region?
Yes they are needed for region identification, as one variable my be lifetime 
extending multiple temporary.
For illustration consider following example from test:
```
  auto const& viaReference = RefAggregate{10, Composite{}}; // extends `int`, 
`Composite`, and `RefAggregate`
  clang_analyzer_dump(viaReference);// expected-warning-re 
{{&lifetime_extended_object{RefAggregate, viaReference, S{{[0-9]+}}} }}
  clang_analyzer_dump(viaReference.rx); // expected-warning-re 
{{&lifetime_extended_object{int, viaReference, S{{[0-9]+}}} }}
  clang_analyzer_dump(viaReference.ry); // expected-warning-re 
{{&lifetime_extended_object{Composite, viaReference, S{{[0-9]+}}} }}
```
`viaReference` declaration is extending 3 temporaries, and they get identified 
by expression.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151325

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

[clang] c68ba12 - [clang][modules] Mark fewer identifiers as out-of-date

[Jan Svoboda via cfe-commits]

Author: Jan Svoboda
Date: 2023-07-04T12:58:44+02:00
New Revision: c68ba12abf490716fd7a57bba9c2dda1d537b19c

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

LOG: [clang][modules] Mark fewer identifiers as out-of-date

In `clang-scan-deps` contexts, the number of interesting identifiers in PCM 
files is fairly low (only macros), while the number of identifiers in the 
importing instance is high (builtins). Marking the whole identifier table 
out-of-date triggers lots of benign and expensive calls to 
`ASTReader::updateOutOfDateIdentifiers()`. (That unfortunately happens even for 
unused identifiers due to `SemaRef.IdResolver.begin(II)` line in 
`ASTWriter::WriteASTCore()`.)

This patch makes the main code path more similar to C++ modules, where the PCM 
files have `INTERESTING_IDENTIFIERS` section which lists identifiers that get 
created in the identifier table of the importing instance and marked as 
out-of-date. The only difference is that the main code path doesn't *create* 
identifiers in the table and relies on the importing instance calling 
`ASTReader::get()` when creating new identifier on-demand. It only marks 
existing identifiers as out-of-date.

This speeds up `clang-scan-deps` by 5-10%.

Reviewed By: Bigcheese, benlangmuir

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

Added: 


Modified: 
clang/lib/Serialization/ASTReader.cpp
clang/lib/Serialization/ASTWriter.cpp

Removed: 




diff  --git a/clang/lib/Serialization/ASTReader.cpp 
b/clang/lib/Serialization/ASTReader.cpp
index a9f25ec2ecb75b..78f718533bd55c 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -4386,27 +4386,56 @@ ASTReader::ASTReadResult ASTReader::ReadAST(StringRef 
FileName,
 if (F.OriginalSourceFileID.isValid())
   F.OriginalSourceFileID = TranslateFileID(F, F.OriginalSourceFileID);
 
-// Preload all the pending interesting identifiers by marking them out of
-// date.
 for (auto Offset : F.PreloadIdentifierOffsets) {
   const unsigned char *Data = F.IdentifierTableData + Offset;
 
   ASTIdentifierLookupTrait Trait(*this, F);
   auto KeyDataLen = Trait.ReadKeyDataLength(Data);
   auto Key = Trait.ReadKey(Data, KeyDataLen.first);
-  auto &II = PP.getIdentifierTable().getOwn(Key);
-  II.setOutOfDate(true);
+
+  IdentifierInfo *II;
+  if (!PP.getLangOpts().CPlusPlus) {
+// Identifiers present in both the module file and the importing
+// instance are marked out-of-date so that they can be deserialized
+// on next use via ASTReader::updateOutOfDateIdentifier().
+// Identifiers present in the module file but not in the importing
+// instance are ignored for now, preventing growth of the identifier
+// table. They will be deserialized on first use via ASTReader::get().
+auto It = PP.getIdentifierTable().find(Key);
+if (It == PP.getIdentifierTable().end())
+  continue;
+II = It->second;
+  } else {
+// With C++ modules, not many identifiers are considered interesting.
+// All identifiers in the module file can be placed into the identifier
+// table of the importing instance and marked as out-of-date. This 
makes
+// ASTReader::get() a no-op, and deserialization will take place on
+// first/next use via ASTReader::updateOutOfDateIdentifier().
+II = &PP.getIdentifierTable().getOwn(Key);
+  }
+
+  II->setOutOfDate(true);
 
   // Mark this identifier as being from an AST file so that we can track
   // whether we need to serialize it.
-  markIdentifierFromAST(*this, II);
+  markIdentifierFromAST(*this, *II);
 
   // Associate the ID with the identifier so that the writer can reuse it.
   auto ID = Trait.ReadIdentifierID(Data + KeyDataLen.first);
-  SetIdentifierInfo(ID, &II);
+  SetIdentifierInfo(ID, II);
 }
   }
 
+  // Builtins and library builtins have already been initialized. Mark all
+  // identifiers as out-of-date, so that they are deserialized on first use.
+  if (Type == MK_PCH || Type == MK_Preamble || Type == MK_MainFile)
+for (auto &Id : PP.getIdentifierTable())
+  Id.second->setOutOfDate(true);
+
+  // Mark selectors as out of date.
+  for (const auto &Sel : SelectorGeneration)
+SelectorOutOfDate[Sel.first] = true;
+
   // Setup the import locations and notify the module manager that we've
   // committed to these module files.
   for (ImportedModule &M : Loaded) {
@@ -4424,25 +4453,6 @@ ASTReader::ASTReadResult ASTReader::ReadAST(StringRef 
FileName,
   F.ImportLoc = TranslateSourceLocation(*M.ImportedBy, M.ImportLoc);
   }
 
-  if (!PP.getLangOpts().CPlusPlus ||
-  (Type != MK_ImplicitModu

[PATCH] D151277: [clang][modules] Mark fewer identifiers as out-of-date

[Jan Svoboda 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 rGc68ba12abf49: [clang][modules] Mark fewer identifiers as 
out-of-date (authored by jansvoboda11).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151277

Files:
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp

Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -3573,14 +3573,16 @@
 // the mapping from persistent IDs to strings.
 Writer.SetIdentifierOffset(II, Out.tell());
 
+auto MacroOffset = Writer.getMacroDirectivesOffset(II);
+
 // Emit the offset of the key/data length information to the interesting
 // identifiers table if necessary.
-if (InterestingIdentifierOffsets && isInterestingIdentifier(II))
+if (InterestingIdentifierOffsets &&
+isInterestingIdentifier(II, MacroOffset))
   InterestingIdentifierOffsets->push_back(Out.tell());
 
 unsigned KeyLen = II->getLength() + 1;
 unsigned DataLen = 4; // 4 bytes for the persistent ID << 1
-auto MacroOffset = Writer.getMacroDirectivesOffset(II);
 if (isInterestingIdentifier(II, MacroOffset)) {
   DataLen += 2; // 2 bytes for builtin ID
   DataLen += 2; // 2 bytes for flags
@@ -3659,9 +3661,8 @@
   // strings.
   {
 llvm::OnDiskChainedHashTableGenerator Generator;
-ASTIdentifierTableTrait Trait(
-*this, PP, IdResolver, IsModule,
-(getLangOpts().CPlusPlus && IsModule) ? &InterestingIdents : nullptr);
+ASTIdentifierTableTrait Trait(*this, PP, IdResolver, IsModule,
+  IsModule ? &InterestingIdents : nullptr);
 
 // Look for any identifiers that were named while processing the
 // headers, but are otherwise not needed. We add these to the hash
Index: clang/lib/Serialization/ASTReader.cpp
===
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -4386,27 +4386,56 @@
 if (F.OriginalSourceFileID.isValid())
   F.OriginalSourceFileID = TranslateFileID(F, F.OriginalSourceFileID);
 
-// Preload all the pending interesting identifiers by marking them out of
-// date.
 for (auto Offset : F.PreloadIdentifierOffsets) {
   const unsigned char *Data = F.IdentifierTableData + Offset;
 
   ASTIdentifierLookupTrait Trait(*this, F);
   auto KeyDataLen = Trait.ReadKeyDataLength(Data);
   auto Key = Trait.ReadKey(Data, KeyDataLen.first);
-  auto &II = PP.getIdentifierTable().getOwn(Key);
-  II.setOutOfDate(true);
+
+  IdentifierInfo *II;
+  if (!PP.getLangOpts().CPlusPlus) {
+// Identifiers present in both the module file and the importing
+// instance are marked out-of-date so that they can be deserialized
+// on next use via ASTReader::updateOutOfDateIdentifier().
+// Identifiers present in the module file but not in the importing
+// instance are ignored for now, preventing growth of the identifier
+// table. They will be deserialized on first use via ASTReader::get().
+auto It = PP.getIdentifierTable().find(Key);
+if (It == PP.getIdentifierTable().end())
+  continue;
+II = It->second;
+  } else {
+// With C++ modules, not many identifiers are considered interesting.
+// All identifiers in the module file can be placed into the identifier
+// table of the importing instance and marked as out-of-date. This makes
+// ASTReader::get() a no-op, and deserialization will take place on
+// first/next use via ASTReader::updateOutOfDateIdentifier().
+II = &PP.getIdentifierTable().getOwn(Key);
+  }
+
+  II->setOutOfDate(true);
 
   // Mark this identifier as being from an AST file so that we can track
   // whether we need to serialize it.
-  markIdentifierFromAST(*this, II);
+  markIdentifierFromAST(*this, *II);
 
   // Associate the ID with the identifier so that the writer can reuse it.
   auto ID = Trait.ReadIdentifierID(Data + KeyDataLen.first);
-  SetIdentifierInfo(ID, &II);
+  SetIdentifierInfo(ID, II);
 }
   }
 
+  // Builtins and library builtins have already been initialized. Mark all
+  // identifiers as out-of-date, so that they are deserialized on first use.
+  if (Type == MK_PCH || Type == MK_Preamble || Type == MK_MainFile)
+for (auto &Id : PP.getIdentifierTable())
+  Id.second->setOutOfDate(true);
+
+  // Mark selectors as out of date.
+  for (const auto &Sel : SelectorGeneration)
+SelectorOutOfDate[Sel.first] = true;
+
   // Setup the import locations and notify the module manager that we've
   // commit

[PATCH] D154339: [clang][dataflow] Make `runDataflowReturnError()` a non-template function.

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

mboehme added inline comments.



Comment at: clang/unittests/Analysis/FlowSensitive/TestingSupport.h:389
 /// verify the results.
-template 
-llvm::Error
-runDataflowReturnError(llvm::StringRef Code, VerifyResultsT VerifyResults,
-   DataflowAnalysisOptions Options,
-   LangStandard::Kind Std = LangStandard::lang_cxx17,
-   llvm::StringRef TargetFun = "target") {
-  using ast_matchers::hasName;
-  llvm::SmallVector ASTBuildArgs = {
-  // -fnodelayed-template-parsing is the default everywhere but on Windows.
-  // Set it explicitly so that tests behave the same on Windows as on other
-  // platforms.
-  "-fsyntax-only", "-fno-delayed-template-parsing",
-  "-std=" +
-  std::string(LangStandard::getLangStandardForKind(Std).getName())};
-  AnalysisInputs AI(
-  Code, hasName(TargetFun),
-  [UseBuiltinModel = Options.BuiltinOpts.has_value()](ASTContext &C,
-  Environment &Env) {
-return NoopAnalysis(
-C,
-DataflowAnalysisOptions{
-UseBuiltinModel ? Env.getDataflowAnalysisContext().getOptions()
-: std::optional()});
-  });
-  AI.ASTBuildArgs = ASTBuildArgs;
-  if (Options.BuiltinOpts)
-AI.BuiltinOptions = *Options.BuiltinOpts;
-  return checkDataflow(
-  std::move(AI),
-  /*VerifyResults=*/
-  [&VerifyResults](
-  const llvm::StringMap> &Results,
-  const AnalysisOutputs &AO) { VerifyResults(Results, AO.ASTCtx); });
-}
+llvm::Error runDataflowReturnError(
+llvm::StringRef Code,

gribozavr2 wrote:
> I don't quite understand your comment from the previous patch:
> 
> > This is really just moved from TransferTest.cpp -- and it's more closely 
> > related to the `runDataflow()` functions there and in the newly added 
> > RecordOps.cpp. (I can't call it `runDataflow()` because then it would 
> > differ only in return type from one of the functions in the overload set.)
> 
> I don't see another `checkDataflow()` function with the same signature. 
> Furthermore, `checkDataflow()` overloads above already return an 
> `llvm::Error`.
What I meant is: This function seemed to me to be more closely related to the 
`runDataflow()` functions in TransferTest.cpp than to the `checkDataflow()` 
functions above -- because of the parameter types (the `checkDataflow()` 
functions take `AnalysisInputs`, while this this function doesn't), and also 
because this function, like the `runDataflow()` functions, is not a template, 
while the `checkDataflow()` functions are.

Should I ignore these superficial similarities?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154339

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

[PATCH] D154434: [clang-tidy] Don't emit the whole spelling include header in include-cleaner diagnostic message

[Haojian Wu via Phabricator via cfe-commits]

hokein created this revision.
hokein added a reviewer: kadircet.
Herald added subscribers: PiotrZSL, carlosgalvezp, xazax.hun.
Herald added a reviewer: njames93.
Herald added a project: All.
hokein requested review of this revision.
Herald added a subscriber: ilya-biryukov.
Herald added a project: clang-tools-extra.

To keep the message short and consistent with clangd, and the diagnostics are
attached to the #include line, users have enough context to understand the 
whole #include.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D154434

Files:
  clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/misc/include-cleaner.cpp


Index: clang-tools-extra/test/clang-tidy/checkers/misc/include-cleaner.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/misc/include-cleaner.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/misc/include-cleaner.cpp
@@ -2,11 +2,11 @@
 #include "bar.h"
 // CHECK-FIXES: {{^}}#include "baz.h"{{$}}
 #include "foo.h"
-// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: included header "foo.h" is not 
used directly [misc-include-cleaner]
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: included header foo.h is not used 
directly [misc-include-cleaner]
 // CHECK-FIXES: {{^}}
 // CHECK-FIXES: {{^}}#include {{$}}
 #include 
-// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: included header  is not 
used directly [misc-include-cleaner]
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: included header vector.h is not 
used directly [misc-include-cleaner]
 // CHECK-FIXES: {{^}}
 int BarResult = bar();
 int BazResult = baz();
Index: clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp
===
--- clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp
+++ clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp
@@ -34,6 +34,7 @@
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/ErrorHandling.h"
+#include "llvm/Support/Path.h"
 #include "llvm/Support/Regex.h"
 #include 
 #include 
@@ -171,7 +172,8 @@
 
   for (const auto *Inc : Unused) {
 diag(Inc->HashLocation, "included header %0 is not used directly")
-<< Inc->quote()
+<< llvm::sys::path::filename(Inc->Spelled,
+ llvm::sys::path::Style::posix)
 << FixItHint::CreateRemoval(CharSourceRange::getCharRange(
SM->translateLineCol(SM->getMainFileID(), Inc->Line, 1),
SM->translateLineCol(SM->getMainFileID(), Inc->Line + 1, 1)));


Index: clang-tools-extra/test/clang-tidy/checkers/misc/include-cleaner.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/misc/include-cleaner.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/misc/include-cleaner.cpp
@@ -2,11 +2,11 @@
 #include "bar.h"
 // CHECK-FIXES: {{^}}#include "baz.h"{{$}}
 #include "foo.h"
-// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: included header "foo.h" is not used directly [misc-include-cleaner]
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: included header foo.h is not used directly [misc-include-cleaner]
 // CHECK-FIXES: {{^}}
 // CHECK-FIXES: {{^}}#include {{$}}
 #include 
-// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: included header  is not used directly [misc-include-cleaner]
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: included header vector.h is not used directly [misc-include-cleaner]
 // CHECK-FIXES: {{^}}
 int BarResult = bar();
 int BazResult = baz();
Index: clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp
===
--- clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp
+++ clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp
@@ -34,6 +34,7 @@
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/ErrorHandling.h"
+#include "llvm/Support/Path.h"
 #include "llvm/Support/Regex.h"
 #include 
 #include 
@@ -171,7 +172,8 @@
 
   for (const auto *Inc : Unused) {
 diag(Inc->HashLocation, "included header %0 is not used directly")
-<< Inc->quote()
+<< llvm::sys::path::filename(Inc->Spelled,
+ llvm::sys::path::Style::posix)
 << FixItHint::CreateRemoval(CharSourceRange::getCharRange(
SM->translateLineCol(SM->getMainFileID(), Inc->Line, 1),
SM->translateLineCol(SM->getMainFileID(), Inc->Line + 1, 1)));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D154339: [clang][dataflow] Make `runDataflowReturnError()` a non-template function.

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr2 added inline comments.



Comment at: clang/unittests/Analysis/FlowSensitive/TestingSupport.h:389
 /// verify the results.
-template 
-llvm::Error
-runDataflowReturnError(llvm::StringRef Code, VerifyResultsT VerifyResults,
-   DataflowAnalysisOptions Options,
-   LangStandard::Kind Std = LangStandard::lang_cxx17,
-   llvm::StringRef TargetFun = "target") {
-  using ast_matchers::hasName;
-  llvm::SmallVector ASTBuildArgs = {
-  // -fnodelayed-template-parsing is the default everywhere but on Windows.
-  // Set it explicitly so that tests behave the same on Windows as on other
-  // platforms.
-  "-fsyntax-only", "-fno-delayed-template-parsing",
-  "-std=" +
-  std::string(LangStandard::getLangStandardForKind(Std).getName())};
-  AnalysisInputs AI(
-  Code, hasName(TargetFun),
-  [UseBuiltinModel = Options.BuiltinOpts.has_value()](ASTContext &C,
-  Environment &Env) {
-return NoopAnalysis(
-C,
-DataflowAnalysisOptions{
-UseBuiltinModel ? Env.getDataflowAnalysisContext().getOptions()
-: std::optional()});
-  });
-  AI.ASTBuildArgs = ASTBuildArgs;
-  if (Options.BuiltinOpts)
-AI.BuiltinOptions = *Options.BuiltinOpts;
-  return checkDataflow(
-  std::move(AI),
-  /*VerifyResults=*/
-  [&VerifyResults](
-  const llvm::StringMap> &Results,
-  const AnalysisOutputs &AO) { VerifyResults(Results, AO.ASTCtx); });
-}
+llvm::Error runDataflowReturnError(
+llvm::StringRef Code,

mboehme wrote:
> gribozavr2 wrote:
> > I don't quite understand your comment from the previous patch:
> > 
> > > This is really just moved from TransferTest.cpp -- and it's more closely 
> > > related to the `runDataflow()` functions there and in the newly added 
> > > RecordOps.cpp. (I can't call it `runDataflow()` because then it would 
> > > differ only in return type from one of the functions in the overload set.)
> > 
> > I don't see another `checkDataflow()` function with the same signature. 
> > Furthermore, `checkDataflow()` overloads above already return an 
> > `llvm::Error`.
> What I meant is: This function seemed to me to be more closely related to the 
> `runDataflow()` functions in TransferTest.cpp than to the `checkDataflow()` 
> functions above -- because of the parameter types (the `checkDataflow()` 
> functions take `AnalysisInputs`, while this this function doesn't), and also 
> because this function, like the `runDataflow()` functions, is not a template, 
> while the `checkDataflow()` functions are.
> 
> Should I ignore these superficial similarities?
Ah I think I understand. I'm just trying to make sense of the many overloads we 
have here.

Okay so since this function hardcodes `NoopAnalysis` (unlike `checkDataflow()`) 
do you think that should be reflected in the name somehow?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154339

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

[PATCH] D154339: [clang][dataflow] Make `runDataflowReturnError()` a non-template function.

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

mboehme added inline comments.



Comment at: clang/unittests/Analysis/FlowSensitive/TestingSupport.h:389
 /// verify the results.
-template 
-llvm::Error
-runDataflowReturnError(llvm::StringRef Code, VerifyResultsT VerifyResults,
-   DataflowAnalysisOptions Options,
-   LangStandard::Kind Std = LangStandard::lang_cxx17,
-   llvm::StringRef TargetFun = "target") {
-  using ast_matchers::hasName;
-  llvm::SmallVector ASTBuildArgs = {
-  // -fnodelayed-template-parsing is the default everywhere but on Windows.
-  // Set it explicitly so that tests behave the same on Windows as on other
-  // platforms.
-  "-fsyntax-only", "-fno-delayed-template-parsing",
-  "-std=" +
-  std::string(LangStandard::getLangStandardForKind(Std).getName())};
-  AnalysisInputs AI(
-  Code, hasName(TargetFun),
-  [UseBuiltinModel = Options.BuiltinOpts.has_value()](ASTContext &C,
-  Environment &Env) {
-return NoopAnalysis(
-C,
-DataflowAnalysisOptions{
-UseBuiltinModel ? Env.getDataflowAnalysisContext().getOptions()
-: std::optional()});
-  });
-  AI.ASTBuildArgs = ASTBuildArgs;
-  if (Options.BuiltinOpts)
-AI.BuiltinOptions = *Options.BuiltinOpts;
-  return checkDataflow(
-  std::move(AI),
-  /*VerifyResults=*/
-  [&VerifyResults](
-  const llvm::StringMap> &Results,
-  const AnalysisOutputs &AO) { VerifyResults(Results, AO.ASTCtx); });
-}
+llvm::Error runDataflowReturnError(
+llvm::StringRef Code,

gribozavr2 wrote:
> mboehme wrote:
> > gribozavr2 wrote:
> > > I don't quite understand your comment from the previous patch:
> > > 
> > > > This is really just moved from TransferTest.cpp -- and it's more 
> > > > closely related to the `runDataflow()` functions there and in the newly 
> > > > added RecordOps.cpp. (I can't call it `runDataflow()` because then it 
> > > > would differ only in return type from one of the functions in the 
> > > > overload set.)
> > > 
> > > I don't see another `checkDataflow()` function with the same signature. 
> > > Furthermore, `checkDataflow()` overloads above already return an 
> > > `llvm::Error`.
> > What I meant is: This function seemed to me to be more closely related to 
> > the `runDataflow()` functions in TransferTest.cpp than to the 
> > `checkDataflow()` functions above -- because of the parameter types (the 
> > `checkDataflow()` functions take `AnalysisInputs`, while this this function 
> > doesn't), and also because this function, like the `runDataflow()` 
> > functions, is not a template, while the `checkDataflow()` functions are.
> > 
> > Should I ignore these superficial similarities?
> Ah I think I understand. I'm just trying to make sense of the many overloads 
> we have here.
> 
> Okay so since this function hardcodes `NoopAnalysis` (unlike 
> `checkDataflow()`) do you think that should be reflected in the name somehow?
> Ah I think I understand. I'm just trying to make sense of the many overloads 
> we have here.

Yup, it's confusing...

> Okay so since this function hardcodes `NoopAnalysis` (unlike 
> `checkDataflow()`) do you think that should be reflected in the name somehow?

Good point. Maybe this is a path to a good name. How about something like 
`checkDataflowWithNoopAnalysis()`? Still a mouthful, but I think it reflects 
better what this actually does, and it also explains why it's not a template.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154339

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

[PATCH] D154421: [clang][dataflow] Add a test for a struct that is directly self-referential through a reference.

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

mboehme added a comment.

Pre-merge failure looks unrelated


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154421

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

[PATCH] D154417: [libc++] Disable tree invariant check in asserts mode

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh accepted this revision.
alexfh added a comment.

Thanks for the temporary fix, Hans! We're also affected by this.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154417

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

[PATCH] D154339: [clang][dataflow] Make `runDataflowReturnError()` a non-template function.

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr2 added inline comments.



Comment at: clang/unittests/Analysis/FlowSensitive/TestingSupport.h:389
 /// verify the results.
-template 
-llvm::Error
-runDataflowReturnError(llvm::StringRef Code, VerifyResultsT VerifyResults,
-   DataflowAnalysisOptions Options,
-   LangStandard::Kind Std = LangStandard::lang_cxx17,
-   llvm::StringRef TargetFun = "target") {
-  using ast_matchers::hasName;
-  llvm::SmallVector ASTBuildArgs = {
-  // -fnodelayed-template-parsing is the default everywhere but on Windows.
-  // Set it explicitly so that tests behave the same on Windows as on other
-  // platforms.
-  "-fsyntax-only", "-fno-delayed-template-parsing",
-  "-std=" +
-  std::string(LangStandard::getLangStandardForKind(Std).getName())};
-  AnalysisInputs AI(
-  Code, hasName(TargetFun),
-  [UseBuiltinModel = Options.BuiltinOpts.has_value()](ASTContext &C,
-  Environment &Env) {
-return NoopAnalysis(
-C,
-DataflowAnalysisOptions{
-UseBuiltinModel ? Env.getDataflowAnalysisContext().getOptions()
-: std::optional()});
-  });
-  AI.ASTBuildArgs = ASTBuildArgs;
-  if (Options.BuiltinOpts)
-AI.BuiltinOptions = *Options.BuiltinOpts;
-  return checkDataflow(
-  std::move(AI),
-  /*VerifyResults=*/
-  [&VerifyResults](
-  const llvm::StringMap> &Results,
-  const AnalysisOutputs &AO) { VerifyResults(Results, AO.ASTCtx); });
-}
+llvm::Error runDataflowReturnError(
+llvm::StringRef Code,

mboehme wrote:
> gribozavr2 wrote:
> > mboehme wrote:
> > > gribozavr2 wrote:
> > > > I don't quite understand your comment from the previous patch:
> > > > 
> > > > > This is really just moved from TransferTest.cpp -- and it's more 
> > > > > closely related to the `runDataflow()` functions there and in the 
> > > > > newly added RecordOps.cpp. (I can't call it `runDataflow()` because 
> > > > > then it would differ only in return type from one of the functions in 
> > > > > the overload set.)
> > > > 
> > > > I don't see another `checkDataflow()` function with the same signature. 
> > > > Furthermore, `checkDataflow()` overloads above already return an 
> > > > `llvm::Error`.
> > > What I meant is: This function seemed to me to be more closely related to 
> > > the `runDataflow()` functions in TransferTest.cpp than to the 
> > > `checkDataflow()` functions above -- because of the parameter types (the 
> > > `checkDataflow()` functions take `AnalysisInputs`, while this this 
> > > function doesn't), and also because this function, like the 
> > > `runDataflow()` functions, is not a template, while the `checkDataflow()` 
> > > functions are.
> > > 
> > > Should I ignore these superficial similarities?
> > Ah I think I understand. I'm just trying to make sense of the many 
> > overloads we have here.
> > 
> > Okay so since this function hardcodes `NoopAnalysis` (unlike 
> > `checkDataflow()`) do you think that should be reflected in the name 
> > somehow?
> > Ah I think I understand. I'm just trying to make sense of the many 
> > overloads we have here.
> 
> Yup, it's confusing...
> 
> > Okay so since this function hardcodes `NoopAnalysis` (unlike 
> > `checkDataflow()`) do you think that should be reflected in the name 
> > somehow?
> 
> Good point. Maybe this is a path to a good name. How about something like 
> `checkDataflowWithNoopAnalysis()`? Still a mouthful, but I think it reflects 
> better what this actually does, and it also explains why it's not a template.
SGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154339

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

[PATCH] D154417: [libc++] Disable tree invariant check in asserts mode

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added a comment.

Given the comment in https://reviews.llvm.org/D153672#4468348, I think, this 
should be fine to submit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154417

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

[PATCH] D153296: [AST] Stop evaluate constant expression if the condition expression which in switch statement contains errors

[Yurong via Phabricator via cfe-commits]

yronglin marked 3 inline comments as done.
yronglin added a comment.

Thanks a lot for your comments! @hokein




Comment at: clang/lib/AST/ExprConstant.cpp:4921
+  // value is.
+  if (isa(E))
+return false;

hokein wrote:
> The constant evaluator is not aware of the "error" concept, it is only aware 
> of value-dependent -- the general idea behind is that we treat the 
> dependent-on-error and dependent-on-template-parameters cases the same, they 
> are potentially constant (if we see an expression contains errors, it could 
> be constant depending on how the error is resolved), this will give us nice 
> recovery and avoid bogus following diagnostics.
> 
> So, a `RecoveryExpr` should not result in failure when checking for a 
> potential constant expression.
> 
> I think the right fix is to remove the conditional check `if 
> (!EvaluateDependentExpr(SS->getCond(), Info))` in `EvaluateSwitch`, and 
> return `ESR_Failed` unconditionally (we don't know its value, any switch-case 
> anwser will be wrong in some cases). We already do this for return-statment, 
> do-statement etc.
> 
> 
Do you mean?
```
if (SS->getCond()->isValueDependent()) {
EvaluateDependentExpr(SS->getCond(), Info);
return ESR_Failed;
}
```



Comment at: clang/test/SemaCXX/constexpr-function-recovery-crash.cpp:91
+
+static_assert(test13(), "should not crash"); // expected-error {{static 
assertion expression is not an integral constant expression}}
+

hokein wrote:
> nit: we can simplify it with the `TEST_EVALUATE` macro:
> 
> ```
> TEST_EVALUATE(SwitchErrorCond, switch(undef) { case 0: return 7; default: 
> break;})
> ```
Thanks, I will use `TEST_EVALUATE ` to simplify.



Comment at: clang/test/SemaCXX/constexpr-function-recovery-crash.cpp:93
+
+constexpr int test14() {
+int sum = 0;

hokein wrote:
> Is this a new crash (and the tests below)?
> 
> They don't look like new crashes, I think the current constant evaluator 
> should be able to handle them well. IIUC the only crash we have is the case 
> where we have a error-dependent condition in `switch`?
Thanks you for catch this, it's my mistake, I have ever copied these tests from 
the code above.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153296

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

[PATCH] D153366: [dataflow] Add dedicated representation of boolean formulas

[Mikael Holmén via Phabricator via cfe-commits]

uabelho added a comment.

  Failed Tests (6):
Clang-Unit :: 
Analysis/FlowSensitive/./ClangAnalysisFlowSensitiveTests/BoolValueDebugStringTest/ComplexBooleanWithSomeNames
Clang-Unit :: 
Analysis/FlowSensitive/./ClangAnalysisFlowSensitiveTests/BoolValueDebugStringTest/Conjunction
Clang-Unit :: 
Analysis/FlowSensitive/./ClangAnalysisFlowSensitiveTests/BoolValueDebugStringTest/Disjunction
Clang-Unit :: 
Analysis/FlowSensitive/./ClangAnalysisFlowSensitiveTests/BoolValueDebugStringTest/Iff
Clang-Unit :: 
Analysis/FlowSensitive/./ClangAnalysisFlowSensitiveTests/BoolValueDebugStringTest/Implication
Clang-Unit :: 
Analysis/FlowSensitive/./ClangAnalysisFlowSensitiveTests/BoolValueDebugStringTest/NestedBoolean

https://lab.llvm.org/buildbot/#/builders/139/builds/44269


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153366

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

[PATCH] D154325: [analyzer][NFC] Move away from using raw-for loops inside StaticAnalyzer

[Donát Nagy via Phabricator via cfe-commits]

donat.nagy added a comment.

I don't have concrete remarks, but I would like to note that I'm happy to see 
this cleanup :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154325

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

[clang] 880f306 - [clang][dataflow] Add a test for a struct that is directly self-referential through a reference.

[Martin Braenne via cfe-commits]

Author: Martin Braenne
Date: 2023-07-04T12:06:13Z
New Revision: 880f306226fcb97d85d422480954eb8765ff31c7

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

LOG: [clang][dataflow] Add a test for a struct that is directly 
self-referential through a reference.

The ongoing migration to strict handling of value
categories (see https://discourse.llvm.org/t/70086) will change the way we
handle fields of reference type, and I want to put a test in place that makes
sure we continue to handle this special case correctly.

Depends On D154420

Reviewed By: gribozavr2, xazax.hun

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

Added: 


Modified: 
clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

Removed: 




diff  --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp 
b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
index f7210d29d06cc9..2ccd3e82baadc9 100644
--- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -656,6 +656,30 @@ TEST(TransferTest, SelfReferentialPointerVarDecl) {
   });
 }
 
+TEST(TransferTest, DirectlySelfReferentialReference) {
+  std::string Code = R"(
+struct target {
+  target() {
+(void)0;
+// [[p]]
+  }
+  target &self = *this;
+};
+  )";
+  runDataflow(
+  Code,
+  [](const llvm::StringMap> &Results,
+ ASTContext &ASTCtx) {
+const Environment &Env = getEnvironmentAtAnnotation(Results, "p");
+const ValueDecl *SelfDecl = findValueDecl(ASTCtx, "self");
+
+auto *ThisLoc = Env.getThisPointeeStorageLocation();
+auto *RefVal =
+cast(Env.getValue(ThisLoc->getChild(*SelfDecl)));
+ASSERT_EQ(&RefVal->getReferentLoc(), ThisLoc);
+  });
+}
+
 TEST(TransferTest, MultipleVarsDecl) {
   std::string Code = R"(
 void target() {



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

[clang] 1e7329c - [clang][dataflow] Model variables / fields / funcs used in default initializers.

[Martin Braenne via cfe-commits]

Author: Martin Braenne
Date: 2023-07-04T12:06:10Z
New Revision: 1e7329cd79c53165f113edfe6a2ff06d12899632

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

LOG: [clang][dataflow] Model variables / fields / funcs used in default 
initializers.

The newly added test fails without the other changes in this patch.

Reviewed By: sammccall, gribozavr2

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

Added: 


Modified: 
clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

Removed: 




diff  --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp 
b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
index 689f8abb51c8e0..4a11c09a44f63b 100644
--- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -194,6 +194,8 @@ getFieldsGlobalsAndFuncs(const Stmt &S,
   for (auto *Child : S.children())
 if (Child != nullptr)
   getFieldsGlobalsAndFuncs(*Child, Fields, Vars, Funcs);
+  if (const auto *DefaultInit = dyn_cast(&S))
+getFieldsGlobalsAndFuncs(*DefaultInit->getExpr(), Fields, Vars, Funcs);
 
   if (auto *DS = dyn_cast(&S)) {
 if (DS->isSingleDecl())

diff  --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp 
b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
index 210b85f7ae8d67..f7210d29d06cc9 100644
--- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -5547,4 +5547,40 @@ TEST(TransferTest, AnonymousStructWithInitializer) {
   });
 }
 
+TEST(TransferTest, AnonymousStructWithReferenceField) {
+  std::string Code = R"(
+int global_i = 0;
+struct target {
+  target() {
+(void)0;
+// [[p]]
+  }
+  struct {
+int &i = global_i;
+  };
+};
+  )";
+  runDataflow(
+  Code,
+  [](const llvm::StringMap> &Results,
+ ASTContext &ASTCtx) {
+const Environment &Env = getEnvironmentAtAnnotation(Results, "p");
+const ValueDecl *GlobalIDecl = findValueDecl(ASTCtx, "global_i");
+const ValueDecl *IDecl = findValueDecl(ASTCtx, "i");
+const IndirectFieldDecl *IndirectField =
+findIndirectFieldDecl(ASTCtx, "i");
+
+auto *ThisLoc =
+
cast(Env.getThisPointeeStorageLocation());
+auto &AnonStruct = cast(ThisLoc->getChild(
+*cast(IndirectField->chain().front(;
+
+auto *RefVal =
+cast(Env.getValue(AnonStruct.getChild(*IDecl)));
+
+ASSERT_EQ(&RefVal->getReferentLoc(),
+  Env.getStorageLocation(*GlobalIDecl));
+  });
+}
+
 } // namespace



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

[PATCH] D154420: [clang][dataflow] Model variables / fields / funcs used in default initializers.

[Martin Böhme 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 rG1e7329cd79c5: [clang][dataflow] Model variables / fields / 
funcs used in default initializers. (authored by mboehme).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154420

Files:
  clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
  clang/unittests/Analysis/FlowSensitive/TransferTest.cpp


Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -5547,4 +5547,40 @@
   });
 }
 
+TEST(TransferTest, AnonymousStructWithReferenceField) {
+  std::string Code = R"(
+int global_i = 0;
+struct target {
+  target() {
+(void)0;
+// [[p]]
+  }
+  struct {
+int &i = global_i;
+  };
+};
+  )";
+  runDataflow(
+  Code,
+  [](const llvm::StringMap> &Results,
+ ASTContext &ASTCtx) {
+const Environment &Env = getEnvironmentAtAnnotation(Results, "p");
+const ValueDecl *GlobalIDecl = findValueDecl(ASTCtx, "global_i");
+const ValueDecl *IDecl = findValueDecl(ASTCtx, "i");
+const IndirectFieldDecl *IndirectField =
+findIndirectFieldDecl(ASTCtx, "i");
+
+auto *ThisLoc =
+
cast(Env.getThisPointeeStorageLocation());
+auto &AnonStruct = cast(ThisLoc->getChild(
+*cast(IndirectField->chain().front(;
+
+auto *RefVal =
+cast(Env.getValue(AnonStruct.getChild(*IDecl)));
+
+ASSERT_EQ(&RefVal->getReferentLoc(),
+  Env.getStorageLocation(*GlobalIDecl));
+  });
+}
+
 } // namespace
Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
===
--- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -194,6 +194,8 @@
   for (auto *Child : S.children())
 if (Child != nullptr)
   getFieldsGlobalsAndFuncs(*Child, Fields, Vars, Funcs);
+  if (const auto *DefaultInit = dyn_cast(&S))
+getFieldsGlobalsAndFuncs(*DefaultInit->getExpr(), Fields, Vars, Funcs);
 
   if (auto *DS = dyn_cast(&S)) {
 if (DS->isSingleDecl())


Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -5547,4 +5547,40 @@
   });
 }
 
+TEST(TransferTest, AnonymousStructWithReferenceField) {
+  std::string Code = R"(
+int global_i = 0;
+struct target {
+  target() {
+(void)0;
+// [[p]]
+  }
+  struct {
+int &i = global_i;
+  };
+};
+  )";
+  runDataflow(
+  Code,
+  [](const llvm::StringMap> &Results,
+ ASTContext &ASTCtx) {
+const Environment &Env = getEnvironmentAtAnnotation(Results, "p");
+const ValueDecl *GlobalIDecl = findValueDecl(ASTCtx, "global_i");
+const ValueDecl *IDecl = findValueDecl(ASTCtx, "i");
+const IndirectFieldDecl *IndirectField =
+findIndirectFieldDecl(ASTCtx, "i");
+
+auto *ThisLoc =
+cast(Env.getThisPointeeStorageLocation());
+auto &AnonStruct = cast(ThisLoc->getChild(
+*cast(IndirectField->chain().front(;
+
+auto *RefVal =
+cast(Env.getValue(AnonStruct.getChild(*IDecl)));
+
+ASSERT_EQ(&RefVal->getReferentLoc(),
+  Env.getStorageLocation(*GlobalIDecl));
+  });
+}
+
 } // namespace
Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
===
--- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -194,6 +194,8 @@
   for (auto *Child : S.children())
 if (Child != nullptr)
   getFieldsGlobalsAndFuncs(*Child, Fields, Vars, Funcs);
+  if (const auto *DefaultInit = dyn_cast(&S))
+getFieldsGlobalsAndFuncs(*DefaultInit->getExpr(), Fields, Vars, Funcs);
 
   if (auto *DS = dyn_cast(&S)) {
 if (DS->isSingleDecl())
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D154421: [clang][dataflow] Add a test for a struct that is directly self-referential through a reference.

[Martin Böhme 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 rG880f306226fc: [clang][dataflow] Add a test for a struct that 
is directly self-referential… (authored by mboehme).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154421

Files:
  clang/unittests/Analysis/FlowSensitive/TransferTest.cpp


Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -656,6 +656,30 @@
   });
 }
 
+TEST(TransferTest, DirectlySelfReferentialReference) {
+  std::string Code = R"(
+struct target {
+  target() {
+(void)0;
+// [[p]]
+  }
+  target &self = *this;
+};
+  )";
+  runDataflow(
+  Code,
+  [](const llvm::StringMap> &Results,
+ ASTContext &ASTCtx) {
+const Environment &Env = getEnvironmentAtAnnotation(Results, "p");
+const ValueDecl *SelfDecl = findValueDecl(ASTCtx, "self");
+
+auto *ThisLoc = Env.getThisPointeeStorageLocation();
+auto *RefVal =
+cast(Env.getValue(ThisLoc->getChild(*SelfDecl)));
+ASSERT_EQ(&RefVal->getReferentLoc(), ThisLoc);
+  });
+}
+
 TEST(TransferTest, MultipleVarsDecl) {
   std::string Code = R"(
 void target() {


Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -656,6 +656,30 @@
   });
 }
 
+TEST(TransferTest, DirectlySelfReferentialReference) {
+  std::string Code = R"(
+struct target {
+  target() {
+(void)0;
+// [[p]]
+  }
+  target &self = *this;
+};
+  )";
+  runDataflow(
+  Code,
+  [](const llvm::StringMap> &Results,
+ ASTContext &ASTCtx) {
+const Environment &Env = getEnvironmentAtAnnotation(Results, "p");
+const ValueDecl *SelfDecl = findValueDecl(ASTCtx, "self");
+
+auto *ThisLoc = Env.getThisPointeeStorageLocation();
+auto *RefVal =
+cast(Env.getValue(ThisLoc->getChild(*SelfDecl)));
+ASSERT_EQ(&RefVal->getReferentLoc(), ThisLoc);
+  });
+}
+
 TEST(TransferTest, MultipleVarsDecl) {
   std::string Code = R"(
 void target() {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D154437: [analyzer][NFC] Make AnalyzerConfig round-tripping deterministic

[Balázs Benics via Phabricator via cfe-commits]

steakhal created this revision.
steakhal added reviewers: NoQ, xazax.hun, jansvoboda11.
Herald added subscribers: PiotrZSL, carlosgalvezp, manas, ASDenysPetrov, 
martong, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, mgrang, 
szepet, baloghadamsoftware.
Herald added a reviewer: Szelethus.
Herald added a reviewer: njames93.
Herald added a project: All.
steakhal requested review of this revision.
Herald added projects: clang, clang-tools-extra.
Herald added a subscriber: cfe-commits.

In asserted builds, we do a so-called "round-tripping". This is called
from `CompilerInvocation::CreateFromArgs`, which eventually calls
`generateCC1CommandLine` from the lambda. That, in turn, calls a bunch
of `GenerateXXX` functions, thus ours as well: `GenerateAnalyzerArgs`.
The round tripping ensures some consistency property, also that the
generator generates the arguments in a deterministic order.

This works at the moment, but as soon as we add a new AnalyzerOption to
the AnalyzerOptions.def file, it's pretty likely that it will produce
the same sequence of args BUT in a different order for the second round
of round-tripping, resulting in an `err_cc1_round_trip_mismatch`
diagnostic.

This happens because inside `GenerateAnalyzerArgs`, we iterate over the
`Opts.Config`, which is of type `StringMap`. Well, the iteration order
is not guaranteed to be deterministic for that. Quote
https://llvm.org/docs/ProgrammersManual.html#llvm-adt-stringmap-h

> StringMap iteration order, however, is not guaranteed to be
> deterministic, so any uses which require that should instead use a
> std::map.

Consequently, to allow adding/removing analyzer options to
`AnalyzerOptions.def` without relying on the current iteration order, we
need to make that deterministic. And that is what I'm proposing in this
patch.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D154437

Files:
  clang-tools-extra/clang-tidy/ClangTidy.cpp
  clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/StaticAnalyzer/Checkers/DebugCheckers.cpp
  clang/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
  clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp

Index: clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
===
--- clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
+++ clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
@@ -357,7 +357,7 @@
   // to it's default value, and if we're in non-compatibility mode, we'll also
   // emit an error.
 
-  StringRef SuppliedValue = It.first->getValue();
+  StringRef SuppliedValue = It.first->second;
 
   if (Option.OptionType == "bool") {
 if (SuppliedValue != "true" && SuppliedValue != "false") {
@@ -366,7 +366,7 @@
 << FullOption << "a boolean value";
   }
 
-  It.first->setValue(std::string(Option.DefaultValStr));
+  It.first->second = Option.DefaultValStr.str();
 }
 return;
   }
@@ -380,7 +380,7 @@
 << FullOption << "an integer value";
   }
 
-  It.first->setValue(std::string(Option.DefaultValStr));
+  It.first->second = Option.DefaultValStr.str();
 }
 return;
   }
@@ -492,7 +492,7 @@
 StringRef SuppliedCheckerOrPackage;
 StringRef SuppliedOption;
 std::tie(SuppliedCheckerOrPackage, SuppliedOption) =
-Config.getKey().split(':');
+StringRef(Config.first).split(':');
 
 if (SuppliedOption.empty())
   continue;
Index: clang/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
===
--- clang/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
+++ clang/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
@@ -136,7 +136,7 @@
 ConfigTable::const_iterator I =
 Config.find((Twine(CheckerName) + ":" + OptionName).str());
 if (I != E)
-  return StringRef(I->getValue());
+  return StringRef(I->second);
 size_t Pos = CheckerName.rfind('.');
 if (Pos == StringRef::npos)
   break;
Index: clang/lib/StaticAnalyzer/Checkers/DebugCheckers.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/DebugCheckers.cpp
+++ clang/lib/StaticAnalyzer/Checkers/DebugCheckers.cpp
@@ -259,9 +259,9 @@
 class ConfigDumper : public Checker< check::EndOfTranslationUnit > {
   typedef AnalyzerOptions::ConfigTable Table;
 
-  static int compareEntry(const Table::MapEntryTy *const *LHS,
-  const Table::MapEntryTy *const *RHS) {
-return (*LHS)->getKey().compare((*RHS)->getKey());
+  static int compareEntry(const Table::value_type *const *LHS,
+  const Table::value_type *const *RHS) {
+return (*LHS)->first.compare((*RHS)->first);
   }
 
 public:
@@ -270,7 +270,7 @@
  BugReporter &BR) const {
 const Table &Config = mgr.options.Config;
 
-SmallVector Keys;
+SmallVector Keys;
 for (Tab

[PATCH] D154339: [clang][dataflow] Make `runDataflowReturnError()` a non-template function.

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

mboehme updated this revision to Diff 537061.
mboehme added a comment.

Rename `runDataflowReturnError()` as discussed in review.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154339

Files:
  clang/include/clang/Analysis/FlowSensitive/RecordOps.h
  clang/unittests/Analysis/FlowSensitive/RecordOpsTest.cpp
  clang/unittests/Analysis/FlowSensitive/TestingSupport.cpp
  clang/unittests/Analysis/FlowSensitive/TestingSupport.h
  clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -38,21 +38,28 @@
 using ::testing::NotNull;
 using ::testing::UnorderedElementsAre;
 
-template 
-void runDataflow(llvm::StringRef Code, VerifyResultsT VerifyResults,
- DataflowAnalysisOptions Options,
- LangStandard::Kind Std = LangStandard::lang_cxx17,
- llvm::StringRef TargetFun = "target") {
-  ASSERT_THAT_ERROR(
-  runDataflowReturnError(Code, VerifyResults, Options, Std, TargetFun),
-  llvm::Succeeded());
-}
-
-template 
-void runDataflow(llvm::StringRef Code, VerifyResultsT VerifyResults,
- LangStandard::Kind Std = LangStandard::lang_cxx17,
- bool ApplyBuiltinTransfer = true,
- llvm::StringRef TargetFun = "target") {
+void runDataflow(
+llvm::StringRef Code,
+std::function<
+void(const llvm::StringMap> &,
+ ASTContext &)>
+VerifyResults,
+DataflowAnalysisOptions Options,
+LangStandard::Kind Std = LangStandard::lang_cxx17,
+llvm::StringRef TargetFun = "target") {
+  ASSERT_THAT_ERROR(checkDataflowWithNoopAnalysis(Code, VerifyResults, Options,
+  Std, TargetFun),
+llvm::Succeeded());
+}
+
+void runDataflow(
+llvm::StringRef Code,
+std::function<
+void(const llvm::StringMap> &,
+ ASTContext &)>
+VerifyResults,
+LangStandard::Kind Std = LangStandard::lang_cxx17,
+bool ApplyBuiltinTransfer = true, llvm::StringRef TargetFun = "target") {
   runDataflow(Code, std::move(VerifyResults),
   {ApplyBuiltinTransfer ? BuiltinOptions{}
 : std::optional()},
@@ -2658,7 +2665,7 @@
 void target() {}
   )";
   ASSERT_THAT_ERROR(
-  runDataflowReturnError(
+  checkDataflowWithNoopAnalysis(
   Code,
   [](const llvm::StringMap> &Results,
  ASTContext &ASTCtx) {},
@@ -2674,7 +2681,7 @@
 };
   )";
   ASSERT_THAT_ERROR(
-  runDataflowReturnError(
+  checkDataflowWithNoopAnalysis(
   Code,
   [](const llvm::StringMap> &Results,
  ASTContext &ASTCtx) {},
Index: clang/unittests/Analysis/FlowSensitive/TestingSupport.h
===
--- clang/unittests/Analysis/FlowSensitive/TestingSupport.h
+++ clang/unittests/Analysis/FlowSensitive/TestingSupport.h
@@ -386,40 +386,15 @@
 
 /// Runs dataflow on `Code` with a `NoopAnalysis` and calls `VerifyResults` to
 /// verify the results.
-template 
-llvm::Error
-runDataflowReturnError(llvm::StringRef Code, VerifyResultsT VerifyResults,
-   DataflowAnalysisOptions Options,
-   LangStandard::Kind Std = LangStandard::lang_cxx17,
-   llvm::StringRef TargetFun = "target") {
-  using ast_matchers::hasName;
-  llvm::SmallVector ASTBuildArgs = {
-  // -fnodelayed-template-parsing is the default everywhere but on Windows.
-  // Set it explicitly so that tests behave the same on Windows as on other
-  // platforms.
-  "-fsyntax-only", "-fno-delayed-template-parsing",
-  "-std=" +
-  std::string(LangStandard::getLangStandardForKind(Std).getName())};
-  AnalysisInputs AI(
-  Code, hasName(TargetFun),
-  [UseBuiltinModel = Options.BuiltinOpts.has_value()](ASTContext &C,
-  Environment &Env) {
-return NoopAnalysis(
-C,
-DataflowAnalysisOptions{
-UseBuiltinModel ? Env.getDataflowAnalysisContext().getOptions()
-: std::optional()});
-  });
-  AI.ASTBuildArgs = ASTBuildArgs;
-  if (Options.BuiltinOpts)
-AI.BuiltinOptions = *Options.BuiltinOpts;
-  return checkDataflow(
-  std::move(AI),
-  /*VerifyResults=*/
-  [&VerifyResults](
-  const llvm::StringMap> &Results,
-  const AnalysisOutputs &AO) { VerifyResults(Results, AO.ASTCtx); });
-}
+llvm::Error checkDataflowWithNoopAnalysis(
+llvm::StringRef Code,
+std::function<
+void(const llvm::StringMap> &,
+ ASTContext &)>
+  

[PATCH] D153366: [dataflow] Add dedicated representation of boolean formulas

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Looks like this breaks tests on windows: http://45.33.8.238/win/80815/step_7.txt

Please take a look etc


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153366

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

[PATCH] D154325: [analyzer][NFC] Move away from using raw-for loops inside StaticAnalyzer

[Balázs Benics via Phabricator via cfe-commits]

steakhal planned changes to this revision.
steakhal marked an inline comment as done.
steakhal added inline comments.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h:501
   public:
+const iterator &operator*() const { return *this; }
+

xazax.hun wrote:
> Is this just a dummy to make sure this satisfies some concepts? I think I 
> comment might be helpful in that case, people might find a noop dereference 
> operator confusing. Same for some other places.
Yes. I'll add a comment.
This falls into the "weird" iterators category.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h:749-750
 
   referenced_vars_iterator referenced_vars_begin() const;
   referenced_vars_iterator referenced_vars_end() const;
+  llvm::iterator_range referenced_vars() const;

xazax.hun wrote:
> Do we need these with the addition of `referenced_vars`?
I no longer remember where, but yes. But for sure I went through these to see 
if I could remove them.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h:637-638
 
   region_iterator region_begin() const { return LiveRegionRoots.begin(); }
   region_iterator region_end() const { return LiveRegionRoots.end(); }
+  llvm::iterator_range regions() const {

xazax.hun wrote:
> Should we remove these?
I'm pretty sure we need these somewhere.
One prominent place is where we dump these into JSONs. We do some fancy 
iteration there. I'm not sure that is the reason for this particular case.
I'll give it another look to confirm.



Comment at: clang/lib/StaticAnalyzer/Checkers/MallocSizeofChecker.cpp:194-195
 
-  for (CallExpr::const_arg_iterator ai = i->AllocCall->arg_begin(),
-   ae = i->AllocCall->arg_end(); ai != ae; ++ai) {
-if (!(*ai)->getType()->isIntegralOrUnscopedEnumerationType())
+  for (const Expr *Arg : make_range(CallRec.AllocCall->arg_begin(),
+CallRec.AllocCall->arg_end())) {
+if (!Arg->getType()->isIntegralOrUnscopedEnumerationType())

xazax.hun wrote:
> 
Makes sense.



Comment at: clang/lib/StaticAnalyzer/Core/AnalysisManager.cpp:54
+  for (PathDiagnosticConsumer *Consumer : PathConsumers) {
+delete Consumer;
   }

xazax.hun wrote:
> Hm, I wonder whether we actually want to use `unique_ptr`s to make the 
> ownership explicit. Feel free to leave as is in this PR.
The ownership model of these consumers is so messed up.
It's not that simple to fix. I was bitten by dangling consumers/graphs so many 
times now only counting **last** year.
So, yea. Maybe one day.



Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp:173-177
+} else {
 // The left-hand side may bind to a different value then the
 // computation type.
 LHSVal = svalBuilder.evalCast(Result, LTy, CTy);
+}

xazax.hun wrote:
> Might be an issue with Phab, but indent looks strange to me here.
Thanks for catching this. Indeed. I'll have a look. Probably I messed something 
up.



Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp:1190
   CXXRecordDecl::field_iterator CurField = LE->getLambdaClass()->field_begin();
-  for (LambdaExpr::const_capture_init_iterator i = LE->capture_init_begin(),
-   e = LE->capture_init_end();
-   i != e; ++i, ++CurField, ++Idx) {
+  for (auto i = LE->capture_init_begin(), e = LE->capture_init_end(); i != e;
+   ++i, ++CurField, ++Idx) {

xazax.hun wrote:
> Ha about using `capture_inits`?
I would count this as a "fancy" iteration as we increment/advance more logical 
sequences together.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154325

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

[PATCH] D154437: [analyzer][NFC] Make AnalyzerConfig round-tripping deterministic

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added a comment.

Has this not been addressed by D142861  
already?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154437

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

[PATCH] D151277: [clang][modules] Mark fewer identifiers as out-of-date

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added a comment.

I finally got around to testing this patch on compiles of explicit modules. The 
number of elapsed CPU cycles decreased (by up to 1.8% for large modules) and 
the cumulative size of PCM files decreased by 2% (though small modules are a 
few bytes larger). These findings give me confidence to land this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151277

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

[PATCH] D153296: [AST] Stop evaluate constant expression if the condition expression which in switch statement contains errors

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/AST/ExprConstant.cpp:4921
+  // value is.
+  if (isa(E))
+return false;

yronglin wrote:
> hokein wrote:
> > The constant evaluator is not aware of the "error" concept, it is only 
> > aware of value-dependent -- the general idea behind is that we treat the 
> > dependent-on-error and dependent-on-template-parameters cases the same, 
> > they are potentially constant (if we see an expression contains errors, it 
> > could be constant depending on how the error is resolved), this will give 
> > us nice recovery and avoid bogus following diagnostics.
> > 
> > So, a `RecoveryExpr` should not result in failure when checking for a 
> > potential constant expression.
> > 
> > I think the right fix is to remove the conditional check `if 
> > (!EvaluateDependentExpr(SS->getCond(), Info))` in `EvaluateSwitch`, and 
> > return `ESR_Failed` unconditionally (we don't know its value, any 
> > switch-case anwser will be wrong in some cases). We already do this for 
> > return-statment, do-statement etc.
> > 
> > 
> Do you mean?
> ```
> if (SS->getCond()->isValueDependent()) {
> EvaluateDependentExpr(SS->getCond(), Info);
> return ESR_Failed;
> }
> ```
> the general idea behind is that we treat the dependent-on-error and 
> dependent-on-template-parameters cases the same, they are potentially 
> constant (if we see an expression contains errors, it could be constant 
> depending on how the error is resolved), this will give us nice recovery and 
> avoid bogus following diagnostics.

I could use some further education on why this is the correct approach. For a 
dependent-on-template-parameters case, this makes sense -- either the template 
will be instantiated (at which point we'll know if it's a constant expression) 
or it won't be (at which point it's constant expression-ness doesn't matter). 
But for error recovery, we will *never* get a valid constant expression.

I worry about the performance overhead of continuing on with constant 
expression evaluation in the error case. We use these code paths not only to 
get a value but to say "is this a constant expression at all?".

I don't see why the fix should be localized to just the switch statement 
condition; it seems like *any* attempt to get a dependent value from an error 
recovery expression is a point at which we can definitively say "this is not a 
constant expression" and move on.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153296

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

[clang-tools-extra] 2444fb9 - [clang-tidy] Don't emit the whole spelling include header in include-cleaner diagnostic message

[Haojian Wu via cfe-commits]

Author: Haojian Wu
Date: 2023-07-04T14:32:27+02:00
New Revision: 2444fb96435ecae73211f3ced3d06e48719afe97

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

LOG: [clang-tidy] Don't emit the whole spelling include header in 
include-cleaner diagnostic message

To keep the message short and consistent with clangd, and the diagnostics are
attached to the #include line, users have enough context to understand the 
whole #include.

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

Added: 


Modified: 
clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp
clang-tools-extra/test/clang-tidy/checkers/misc/include-cleaner.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp 
b/clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp
index b9f44c96818db0..064eccd9cd6678 100644
--- a/clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp
+++ b/clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp
@@ -34,6 +34,7 @@
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/ErrorHandling.h"
+#include "llvm/Support/Path.h"
 #include "llvm/Support/Regex.h"
 #include 
 #include 
@@ -171,7 +172,8 @@ void IncludeCleanerCheck::check(const 
MatchFinder::MatchResult &Result) {
 
   for (const auto *Inc : Unused) {
 diag(Inc->HashLocation, "included header %0 is not used directly")
-<< Inc->quote()
+<< llvm::sys::path::filename(Inc->Spelled,
+ llvm::sys::path::Style::posix)
 << FixItHint::CreateRemoval(CharSourceRange::getCharRange(
SM->translateLineCol(SM->getMainFileID(), Inc->Line, 1),
SM->translateLineCol(SM->getMainFileID(), Inc->Line + 1, 1)));

diff  --git 
a/clang-tools-extra/test/clang-tidy/checkers/misc/include-cleaner.cpp 
b/clang-tools-extra/test/clang-tidy/checkers/misc/include-cleaner.cpp
index aef115d59bbef2..e10ac3f46e2e9d 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/misc/include-cleaner.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/misc/include-cleaner.cpp
@@ -2,11 +2,11 @@
 #include "bar.h"
 // CHECK-FIXES: {{^}}#include "baz.h"{{$}}
 #include "foo.h"
-// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: included header "foo.h" is not 
used directly [misc-include-cleaner]
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: included header foo.h is not used 
directly [misc-include-cleaner]
 // CHECK-FIXES: {{^}}
 // CHECK-FIXES: {{^}}#include {{$}}
 #include 
-// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: included header  is not 
used directly [misc-include-cleaner]
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: included header vector.h is not 
used directly [misc-include-cleaner]
 // CHECK-FIXES: {{^}}
 int BarResult = bar();
 int BazResult = baz();



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

[PATCH] D154434: [clang-tidy] Don't emit the whole spelling include header in include-cleaner diagnostic message

[Haojian Wu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG2444fb96435e: [clang-tidy] Don't emit the whole 
spelling include header in include-cleaner… (authored by hokein).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154434

Files:
  clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/misc/include-cleaner.cpp


Index: clang-tools-extra/test/clang-tidy/checkers/misc/include-cleaner.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/misc/include-cleaner.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/misc/include-cleaner.cpp
@@ -2,11 +2,11 @@
 #include "bar.h"
 // CHECK-FIXES: {{^}}#include "baz.h"{{$}}
 #include "foo.h"
-// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: included header "foo.h" is not 
used directly [misc-include-cleaner]
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: included header foo.h is not used 
directly [misc-include-cleaner]
 // CHECK-FIXES: {{^}}
 // CHECK-FIXES: {{^}}#include {{$}}
 #include 
-// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: included header  is not 
used directly [misc-include-cleaner]
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: included header vector.h is not 
used directly [misc-include-cleaner]
 // CHECK-FIXES: {{^}}
 int BarResult = bar();
 int BazResult = baz();
Index: clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp
===
--- clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp
+++ clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp
@@ -34,6 +34,7 @@
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/ErrorHandling.h"
+#include "llvm/Support/Path.h"
 #include "llvm/Support/Regex.h"
 #include 
 #include 
@@ -171,7 +172,8 @@
 
   for (const auto *Inc : Unused) {
 diag(Inc->HashLocation, "included header %0 is not used directly")
-<< Inc->quote()
+<< llvm::sys::path::filename(Inc->Spelled,
+ llvm::sys::path::Style::posix)
 << FixItHint::CreateRemoval(CharSourceRange::getCharRange(
SM->translateLineCol(SM->getMainFileID(), Inc->Line, 1),
SM->translateLineCol(SM->getMainFileID(), Inc->Line + 1, 1)));


Index: clang-tools-extra/test/clang-tidy/checkers/misc/include-cleaner.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/misc/include-cleaner.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/misc/include-cleaner.cpp
@@ -2,11 +2,11 @@
 #include "bar.h"
 // CHECK-FIXES: {{^}}#include "baz.h"{{$}}
 #include "foo.h"
-// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: included header "foo.h" is not used directly [misc-include-cleaner]
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: included header foo.h is not used directly [misc-include-cleaner]
 // CHECK-FIXES: {{^}}
 // CHECK-FIXES: {{^}}#include {{$}}
 #include 
-// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: included header  is not used directly [misc-include-cleaner]
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: included header vector.h is not used directly [misc-include-cleaner]
 // CHECK-FIXES: {{^}}
 int BarResult = bar();
 int BazResult = baz();
Index: clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp
===
--- clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp
+++ clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp
@@ -34,6 +34,7 @@
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/ErrorHandling.h"
+#include "llvm/Support/Path.h"
 #include "llvm/Support/Regex.h"
 #include 
 #include 
@@ -171,7 +172,8 @@
 
   for (const auto *Inc : Unused) {
 diag(Inc->HashLocation, "included header %0 is not used directly")
-<< Inc->quote()
+<< llvm::sys::path::filename(Inc->Spelled,
+ llvm::sys::path::Style::posix)
 << FixItHint::CreateRemoval(CharSourceRange::getCharRange(
SM->translateLineCol(SM->getMainFileID(), Inc->Line, 1),
SM->translateLineCol(SM->getMainFileID(), Inc->Line + 1, 1)));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D151696: [x86] Remove CPU_SPECIFIC* MACROs and add getCPUDispatchMangling

[Freddy, Ye via Phabricator via cfe-commits]

FreddyYe updated this revision to Diff 537065.
FreddyYe marked 2 inline comments as done.
FreddyYe added a comment.

Add comments about Mangling and OnlyForCPUDispatchSpecific,
and remove the supporting more/new CPU names for cpu_specific/dispatch feature.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151696

Files:
  clang/lib/Basic/Targets/X86.cpp
  clang/lib/Basic/Targets/X86.h
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGen/attr-cpuspecific-avx-abi.c
  clang/test/CodeGen/attr-cpuspecific.c
  llvm/include/llvm/TargetParser/X86TargetParser.def
  llvm/include/llvm/TargetParser/X86TargetParser.h
  llvm/lib/Target/X86/X86.td
  llvm/lib/TargetParser/X86TargetParser.cpp
  llvm/test/CodeGen/X86/cpus-intel.ll

Index: llvm/test/CodeGen/X86/cpus-intel.ll
===
--- llvm/test/CodeGen/X86/cpus-intel.ll
+++ llvm/test/CodeGen/X86/cpus-intel.ll
@@ -6,16 +6,24 @@
 ; RUN: llc < %s -o /dev/null -mtriple=i686-unknown-unknown -mcpu=i586 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
 ; RUN: llc < %s -o /dev/null -mtriple=i686-unknown-unknown -mcpu=pentium 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
 ; RUN: llc < %s -o /dev/null -mtriple=i686-unknown-unknown -mcpu=pentium-mmx 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
+; RUN: llc < %s -o /dev/null -mtriple=i686-unknown-unknown -mcpu=pentium_mmx 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
 ; RUN: llc < %s -o /dev/null -mtriple=i686-unknown-unknown -mcpu=i686 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
 ; RUN: llc < %s -o /dev/null -mtriple=i686-unknown-unknown -mcpu=pentiumpro 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
+; RUN: llc < %s -o /dev/null -mtriple=i686-unknown-unknown -mcpu=pentium_pro 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
 ; RUN: llc < %s -o /dev/null -mtriple=i686-unknown-unknown -mcpu=pentium2 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
+; RUN: llc < %s -o /dev/null -mtriple=i686-unknown-unknown -mcpu=pentium_ii 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
 ; RUN: llc < %s -o /dev/null -mtriple=i686-unknown-unknown -mcpu=pentium3 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
 ; RUN: llc < %s -o /dev/null -mtriple=i686-unknown-unknown -mcpu=pentium3m 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
+; RUN: llc < %s -o /dev/null -mtriple=i686-unknown-unknown -mcpu=pentium_iii 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
+; RUN: llc < %s -o /dev/null -mtriple=i686-unknown-unknown -mcpu=pentium_iii_no_xmm_regs 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
+; RUN: llc < %s -o /dev/null -mtriple=i686-unknown-unknown -mcpu=pentium_m 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
 ; RUN: llc < %s -o /dev/null -mtriple=i686-unknown-unknown -mcpu=pentium-m 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
 ; RUN: llc < %s -o /dev/null -mtriple=i686-unknown-unknown -mcpu=pentium4 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
 ; RUN: llc < %s -o /dev/null -mtriple=i686-unknown-unknown -mcpu=pentium4m 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
+; RUN: llc < %s -o /dev/null -mtriple=i686-unknown-unknown -mcpu=pentium_4 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
 ; RUN: llc < %s -o /dev/null -mtriple=i686-unknown-unknown -mcpu=yonah 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
 ; RUN: llc < %s -o /dev/null -mtriple=i686-unknown-unknown -mcpu=prescott 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
+; RUN: llc < %s -o /dev/null -mtriple=i686-unknown-unknown -mcpu=pentium_4_sse3 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
 ; RUN: llc < %s -o /dev/null -mtriple=i686-unknown-unknown -mcpu=lakemont 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
 ; RUN: llc < %s -o /dev/null -mtriple=i686-unknown-unknown -mcpu=raptorlake 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
 ; RUN: llc < %s -o /dev/null -mtriple=i686-unknown-unknown -mcpu=meteorlake 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
@@ -26,26 +34,39 @@
 
 ; RUN: llc < %s -o /dev/null -mtriple=x86_64-unknown-unknown -mcpu=nocona 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
 ; RUN: llc < %s -o /dev/null -mtriple=x86_64-unknown-unknown -mcpu=core2 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
+; RUN: llc < %s -o /dev/null -mtriple=x86_64-unknown-unknown -mcpu=core_2_duo_ssse3 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
 ; RUN: llc < %s -o /dev/null -mtriple=x86_64-unknown-unknown -mcpu=penryn 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
+; RUN: llc < %s -o

[PATCH] D61508: [clang-tidy] bugprone-header-guard : a simple version of llvm-header-guard

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D61508#4469426 , @garymm wrote:

> @aaron.ballman what's remaining for this to be mergeable?

It needs to be rebased onto trunk and it looks like there's some test coverage 
missing for unsupported styles. It should probably go through more review with 
the current code owners as well, in case the landscape in clang-tidy has 
changed in the past few years in this area.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D61508

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

[PATCH] D153366: [dataflow] Add dedicated representation of boolean formulas

[Tom Weaver via Phabricator via cfe-commits]

TWeaver added a comment.

Hello and good afternoon from the UK,

I believe this change has introduced test failures on the following buildbot:

https://lab.llvm.org/buildbot/#/builders/139/builds/44269

are you able to take a look see?

Much appreciated,
Tom W


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153366

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

[clang] d0be47c - [clang][dataflow] Make `runDataflowReturnError()` a non-template function.

[Martin Braenne via cfe-commits]

Author: Martin Braenne
Date: 2023-07-04T12:44:49Z
New Revision: d0be47c51cfdb8b94eb20279c02e8e2875380919

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

LOG: [clang][dataflow] Make `runDataflowReturnError()` a non-template function.

It turns out this didn't need to be a template at all.

Likewise, change callers to they're non-template functions.

Also, correct / clarify some comments in RecordOps.h.

This is in response to post-commit comments on https://reviews.llvm.org/D153006.

Reviewed By: gribozavr2

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

Added: 


Modified: 
clang/include/clang/Analysis/FlowSensitive/RecordOps.h
clang/unittests/Analysis/FlowSensitive/RecordOpsTest.cpp
clang/unittests/Analysis/FlowSensitive/TestingSupport.cpp
clang/unittests/Analysis/FlowSensitive/TestingSupport.h
clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

Removed: 




diff  --git a/clang/include/clang/Analysis/FlowSensitive/RecordOps.h 
b/clang/include/clang/Analysis/FlowSensitive/RecordOps.h
index f5a0a5a501c119..c9c302b9199bf2 100644
--- a/clang/include/clang/Analysis/FlowSensitive/RecordOps.h
+++ b/clang/include/clang/Analysis/FlowSensitive/RecordOps.h
@@ -23,7 +23,7 @@ namespace dataflow {
 ///
 /// This performs a deep copy, i.e. it copies every field and recurses on
 /// fields of record type. It also copies properties from the `StructValue`
-/// associated with `Dst` to the `StructValue` associated with `Src` (if these
+/// associated with `Src` to the `StructValue` associated with `Dst` (if these
 /// `StructValue`s exist).
 ///
 /// If there is a `StructValue` associated with `Dst` in the environment, this
@@ -52,6 +52,11 @@ void copyRecord(AggregateStorageLocation &Src, 
AggregateStorageLocation &Dst,
 /// refer to the same storage location. If `StructValue`s are associated with
 /// `Loc1` and `Loc2`, it also compares the properties on those `StructValue`s.
 ///
+/// Note on how to interpret the result:
+/// - If this returns true, the records are guaranteed to be equal at runtime.
+/// - If this returns false, the records may still be equal at runtime; our
+///   analysis merely cannot guarantee that they will be equal.
+///
 /// Requirements:
 ///
 ///  `Src` and `Dst` must have the same canonical unqualified type.

diff  --git a/clang/unittests/Analysis/FlowSensitive/RecordOpsTest.cpp 
b/clang/unittests/Analysis/FlowSensitive/RecordOpsTest.cpp
index a89011e423cd27..1f5fce1f2dd467 100644
--- a/clang/unittests/Analysis/FlowSensitive/RecordOpsTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/RecordOpsTest.cpp
@@ -16,14 +16,18 @@ namespace dataflow {
 namespace test {
 namespace {
 
-template 
-void runDataflow(llvm::StringRef Code, VerifyResultsT VerifyResults,
- LangStandard::Kind Std = LangStandard::lang_cxx17,
- llvm::StringRef TargetFun = "target") {
+void runDataflow(
+llvm::StringRef Code,
+std::function<
+void(const llvm::StringMap> &,
+ ASTContext &)>
+VerifyResults,
+LangStandard::Kind Std = LangStandard::lang_cxx17,
+llvm::StringRef TargetFun = "target") {
   ASSERT_THAT_ERROR(
-  runDataflowReturnError(Code, VerifyResults,
- DataflowAnalysisOptions{BuiltinOptions{}}, Std,
- TargetFun),
+  checkDataflowWithNoopAnalysis(Code, VerifyResults,
+DataflowAnalysisOptions{BuiltinOptions{}},
+Std, TargetFun),
   llvm::Succeeded());
 }
 

diff  --git a/clang/unittests/Analysis/FlowSensitive/TestingSupport.cpp 
b/clang/unittests/Analysis/FlowSensitive/TestingSupport.cpp
index a88b8d88c74c07..72bdfee26fe7f3 100644
--- a/clang/unittests/Analysis/FlowSensitive/TestingSupport.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/TestingSupport.cpp
@@ -153,6 +153,43 @@ test::buildStatementToAnnotationMapping(const FunctionDecl 
*Func,
   return Result;
 }
 
+llvm::Error test::checkDataflowWithNoopAnalysis(
+llvm::StringRef Code,
+std::function<
+void(const llvm::StringMap> &,
+ ASTContext &)>
+VerifyResults,
+DataflowAnalysisOptions Options, LangStandard::Kind Std,
+llvm::StringRef TargetFun) {
+  using ast_matchers::hasName;
+  llvm::SmallVector ASTBuildArgs = {
+  // -fnodelayed-template-parsing is the default everywhere but on Windows.
+  // Set it explicitly so that tests behave the same on Windows as on other
+  // platforms.
+  "-fsyntax-only", "-fno-delayed-template-parsing",
+  "-std=" +
+  std::string(LangStandard::getLangStandardForKind(Std).getName())};
+  AnalysisInputs AI(
+  Code, hasName(TargetFun),
+  [UseBuiltinModel = Options.BuiltinOpts.has

[PATCH] D154339: [clang][dataflow] Make `runDataflowReturnError()` a non-template function.

[Martin Böhme 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 rGd0be47c51cfd: [clang][dataflow] Make 
`runDataflowReturnError()` a non-template function. (authored by mboehme).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154339

Files:
  clang/include/clang/Analysis/FlowSensitive/RecordOps.h
  clang/unittests/Analysis/FlowSensitive/RecordOpsTest.cpp
  clang/unittests/Analysis/FlowSensitive/TestingSupport.cpp
  clang/unittests/Analysis/FlowSensitive/TestingSupport.h
  clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -38,21 +38,28 @@
 using ::testing::NotNull;
 using ::testing::UnorderedElementsAre;
 
-template 
-void runDataflow(llvm::StringRef Code, VerifyResultsT VerifyResults,
- DataflowAnalysisOptions Options,
- LangStandard::Kind Std = LangStandard::lang_cxx17,
- llvm::StringRef TargetFun = "target") {
-  ASSERT_THAT_ERROR(
-  runDataflowReturnError(Code, VerifyResults, Options, Std, TargetFun),
-  llvm::Succeeded());
-}
-
-template 
-void runDataflow(llvm::StringRef Code, VerifyResultsT VerifyResults,
- LangStandard::Kind Std = LangStandard::lang_cxx17,
- bool ApplyBuiltinTransfer = true,
- llvm::StringRef TargetFun = "target") {
+void runDataflow(
+llvm::StringRef Code,
+std::function<
+void(const llvm::StringMap> &,
+ ASTContext &)>
+VerifyResults,
+DataflowAnalysisOptions Options,
+LangStandard::Kind Std = LangStandard::lang_cxx17,
+llvm::StringRef TargetFun = "target") {
+  ASSERT_THAT_ERROR(checkDataflowWithNoopAnalysis(Code, VerifyResults, Options,
+  Std, TargetFun),
+llvm::Succeeded());
+}
+
+void runDataflow(
+llvm::StringRef Code,
+std::function<
+void(const llvm::StringMap> &,
+ ASTContext &)>
+VerifyResults,
+LangStandard::Kind Std = LangStandard::lang_cxx17,
+bool ApplyBuiltinTransfer = true, llvm::StringRef TargetFun = "target") {
   runDataflow(Code, std::move(VerifyResults),
   {ApplyBuiltinTransfer ? BuiltinOptions{}
 : std::optional()},
@@ -2682,7 +2689,7 @@
 void target() {}
   )";
   ASSERT_THAT_ERROR(
-  runDataflowReturnError(
+  checkDataflowWithNoopAnalysis(
   Code,
   [](const llvm::StringMap> &Results,
  ASTContext &ASTCtx) {},
@@ -2698,7 +2705,7 @@
 };
   )";
   ASSERT_THAT_ERROR(
-  runDataflowReturnError(
+  checkDataflowWithNoopAnalysis(
   Code,
   [](const llvm::StringMap> &Results,
  ASTContext &ASTCtx) {},
Index: clang/unittests/Analysis/FlowSensitive/TestingSupport.h
===
--- clang/unittests/Analysis/FlowSensitive/TestingSupport.h
+++ clang/unittests/Analysis/FlowSensitive/TestingSupport.h
@@ -387,40 +387,15 @@
 
 /// Runs dataflow on `Code` with a `NoopAnalysis` and calls `VerifyResults` to
 /// verify the results.
-template 
-llvm::Error
-runDataflowReturnError(llvm::StringRef Code, VerifyResultsT VerifyResults,
-   DataflowAnalysisOptions Options,
-   LangStandard::Kind Std = LangStandard::lang_cxx17,
-   llvm::StringRef TargetFun = "target") {
-  using ast_matchers::hasName;
-  llvm::SmallVector ASTBuildArgs = {
-  // -fnodelayed-template-parsing is the default everywhere but on Windows.
-  // Set it explicitly so that tests behave the same on Windows as on other
-  // platforms.
-  "-fsyntax-only", "-fno-delayed-template-parsing",
-  "-std=" +
-  std::string(LangStandard::getLangStandardForKind(Std).getName())};
-  AnalysisInputs AI(
-  Code, hasName(TargetFun),
-  [UseBuiltinModel = Options.BuiltinOpts.has_value()](ASTContext &C,
-  Environment &Env) {
-return NoopAnalysis(
-C,
-DataflowAnalysisOptions{
-UseBuiltinModel ? Env.getDataflowAnalysisContext().getOptions()
-: std::optional()});
-  });
-  AI.ASTBuildArgs = ASTBuildArgs;
-  if (Options.BuiltinOpts)
-AI.BuiltinOptions = *Options.BuiltinOpts;
-  return checkDataflow(
-  std::move(AI),
-  /*VerifyResults=*/
-  [&VerifyResults](
-  const llvm::StringMap> &Results,
-  const AnalysisOutputs &AO) { VerifyResults(Results, AO.ASTCtx); });
-}
+llvm::Error checkDataflowWithNo

[PATCH] D152770: [clang][ExtractAPI] Add support for Objective-C categories

[R4444 via Phabricator via cfe-commits]

Ruturaj4 created this revision.
Herald added a reviewer: ributzka.
Herald added a project: All.
Ruturaj4 added a reviewer: dang.
Ruturaj4 edited the summary of this revision.
Ruturaj4 edited the summary of this revision.
Ruturaj4 updated this revision to Diff 537064.
Ruturaj4 added a comment.
Ruturaj4 edited the summary of this revision.
Ruturaj4 published this revision for review.
Herald added subscribers: cfe-commits, wangpc.
Herald added a project: clang.



1. Updating D152770 : [clang][ExtractAPI] Add 
support for Objective-C categories #
2. Enter a brief description of the changes included in this update.
3. The first line is used as subject, next lines as comment. #
4. If you intended to create a new revision, use:
5. $ arc diff --create

Introducing support for Objective-C Categories for ExtractAPI. The suuport for 
Objective-C Categories is currently lacking
in terms of categories that extend a type defined in current module and 
categories that extend types that belong to external
modules such as NSString. Thus the objective of this patch is two fold, to fix 
former and the later.

This is achieved by introducing two new symbols ->

objective-c.module.extension.
objective-c.class.extension.
Note that the extension represents objective-c "category" (orienting well with 
Swift's Extension, which in spite that fact
that is different, however "broadly" serves similar purpose). The original idea 
is inspired by Max's @theMomax initial
post - 
https://forums.swift.org/t/symbol-graph-adaptions-for-documenting-extensions-to-external-types-in-docc/56684,
and Daniel's helpful comments and feedback. The implementation nonetheless is 
different serving purpose for this project.


Introducing support for Objective-C Categories for ExtractAPI. The suuport for 
Objective-C Categories is currently lacking
in terms of categories that extend a type defined in current module and 
categories that extend types that belong to external
modules such as `NSString`. Thus the objective of this patch is two fold, to 
fix former and the later.

This is achieved by introducing two new symbols ->

1. `objective-c.module.extension`.
2. `objective-c.class.extension`.

Note that the extension represents objective-c "category" (orienting well with 
Swift's Extension, which in spite that fact
that is different, however "broadly" serves similar purpose). The original idea 
is inspired by Max's `@theMomax` initial
post - 
https://forums.swift.org/t/symbol-graph-adaptions-for-documenting-extensions-to-external-types-in-docc/56684,
and Daniel's helpful comments and feedback. The implementation nonetheless is 
different serving purpose for this project.

The following diagram well represents the purpose ->
F28127510: llvm_categories.png 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D152770

Files:
  clang/include/clang/ExtractAPI/API.h
  clang/include/clang/ExtractAPI/ExtractAPIVisitor.h
  clang/include/clang/ExtractAPI/Serialization/SerializerBase.h
  clang/include/clang/ExtractAPI/Serialization/SymbolGraphSerializer.h
  clang/lib/ExtractAPI/API.cpp
  clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
  clang/test/ExtractAPI/objc_module_category.m
  clang/test/ExtractAPI/objc_various_categories.m

Index: clang/test/ExtractAPI/objc_various_categories.m
===
--- /dev/null
+++ clang/test/ExtractAPI/objc_various_categories.m
@@ -0,0 +1,519 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: sed -e "s@INPUT_DIR@%{/t:regex_replacement}@g" \
+// RUN: %t/reference.output.json.in >> %t/reference.output.json
+// RUN: %clang -extract-api -x objective-c-header \
+// RUN: -target arm64-apple-macosx \
+// RUN: %t/myclass_1.h \
+// RUN: %t/input.h -o %t/output.json | FileCheck -allow-empty %s
+
+// Generator version is not consistent across test runs, normalize it.
+// RUN: sed -e "s@\"generator\": \".*\"@\"generator\": \"?\"@g" \
+// RUN: %t/output.json >> %t/output-normalized.json
+// RUN: diff %t/reference.output.json %t/output-normalized.json
+
+// CHECK-NOT: error:
+// CHECK-NOT: warning:
+
+//--- input.h
+#import 
+#import "myclass_1.h"
+#import "myclass_2.h"
+
+@interface MyClass1 (MyCategory1)
+- (int) SomeMethod;
+@end
+
+@interface MyClass2 (MyCategory2)
+- (int) SomeMethod;
+@end
+
+@interface NSString (Category)
+-(void) StringMethod;
+@end
+
+//--- myclass_1.h
+@interface MyClass1
+@end
+
+//--- myclass_2.h
+@interface MyClass2
+@end
+
+//--- reference.output.json.in
+{
+  "metadata": {
+"formatVersion": {
+  "major": 0,
+  "minor": 5,
+  "patch": 3
+},
+"generator": "?"
+  },
+  "module": {
+"name": "",
+"platform": {
+  "architecture": "arm64",
+  "operatingSystem": {
+"minimumVersion": {
+  "major": 11,
+  "minor": 0,
+  "patch": 0
+},
+"name": "macosx"
+  },
+  "vendor": "a

[PATCH] D154437: [analyzer][NFC] Make AnalyzerConfig round-tripping deterministic

[Balázs Benics via Phabricator via cfe-commits]

steakhal added a comment.

In D154437#4471225 , @jansvoboda11 
wrote:

> Has this not been addressed by D142861  
> already?

Hm, The same failure. This must be related. I'll have a look.
But, you can try it yourself that the issue still remains:
Add a new StringRef option to the AnalyzerOptions.def and it's likely it will 
break the `clang/test/Analysis/shallow-mode.m` test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154437

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

[clang] 7a72ce9 - Revert "[dataflow] Add dedicated representation of boolean formulas"

[Tom Weaver via cfe-commits]

Author: Tom Weaver
Date: 2023-07-04T14:05:54+01:00
New Revision: 7a72ce98224be76d9328e65eee472381f7c8e7fe

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

LOG: Revert "[dataflow] Add dedicated representation of boolean formulas"

This reverts commit 2fd614efc1bb9c27f1bc6c3096c60a7fe121e274.

Commit caused failures on the following two build bots:
  http://45.33.8.238/win/80815/step_7.txt
  https://lab.llvm.org/buildbot/#/builders/139/builds/44269

Added: 


Modified: 
clang/include/clang/Analysis/FlowSensitive/Arena.h
clang/include/clang/Analysis/FlowSensitive/DebugSupport.h
clang/include/clang/Analysis/FlowSensitive/Solver.h
clang/include/clang/Analysis/FlowSensitive/WatchedLiteralsSolver.h
clang/lib/Analysis/FlowSensitive/Arena.cpp
clang/lib/Analysis/FlowSensitive/CMakeLists.txt
clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
clang/lib/Analysis/FlowSensitive/DebugSupport.cpp
clang/lib/Analysis/FlowSensitive/WatchedLiteralsSolver.cpp
clang/unittests/Analysis/FlowSensitive/ArenaTest.cpp
clang/unittests/Analysis/FlowSensitive/DebugSupportTest.cpp
clang/unittests/Analysis/FlowSensitive/SolverTest.cpp
clang/unittests/Analysis/FlowSensitive/TestingSupport.h

Removed: 
clang/include/clang/Analysis/FlowSensitive/Formula.h
clang/lib/Analysis/FlowSensitive/Formula.cpp



diff  --git a/clang/include/clang/Analysis/FlowSensitive/Arena.h 
b/clang/include/clang/Analysis/FlowSensitive/Arena.h
index b6c62e76246254..83b4ddeec02564 100644
--- a/clang/include/clang/Analysis/FlowSensitive/Arena.h
+++ b/clang/include/clang/Analysis/FlowSensitive/Arena.h
@@ -8,7 +8,6 @@
 #ifndef LLVM_CLANG_ANALYSIS_FLOWSENSITIVE__ARENA_H
 #define LLVM_CLANG_ANALYSIS_FLOWSENSITIVE__ARENA_H
 
-#include "clang/Analysis/FlowSensitive/Formula.h"
 #include "clang/Analysis/FlowSensitive/StorageLocation.h"
 #include "clang/Analysis/FlowSensitive/Value.h"
 #include 
@@ -105,17 +104,7 @@ class Arena {
 return create();
   }
 
-  /// Gets the boolean formula equivalent of a BoolValue.
-  /// Each unique Top values is translated to an Atom.
-  /// TODO: migrate to storing Formula directly in Values instead.
-  const Formula &getFormula(const BoolValue &);
-
-  /// Returns a new atomic boolean variable, distinct from any other.
-  Atom makeAtom() { return static_cast(NextAtom++); };
-
 private:
-  llvm::BumpPtrAllocator Alloc;
-
   // Storage for the state of a program.
   std::vector> Locs;
   std::vector> Vals;
@@ -133,9 +122,6 @@ class Arena {
   llvm::DenseMap, BiconditionalValue *>
   BiconditionalVals;
 
-  llvm::DenseMap ValToFormula;
-  unsigned NextAtom = 0;
-
   AtomicBoolValue &TrueVal;
   AtomicBoolValue &FalseVal;
 };

diff  --git a/clang/include/clang/Analysis/FlowSensitive/DebugSupport.h 
b/clang/include/clang/Analysis/FlowSensitive/DebugSupport.h
index 6b9f3681490af1..360786b02729f2 100644
--- a/clang/include/clang/Analysis/FlowSensitive/DebugSupport.h
+++ b/clang/include/clang/Analysis/FlowSensitive/DebugSupport.h
@@ -19,6 +19,7 @@
 
 #include "clang/Analysis/FlowSensitive/Solver.h"
 #include "clang/Analysis/FlowSensitive/Value.h"
+#include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/StringRef.h"
 
 namespace clang {
@@ -27,9 +28,52 @@ namespace dataflow {
 /// Returns a string representation of a value kind.
 llvm::StringRef debugString(Value::Kind Kind);
 
+/// Returns a string representation of a boolean assignment to true or false.
+llvm::StringRef debugString(Solver::Result::Assignment Assignment);
+
 /// Returns a string representation of the result status of a SAT check.
 llvm::StringRef debugString(Solver::Result::Status Status);
 
+/// Returns a string representation for the boolean value `B`.
+///
+/// Atomic booleans appearing in the boolean value `B` are assigned to labels
+/// either specified in `AtomNames` or created by default rules as B0, B1, ...
+///
+/// Requirements:
+///
+///   Names assigned to atoms should not be repeated in `AtomNames`.
+std::string debugString(
+const BoolValue &B,
+llvm::DenseMap AtomNames = {{}});
+
+/// Returns a string representation for `Constraints` - a collection of boolean
+/// formulas.
+///
+/// Atomic booleans appearing in the boolean value `Constraints` are assigned 
to
+/// labels either specified in `AtomNames` or created by default rules as B0,
+/// B1, ...
+///
+/// Requirements:
+///
+///   Names assigned to atoms should not be repeated in `AtomNames`.
+std::string debugString(
+const llvm::ArrayRef Constraints,
+llvm::DenseMap AtomNames = {{}});
+
+/// Returns a string representation for `Constraints` - a collection of boolean
+/// formulas and the `Result` of satisfiability checking.
+///
+/// Atomic booleans appearing in `Constraints` and `Result` are assigned to

[PATCH] D153366: [dataflow] Add dedicated representation of boolean formulas

[Tom Weaver via Phabricator via cfe-commits]

TWeaver added a comment.

My apologies but I've had to revert this change for now until the author can 
address the buildbot failures.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153366

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

[PATCH] D154417: [libc++] Disable tree invariant check in asserts mode

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

In D154417#4471141 , @alexfh wrote:

> Given the comment in https://reviews.llvm.org/D153672#4468348, I think, this 
> should be fine to submit.

Sounds good to me. Happy to follow up if libc++ maintainers have more input.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154417

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

[PATCH] D154417: [libc++] Disable tree invariant check in asserts mode

[Hans Wennborg via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rG1e35e93e30c2: [libc++] Disable tree invariant check in 
asserts mode (authored by hans).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154417

Files:
  libcxx/include/__tree


Index: libcxx/include/__tree
===
--- libcxx/include/__tree
+++ libcxx/include/__tree
@@ -376,7 +376,8 @@
 {
 _LIBCPP_ASSERT_UNCATEGORIZED(__root != nullptr, "Root node should not be 
null");
 _LIBCPP_ASSERT_UNCATEGORIZED(__z != nullptr, "The node to remove should 
not be null");
-_LIBCPP_ASSERT_UNCATEGORIZED(std::__tree_invariant(__root), "The tree 
invariants should hold");
+// TODO: Use in the new debug mode:
+// _LIBCPP_DEBUG_ASSERT(std::__tree_invariant(__root), "The tree 
invariants should hold");
 // __z will be removed from the tree.  Client still needs to 
destruct/deallocate it
 // __y is either __z, or if __z has two children, __tree_next(__z).
 // __y will have at most one child.


Index: libcxx/include/__tree
===
--- libcxx/include/__tree
+++ libcxx/include/__tree
@@ -376,7 +376,8 @@
 {
 _LIBCPP_ASSERT_UNCATEGORIZED(__root != nullptr, "Root node should not be null");
 _LIBCPP_ASSERT_UNCATEGORIZED(__z != nullptr, "The node to remove should not be null");
-_LIBCPP_ASSERT_UNCATEGORIZED(std::__tree_invariant(__root), "The tree invariants should hold");
+// TODO: Use in the new debug mode:
+// _LIBCPP_DEBUG_ASSERT(std::__tree_invariant(__root), "The tree invariants should hold");
 // __z will be removed from the tree.  Client still needs to destruct/deallocate it
 // __y is either __z, or if __z has two children, __tree_next(__z).
 // __y will have at most one child.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D154443: [clangd] Downgrade deprecated warnings to hints

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

This tries to improve adoption of noisy warnings in existing codebases.
Hints have a lot less visual clutter in most of the editors, and DiagnosticTags
already imply a custom decorations per LSP.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D154443

Files:
  clang-tools-extra/clangd/Diagnostics.cpp
  clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp

Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -38,6 +38,7 @@
 #include "gtest/gtest.h"
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -1879,6 +1880,42 @@
 withTag(DiagnosticTag::Deprecated);
 }
 
+TEST(Diagnostics, DeprecatedDiagsAreHints) {
+  ClangdDiagnosticOptions Opts;
+  std::optional Diag;
+  clangd::Diag D;
+  D.Range = {pos(1, 2), pos(3, 4)};
+  D.InsideMainFile = true;
+
+  // Downgrade warnings with deprecated tags to remark.
+  D.Tags = {Deprecated};
+  D.Severity = DiagnosticsEngine::Warning;
+  toLSPDiags(D, {}, Opts,
+ [&](clangd::Diagnostic LSPDiag, ArrayRef) {
+   Diag = std::move(LSPDiag);
+ });
+  EXPECT_EQ(Diag->severity, getSeverity(DiagnosticsEngine::Remark));
+  Diag.reset();
+
+  // Preserve errors.
+  D.Severity = DiagnosticsEngine::Error;
+  toLSPDiags(D, {}, Opts,
+ [&](clangd::Diagnostic LSPDiag, ArrayRef) {
+   Diag = std::move(LSPDiag);
+ });
+  EXPECT_EQ(Diag->severity, getSeverity(DiagnosticsEngine::Error));
+  Diag.reset();
+
+  // No-op without tag.
+  D.Tags = {};
+  D.Severity = DiagnosticsEngine::Warning;
+  toLSPDiags(D, {}, Opts,
+ [&](clangd::Diagnostic LSPDiag, ArrayRef) {
+   Diag = std::move(LSPDiag);
+ });
+  EXPECT_EQ(Diag->severity, getSeverity(DiagnosticsEngine::Warning));
+}
+
 TEST(DiagnosticsTest, IncludeCleaner) {
   Annotations Test(R"cpp(
 $fix[[  $diag[[#include "unused.h"]]
Index: clang-tools-extra/clangd/Diagnostics.cpp
===
--- clang-tools-extra/clangd/Diagnostics.cpp
+++ clang-tools-extra/clangd/Diagnostics.cpp
@@ -15,24 +15,35 @@
 #include "clang/Basic/AllDiagnostics.h" // IWYU pragma: keep
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/DiagnosticIDs.h"
+#include "clang/Basic/LLVM.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
+#include "clang/Basic/TokenKinds.h"
 #include "clang/Lex/Lexer.h"
 #include "clang/Lex/Token.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/STLFunctionalExtras.h"
 #include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/StringSet.h"
 #include "llvm/ADT/Twine.h"
+#include "llvm/Support/ErrorHandling.h"
+#include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Support/SourceMgr.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
 #include 
-#include 
 #include 
+#include 
+#include 
+#include 
+#include 
 #include 
 
 namespace clang {
@@ -460,6 +471,14 @@
 llvm::function_ref)> OutFn) {
   clangd::Diagnostic Main;
   Main.severity = getSeverity(D.Severity);
+  // We downgrade severity for certain noisy warnings, like deprecated
+  // declartions. These already have visible decorations inside the editor and
+  // most users find the extra clutter in the UI (gutter, minimap, diagnostics
+  // views) overwhelming.
+  if (D.Severity == DiagnosticsEngine::Warning) {
+if (llvm::is_contained(D.Tags, DiagnosticTag::Deprecated))
+  Main.severity = getSeverity(DiagnosticsEngine::Remark);
+  }
 
   // Main diagnostic should always refer to a range inside main file. If a
   // diagnostic made it so for, it means either itself or one of its notes is
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D154443: [clangd] Downgrade deprecated warnings to hints

[Sam McCall via Phabricator via cfe-commits]

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

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154443

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

[PATCH] D153366: [dataflow] Add dedicated representation of boolean formulas

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In D153366#4471320 , @TWeaver wrote:

> My apologies but I've had to revert this change for now until the author can 
> address the buildbot failures.

Thanks for the revert, and sorry for the disruption - I expected to be around 
to keep an eye on this, but was away.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153366

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

[clang-tools-extra] a59b24b - [clangd] Downgrade deprecated warnings to hints

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2023-07-04T16:07:08+02:00
New Revision: a59b24be47ed6263c254d168567b9ebba391fac9

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

LOG: [clangd] Downgrade deprecated warnings to hints

This tries to improve adoption of noisy warnings in existing codebases.
Hints have a lot less visual clutter in most of the editors, and DiagnosticTags
already imply a custom decorations per LSP.

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

Added: 


Modified: 
clang-tools-extra/clangd/Diagnostics.cpp
clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/Diagnostics.cpp 
b/clang-tools-extra/clangd/Diagnostics.cpp
index bae528a105c87d..704e61b1e4dd79 100644
--- a/clang-tools-extra/clangd/Diagnostics.cpp
+++ b/clang-tools-extra/clangd/Diagnostics.cpp
@@ -15,24 +15,35 @@
 #include "clang/Basic/AllDiagnostics.h" // IWYU pragma: keep
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/DiagnosticIDs.h"
+#include "clang/Basic/LLVM.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
+#include "clang/Basic/TokenKinds.h"
 #include "clang/Lex/Lexer.h"
 #include "clang/Lex/Token.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/STLFunctionalExtras.h"
 #include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/StringSet.h"
 #include "llvm/ADT/Twine.h"
+#include "llvm/Support/ErrorHandling.h"
+#include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Support/SourceMgr.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
 #include 
-#include 
 #include 
+#include 
+#include 
+#include 
+#include 
 #include 
 
 namespace clang {
@@ -460,6 +471,14 @@ void toLSPDiags(
 llvm::function_ref)> OutFn) {
   clangd::Diagnostic Main;
   Main.severity = getSeverity(D.Severity);
+  // We downgrade severity for certain noisy warnings, like deprecated
+  // declartions. These already have visible decorations inside the editor and
+  // most users find the extra clutter in the UI (gutter, minimap, diagnostics
+  // views) overwhelming.
+  if (D.Severity == DiagnosticsEngine::Warning) {
+if (llvm::is_contained(D.Tags, DiagnosticTag::Deprecated))
+  Main.severity = getSeverity(DiagnosticsEngine::Remark);
+  }
 
   // Main diagnostic should always refer to a range inside main file. If a
   // diagnostic made it so for, it means either itself or one of its notes is

diff  --git a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp 
b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
index 5f36f28becb0da..51ffa45dbc8f6f 100644
--- a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -38,6 +38,7 @@
 #include "gtest/gtest.h"
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -1879,6 +1880,42 @@ TEST(Diagnostics, Tags) {
 withTag(DiagnosticTag::Deprecated);
 }
 
+TEST(Diagnostics, DeprecatedDiagsAreHints) {
+  ClangdDiagnosticOptions Opts;
+  std::optional Diag;
+  clangd::Diag D;
+  D.Range = {pos(1, 2), pos(3, 4)};
+  D.InsideMainFile = true;
+
+  // Downgrade warnings with deprecated tags to remark.
+  D.Tags = {Deprecated};
+  D.Severity = DiagnosticsEngine::Warning;
+  toLSPDiags(D, {}, Opts,
+ [&](clangd::Diagnostic LSPDiag, ArrayRef) {
+   Diag = std::move(LSPDiag);
+ });
+  EXPECT_EQ(Diag->severity, getSeverity(DiagnosticsEngine::Remark));
+  Diag.reset();
+
+  // Preserve errors.
+  D.Severity = DiagnosticsEngine::Error;
+  toLSPDiags(D, {}, Opts,
+ [&](clangd::Diagnostic LSPDiag, ArrayRef) {
+   Diag = std::move(LSPDiag);
+ });
+  EXPECT_EQ(Diag->severity, getSeverity(DiagnosticsEngine::Error));
+  Diag.reset();
+
+  // No-op without tag.
+  D.Tags = {};
+  D.Severity = DiagnosticsEngine::Warning;
+  toLSPDiags(D, {}, Opts,
+ [&](clangd::Diagnostic LSPDiag, ArrayRef) {
+   Diag = std::move(LSPDiag);
+ });
+  EXPECT_EQ(Diag->severity, getSeverity(DiagnosticsEngine::Warning));
+}
+
 TEST(DiagnosticsTest, IncludeCleaner) {
   Annotations Test(R"cpp(
 $fix[[  $diag[[#include "unused.h"]]



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

[PATCH] D154443: [clangd] Downgrade deprecated warnings to hints

[Kadir Cetinkaya via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGa59b24be47ed: [clangd] Downgrade deprecated warnings to 
hints (authored by kadircet).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154443

Files:
  clang-tools-extra/clangd/Diagnostics.cpp
  clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp

Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -38,6 +38,7 @@
 #include "gtest/gtest.h"
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -1879,6 +1880,42 @@
 withTag(DiagnosticTag::Deprecated);
 }
 
+TEST(Diagnostics, DeprecatedDiagsAreHints) {
+  ClangdDiagnosticOptions Opts;
+  std::optional Diag;
+  clangd::Diag D;
+  D.Range = {pos(1, 2), pos(3, 4)};
+  D.InsideMainFile = true;
+
+  // Downgrade warnings with deprecated tags to remark.
+  D.Tags = {Deprecated};
+  D.Severity = DiagnosticsEngine::Warning;
+  toLSPDiags(D, {}, Opts,
+ [&](clangd::Diagnostic LSPDiag, ArrayRef) {
+   Diag = std::move(LSPDiag);
+ });
+  EXPECT_EQ(Diag->severity, getSeverity(DiagnosticsEngine::Remark));
+  Diag.reset();
+
+  // Preserve errors.
+  D.Severity = DiagnosticsEngine::Error;
+  toLSPDiags(D, {}, Opts,
+ [&](clangd::Diagnostic LSPDiag, ArrayRef) {
+   Diag = std::move(LSPDiag);
+ });
+  EXPECT_EQ(Diag->severity, getSeverity(DiagnosticsEngine::Error));
+  Diag.reset();
+
+  // No-op without tag.
+  D.Tags = {};
+  D.Severity = DiagnosticsEngine::Warning;
+  toLSPDiags(D, {}, Opts,
+ [&](clangd::Diagnostic LSPDiag, ArrayRef) {
+   Diag = std::move(LSPDiag);
+ });
+  EXPECT_EQ(Diag->severity, getSeverity(DiagnosticsEngine::Warning));
+}
+
 TEST(DiagnosticsTest, IncludeCleaner) {
   Annotations Test(R"cpp(
 $fix[[  $diag[[#include "unused.h"]]
Index: clang-tools-extra/clangd/Diagnostics.cpp
===
--- clang-tools-extra/clangd/Diagnostics.cpp
+++ clang-tools-extra/clangd/Diagnostics.cpp
@@ -15,24 +15,35 @@
 #include "clang/Basic/AllDiagnostics.h" // IWYU pragma: keep
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/DiagnosticIDs.h"
+#include "clang/Basic/LLVM.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
+#include "clang/Basic/TokenKinds.h"
 #include "clang/Lex/Lexer.h"
 #include "clang/Lex/Token.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/STLFunctionalExtras.h"
 #include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/StringSet.h"
 #include "llvm/ADT/Twine.h"
+#include "llvm/Support/ErrorHandling.h"
+#include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Support/SourceMgr.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
 #include 
-#include 
 #include 
+#include 
+#include 
+#include 
+#include 
 #include 
 
 namespace clang {
@@ -460,6 +471,14 @@
 llvm::function_ref)> OutFn) {
   clangd::Diagnostic Main;
   Main.severity = getSeverity(D.Severity);
+  // We downgrade severity for certain noisy warnings, like deprecated
+  // declartions. These already have visible decorations inside the editor and
+  // most users find the extra clutter in the UI (gutter, minimap, diagnostics
+  // views) overwhelming.
+  if (D.Severity == DiagnosticsEngine::Warning) {
+if (llvm::is_contained(D.Tags, DiagnosticTag::Deprecated))
+  Main.severity = getSeverity(DiagnosticsEngine::Remark);
+  }
 
   // Main diagnostic should always refer to a range inside main file. If a
   // diagnostic made it so for, it means either itself or one of its notes is
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 1c8c5a8 - [clang][Interp][NFC] Merge two if statements

[Timm Bäder via cfe-commits]

Author: Timm Bäder
Date: 2023-07-04T16:16:17+02:00
New Revision: 1c8c5a89c59cee16d6500a4e83c8c29b8a4a37a4

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

LOG: [clang][Interp][NFC] Merge two if statements

Added: 


Modified: 
clang/lib/AST/Interp/Interp.cpp

Removed: 




diff  --git a/clang/lib/AST/Interp/Interp.cpp b/clang/lib/AST/Interp/Interp.cpp
index f646e876554cad..d68accdf3bf850 100644
--- a/clang/lib/AST/Interp/Interp.cpp
+++ b/clang/lib/AST/Interp/Interp.cpp
@@ -296,12 +296,10 @@ bool CheckInit(InterpState &S, CodePtr OpPC, const 
Pointer &Ptr) {
 
 bool CheckCallable(InterpState &S, CodePtr OpPC, const Function *F) {
 
-  if (F->isVirtual()) {
-if (!S.getLangOpts().CPlusPlus20) {
-  const SourceLocation &Loc = S.Current->getLocation(OpPC);
-  S.CCEDiag(Loc, diag::note_constexpr_virtual_call);
-  return false;
-}
+  if (F->isVirtual() && !S.getLangOpts().CPlusPlus20) {
+const SourceLocation &Loc = S.Current->getLocation(OpPC);
+S.CCEDiag(Loc, diag::note_constexpr_virtual_call);
+return false;
   }
 
   if (!F->isConstexpr()) {



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

[clang] 843ff75 - [clang][Interp][NFC] Return std::nullopt explicitly

[Timm Bäder via cfe-commits]

Author: Timm Bäder
Date: 2023-07-04T16:16:18+02:00
New Revision: 843ff7581408a02e852c0f1f7ebf176cabbc7527

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

LOG: [clang][Interp][NFC] Return std::nullopt explicitly

Added: 


Modified: 
clang/lib/AST/Interp/Program.cpp

Removed: 




diff  --git a/clang/lib/AST/Interp/Program.cpp 
b/clang/lib/AST/Interp/Program.cpp
index f6f378dd49f167..a9749022ee677b 100644
--- a/clang/lib/AST/Interp/Program.cpp
+++ b/clang/lib/AST/Interp/Program.cpp
@@ -120,7 +120,7 @@ std::optional Program::getGlobal(const ValueDecl 
*VD) {
   // Map the decl to the existing index.
   if (Index) {
 GlobalIndices[VD] = *Index;
-return {};
+return std::nullopt;
   }
 
   return Index;
@@ -135,7 +135,7 @@ std::optional Program::getOrCreateGlobal(const 
ValueDecl *VD,
 GlobalIndices[VD] = *Idx;
 return Idx;
   }
-  return {};
+  return std::nullopt;
 }
 
 std::optional Program::getOrCreateDummy(const ParmVarDecl *PD) {
@@ -154,7 +154,7 @@ std::optional Program::getOrCreateDummy(const 
ParmVarDecl *PD) {
 DummyParams[PD] = *Idx;
 return Idx;
   }
-  return {};
+  return std::nullopt;
 }
 
 std::optional Program::createGlobal(const ValueDecl *VD,
@@ -173,7 +173,7 @@ std::optional Program::createGlobal(const 
ValueDecl *VD,
   GlobalIndices[P] = *Idx;
 return *Idx;
   }
-  return {};
+  return std::nullopt;
 }
 
 std::optional Program::createGlobal(const Expr *E) {
@@ -194,7 +194,7 @@ std::optional Program::createGlobal(const DeclTy 
&D, QualType Ty,
 IsTemporary);
   }
   if (!Desc)
-return {};
+return std::nullopt;
 
   // Allocate a block for storage.
   unsigned I = Globals.size();



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

[clang] 044be8f - [clang][Interp][NFC] Add some missing const qualifiers

[Timm Bäder via cfe-commits]

Author: Timm Bäder
Date: 2023-07-04T16:16:17+02:00
New Revision: 044be8f5d12fcb35cb5c30fa05d95b1abbf19232

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

LOG: [clang][Interp][NFC] Add some missing const qualifiers

Added: 


Modified: 
clang/lib/AST/Interp/Interp.cpp

Removed: 




diff  --git a/clang/lib/AST/Interp/Interp.cpp b/clang/lib/AST/Interp/Interp.cpp
index d68accdf3bf850..e906f65c371c27 100644
--- a/clang/lib/AST/Interp/Interp.cpp
+++ b/clang/lib/AST/Interp/Interp.cpp
@@ -127,7 +127,7 @@ bool CheckExtern(InterpState &S, CodePtr OpPC, const 
Pointer &Ptr) {
 return true;
 
   if (!S.checkingPotentialConstantExpression()) {
-auto *VD = Ptr.getDeclDesc()->asValueDecl();
+const auto *VD = Ptr.getDeclDesc()->asValueDecl();
 const SourceInfo &Loc = S.Current->getSource(OpPC);
 S.FFDiag(Loc, diag::note_constexpr_ltor_non_constexpr, 1) << VD;
 S.Note(VD->getLocation(), diag::note_declared_at);
@@ -314,9 +314,9 @@ bool CheckCallable(InterpState &S, CodePtr OpPC, const 
Function *F) {
 
   // If this function is not constexpr because it is an inherited
   // non-constexpr constructor, diagnose that directly.
-  auto *CD = dyn_cast(DiagDecl);
+  const auto *CD = dyn_cast(DiagDecl);
   if (CD && CD->isInheritingConstructor()) {
-auto *Inherited = CD->getInheritedConstructor().getConstructor();
+const auto *Inherited = CD->getInheritedConstructor().getConstructor();
 if (!Inherited->isConstexpr())
   DiagDecl = CD = Inherited;
   }
@@ -358,7 +358,7 @@ bool CheckThis(InterpState &S, CodePtr OpPC, const Pointer 
&This) {
   const SourceInfo &Loc = S.Current->getSource(OpPC);
 
   bool IsImplicit = false;
-  if (auto *E = dyn_cast_if_present(Loc.asExpr()))
+  if (const auto *E = dyn_cast_if_present(Loc.asExpr()))
 IsImplicit = E->isImplicit();
 
   if (S.getLangOpts().CPlusPlus11)
@@ -402,7 +402,7 @@ static bool CheckArrayInitialized(InterpState &S, CodePtr 
OpPC,
   Pointer ElemPtr = BasePtr.atIndex(I).narrow();
   Result &= CheckFieldsInitialized(S, OpPC, ElemPtr, R);
 }
-  } else if (auto *ElemCAT = dyn_cast(ElemType)) {
+  } else if (const auto *ElemCAT = dyn_cast(ElemType)) {
 for (size_t I = 0; I != NumElems; ++I) {
   Pointer ElemPtr = BasePtr.atIndex(I).narrow();
   Result &= CheckArrayInitialized(S, OpPC, ElemPtr, ElemCAT);



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

[PATCH] D151325: [analyzer] Differentiate lifetime extended temporaries

[Tomasz Kamiński via Phabricator via cfe-commits]

tomasz-kaminski-sonarsource marked an inline comment as done.
tomasz-kaminski-sonarsource added a comment.

In D151325#4470832 , @xazax.hun wrote:

> Overall looks good to me. I think the changes to the notes already make the 
> analyzer more useful so there are some observable benefits to this patch.

Also, user-after-move now correctly detect the use of of move-from lifetime 
extended temporaries.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151325

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

[PATCH] D151325: [analyzer] Differentiate lifetime extended temporaries

[Tomasz Kamiński via Phabricator via cfe-commits]

tomasz-kaminski-sonarsource updated this revision to Diff 537092.
tomasz-kaminski-sonarsource added a comment.

Reformatting and link to revelant core issue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151325

Files:
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/Regions.def
  clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
  clang/lib/StaticAnalyzer/Core/MemRegion.cpp
  clang/lib/StaticAnalyzer/Core/Store.cpp
  clang/test/Analysis/lifetime-extended-regions.cpp
  clang/test/Analysis/stack-addr-ps.cpp
  clang/test/Analysis/use-after-move.cpp

Index: clang/test/Analysis/use-after-move.cpp
===
--- clang/test/Analysis/use-after-move.cpp
+++ clang/test/Analysis/use-after-move.cpp
@@ -613,6 +613,14 @@
   }
 }
 
+void lifeTimeExtendTest() {
+  A&& a = A{};
+  A b = std::move(a); // peaceful-note {{Object is moved}}
+
+  a.foo(); // peaceful-warning {{Method called on moved-from object}}
+   // peaceful-note@-1 {{Method called on moved-from object}}
+}
+
 void interFunTest1(A &a) {
   a.bar(); // peaceful-warning {{Method called on moved-from object 'a'}}
// peaceful-note@-1 {{Method called on moved-from object 'a'}}
Index: clang/test/Analysis/stack-addr-ps.cpp
===
--- clang/test/Analysis/stack-addr-ps.cpp
+++ clang/test/Analysis/stack-addr-ps.cpp
@@ -30,13 +30,13 @@
 
 const int &get_reference2() {
   const int &x = get_value(); // expected-note {{binding reference variable 'x' here}}
-  return x; // expected-warning{{Address of stack memory associated with temporary object of type 'int' returned}} expected-warning {{returning reference to local temporary}}
+  return x; // expected-warning{{Address of stack memory associated with temporary object of type 'int' lifetime extended by local variable 'x' returned to caller}} expected-warning {{returning reference to local temporary}} 
 }
 
 const int &get_reference3() {
   const int &x1 = get_value(); // expected-note {{binding reference variable 'x1' here}}
   const int &x2 = x1; // expected-note {{binding reference variable 'x2' here}}
-  return x2; // expected-warning{{Address of stack memory associated with temporary object of type 'int' returned}} expected-warning {{returning reference to local temporary}}
+  return x2; // expected-warning{{Address of stack memory associated with temporary object of type 'int' lifetime extended by local variable 'x1' returned to caller}} expected-warning {{returning reference to local temporary}}
 }
 
 int global_var;
@@ -60,7 +60,7 @@
 const int *f4() {
   const int &x1 = get_value(); // expected-note {{binding reference variable 'x1' here}}
   const int &x2 = x1; // expected-note {{binding reference variable 'x2' here}}
-  return &x2; // expected-warning{{Address of stack memory associated with temporary object of type 'int' returned}} expected-warning {{returning address of local temporary}}
+  return &x2; // expected-warning{{Address of stack memory associated with temporary object of type 'int' lifetime extended by local variable 'x1' returned to caller}} expected-warning {{returning address of local temporary}}
 }
 
 struct S {
Index: clang/test/Analysis/lifetime-extended-regions.cpp
===
--- /dev/null
+++ clang/test/Analysis/lifetime-extended-regions.cpp
@@ -0,0 +1,171 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core\
+// RUN:-analyzer-checker=debug.ExprInspection\
+// RUN:-Wno-dangling -Wno-c++1z-extensions\
+// RUN:-verify=expected,cpp14\
+// RUN:-x c++ -std=c++14 %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core\
+// RUN:-analyzer-checker=debug.ExprInspection\
+// RUN:-analyzer-config elide-constructors=false\
+// RUN:-Wno-dangling -Wno-c++1z-extensions\
+// RUN:-verify=expected,cpp14\
+// RUN:-x c++ -std=c++14 %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core\
+// RUN:-analyzer-checker=debug.ExprInspection\
+// RUN:-Wno-dangling -verify=expected,cpp17\
+// RUN:-x c++ -std=c++17 %s
+
+template
+void clang_analyzer_dump(T&&) {}
+
+template
+T create() { return T{}; }
+
+template
+T const& select(bool cond, T const& t, T const& u) { return cond ? t : u; }
+
+struct Composite {
+  int x;
+  int y;
+};
+
+struct Derived : Composite {
+  int z;
+};
+
+template
+struct Array {
+  T array[20];
+
+  T&& front() && { r

[PATCH] D153296: [AST] Stop evaluate constant expression if the condition expression which in switch statement contains errors

[Yurong via Phabricator via cfe-commits]

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



Comment at: clang/lib/AST/ExprConstant.cpp:4921
+  // value is.
+  if (isa(E))
+return false;

aaron.ballman wrote:
> yronglin wrote:
> > hokein wrote:
> > > The constant evaluator is not aware of the "error" concept, it is only 
> > > aware of value-dependent -- the general idea behind is that we treat the 
> > > dependent-on-error and dependent-on-template-parameters cases the same, 
> > > they are potentially constant (if we see an expression contains errors, 
> > > it could be constant depending on how the error is resolved), this will 
> > > give us nice recovery and avoid bogus following diagnostics.
> > > 
> > > So, a `RecoveryExpr` should not result in failure when checking for a 
> > > potential constant expression.
> > > 
> > > I think the right fix is to remove the conditional check `if 
> > > (!EvaluateDependentExpr(SS->getCond(), Info))` in `EvaluateSwitch`, and 
> > > return `ESR_Failed` unconditionally (we don't know its value, any 
> > > switch-case anwser will be wrong in some cases). We already do this for 
> > > return-statment, do-statement etc.
> > > 
> > > 
> > Do you mean?
> > ```
> > if (SS->getCond()->isValueDependent()) {
> > EvaluateDependentExpr(SS->getCond(), Info);
> > return ESR_Failed;
> > }
> > ```
> > the general idea behind is that we treat the dependent-on-error and 
> > dependent-on-template-parameters cases the same, they are potentially 
> > constant (if we see an expression contains errors, it could be constant 
> > depending on how the error is resolved), this will give us nice recovery 
> > and avoid bogus following diagnostics.
> 
> I could use some further education on why this is the correct approach. For a 
> dependent-on-template-parameters case, this makes sense -- either the 
> template will be instantiated (at which point we'll know if it's a constant 
> expression) or it won't be (at which point it's constant expression-ness 
> doesn't matter). But for error recovery, we will *never* get a valid constant 
> expression.
> 
> I worry about the performance overhead of continuing on with constant 
> expression evaluation in the error case. We use these code paths not only to 
> get a value but to say "is this a constant expression at all?".
> 
> I don't see why the fix should be localized to just the switch statement 
> condition; it seems like *any* attempt to get a dependent value from an error 
> recovery expression is a point at which we can definitively say "this is not 
> a constant expression" and move on.
I understand that continuing to perform constant evaluation when an error 
occurs can bring more additional diagnostic information (such as jumping to the 
default branch to continue calculation when the condition expression evaluation 
of switch-statement fails), but the additional diagnostic message that is 
emitted is in some cases doesn't usually useful, and as Aaron said may affect 
performance of clang. I don't have enough experience to make a tradeoff between 
the two. BTW 
https://github.com/llvm/llvm-project/blob/843ff7581408a02e852c0f1f7ebf176cabbc7527/clang/lib/Parse/ParseStmt.cpp#L1894-L1909
 I don't quite understand why a RecoveryExpr is not created here, which caused 
to the whole do statement disappears on the 
AST(https://godbolt.org/z/PsPb31YKP), should we fix this? 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153296

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

[PATCH] D148216: Add support for annotations in UpdateTestChecks (NFC)

[Nikita Popov via Phabricator via cfe-commits]

nikic accepted this revision.
nikic added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: StephenFan.

LGTM

> Fixed. Does this test case exist somewhere? I couldn't find it upstream.

That wasn't an existing test case. Though if you add a `--check-globals all` 
variant to generated-funcs.c, I believe that would cover that case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148216

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

[PATCH] D154450: [clangd][c++20] Drop first template argument in code completion in some contexts.

[Jens Massberg via Phabricator via cfe-commits]

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

In case of a top level context the first template argument of a concept
should be dropped. Currently the indexer doesn't support different
signatures for different contexts (for an index entry always the default
`Symbol` context is used). Thus we add a hack which checks if we are in
a top level context and have a concept and in that case removes the
first argment of the signature and snippet suffix. If there is only a
single argument, the signature and snippet suffix are completly
removed. The check for the first argument is done by simply looking for
the first comma which should be sufficient in most cases.

Additionally extend test environment to support adding artificial index
entries with signature and completion snippet suffix.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D154450

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

Index: clang-tools-extra/clangd/unittests/TestIndex.h
===
--- clang-tools-extra/clangd/unittests/TestIndex.h
+++ clang-tools-extra/clangd/unittests/TestIndex.h
@@ -20,7 +20,8 @@
 // Helpers to produce fake index symbols with proper SymbolID.
 // USRFormat is a regex replacement string for the unqualified part of the USR.
 Symbol sym(llvm::StringRef QName, index::SymbolKind Kind,
-   llvm::StringRef USRFormat);
+   llvm::StringRef USRFormat, llvm::StringRef Signature = "",
+   llvm::StringRef CompletionSnippetSuffix = "");
 // Creats a function symbol assuming no function arg.
 Symbol func(llvm::StringRef Name);
 // Creates a class symbol.
@@ -34,7 +35,8 @@
 // Creates a namespace symbol.
 Symbol ns(llvm::StringRef Name);
 // Create a C++20 concept symbol.
-Symbol conceptSym(llvm::StringRef Name);
+Symbol conceptSym(llvm::StringRef Name, llvm::StringRef Signature = "",
+  llvm::StringRef CompletionSnippetSuffix = "");
 
 // Create an Objective-C symbol.
 Symbol objcSym(llvm::StringRef Name, index::SymbolKind Kind,
Index: clang-tools-extra/clangd/unittests/TestIndex.cpp
===
--- clang-tools-extra/clangd/unittests/TestIndex.cpp
+++ clang-tools-extra/clangd/unittests/TestIndex.cpp
@@ -38,7 +38,8 @@
 // Helpers to produce fake index symbols for memIndex() or completions().
 // USRFormat is a regex replacement string for the unqualified part of the USR.
 Symbol sym(llvm::StringRef QName, index::SymbolKind Kind,
-   llvm::StringRef USRFormat) {
+   llvm::StringRef USRFormat, llvm::StringRef Signature,
+   llvm::StringRef CompletionSnippetSuffix) {
   Symbol Sym;
   std::string USR = "c:"; // We synthesize a few simple cases of USRs by hand!
   size_t Pos = QName.rfind("::");
@@ -55,6 +56,8 @@
   Sym.SymInfo.Kind = Kind;
   Sym.Flags |= Symbol::IndexedForCodeCompletion;
   Sym.Origin = SymbolOrigin::Static;
+  Sym.Signature = Signature;
+  Sym.CompletionSnippetSuffix = CompletionSnippetSuffix;
   return Sym;
 }
 
@@ -82,8 +85,10 @@
   return sym(Name, index::SymbolKind::Namespace, "@N@\\0");
 }
 
-Symbol conceptSym(llvm::StringRef Name) {
-  return sym(Name, index::SymbolKind::Concept, "@CT@\\0");
+Symbol conceptSym(llvm::StringRef Name, llvm::StringRef Signature,
+  llvm::StringRef CompletionSnippetSuffix) {
+  return sym(Name, index::SymbolKind::Concept, "@CT@\\0", Signature,
+ CompletionSnippetSuffix);
 }
 
 Symbol objcSym(llvm::StringRef Name, index::SymbolKind Kind,
Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -3961,26 +3961,48 @@
 template<$tparam^A U>
 int foo();
 
+template
+int bar() requires $other^A;
+
 template
-concept b = $other^A && $other^sizeof(T) % 2 == 0 || $other^A && sizeof(T) == 1;
+concept b = $other^A && $other^sizeof(T) % 2 == 0 || $other^A && sizeof(T) == 1;
 
-$other^A auto i = 19;
+$toplevel^A auto i = 19;
   )cpp");
   TestTU TU;
   TU.Code = Code.code().str();
   TU.ExtraArgs = {"-std=c++20"};
 
-  std::vector Syms = {conceptSym("same_as")};
+  std::vector Syms = {
+  conceptSym("same_as", "",
+ "<${1:typename Tp}, ${2:typename Up}>")};
   for (auto P : Code.points("tparam")) {
-ASSERT_THAT(completions(TU, P, Syms).Completions,
-AllOf(Contains(named("A")), Contains(named("same_as")),
-   

[PATCH] D151325: [analyzer] Differentiate lifetime extended temporaries

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun accepted this revision.
xazax.hun added inline comments.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h:1282
+  LLVM_ATTRIBUTE_RETURNS_NONNULL
+  const Expr *getExpr() const { return Ex; }
+  LLVM_ATTRIBUTE_RETURNS_NONNULL

tomasz-kaminski-sonarsource wrote:
> xazax.hun wrote:
> > Do we actually need this `Expr` in the memory region?
> Yes they are needed for region identification, as one variable my be lifetime 
> extending multiple temporary.
> For illustration consider following example from test:
> ```
>   auto const& viaReference = RefAggregate{10, Composite{}}; // extends `int`, 
> `Composite`, and `RefAggregate`
>   clang_analyzer_dump(viaReference);// expected-warning-re 
> {{&lifetime_extended_object{RefAggregate, viaReference, S{{[0-9]+}}} }}
>   clang_analyzer_dump(viaReference.rx); // expected-warning-re 
> {{&lifetime_extended_object{int, viaReference, S{{[0-9]+}}} }}
>   clang_analyzer_dump(viaReference.ry); // expected-warning-re 
> {{&lifetime_extended_object{Composite, viaReference, S{{[0-9]+}}} }}
> ```
> `viaReference` declaration is extending 3 temporaries, and they get 
> identified by expression.
Ah, OK. Makes sense, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151325

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

[PATCH] D154221: [analyzer] Fix false negative when pass implicit cast nil to nonnull

[tripleCC via Phabricator via cfe-commits]

tripleCC updated this revision to Diff 537110.
tripleCC added a comment.

try to fix nullability-arc.mm test case error


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154221

Files:
  clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
  clang/test/Analysis/nullability-arc.mm
  clang/test/Analysis/nullability.mm


Index: clang/test/Analysis/nullability.mm
===
--- clang/test/Analysis/nullability.mm
+++ clang/test/Analysis/nullability.mm
@@ -69,6 +69,7 @@
 void takesNonnull(Dummy *_Nonnull);
 void takesUnspecified(Dummy *);
 void takesNonnullBlock(void (^ _Nonnull)(void));
+void takesNonnullObject(NSObject *_Nonnull);
 
 Dummy *_Nullable returnsNullable();
 Dummy *_Nonnull returnsNonnull();
@@ -264,6 +265,17 @@
   return (Dummy * _Nonnull)0; // no-warning
 }
 
+void testImplicitCastNilToNonnull() {
+  id obj = nil;
+  takesNonnullObject(obj); // expected-warning {{nil passed to a callee that 
requires a non-null 1st parameter}}
+}
+
+void testImplicitCastNullableArgToNonnull(TestObject *_Nullable obj) {
+  if (!obj) {
+takesNonnullObject(obj); // expected-warning {{nil passed to a callee that 
requires a non-null 1st parameter}}
+  }
+}
+
 void testIndirectCastNilToNonnullAndPass() {
   Dummy *p = (Dummy * _Nonnull)0;
   // FIXME: Ideally the cast above would suppress this warning.
Index: clang/test/Analysis/nullability-arc.mm
===
--- clang/test/Analysis/nullability-arc.mm
+++ clang/test/Analysis/nullability-arc.mm
@@ -3,9 +3,6 @@
 // RUN: %clang_analyze_cc1 -w -analyzer-checker=core,nullability\
 // RUN:   -analyzer-output=text -verify %s -fobjc-arc
 
-#if !__has_feature(objc_arc)
-// expected-no-diagnostics
-#endif
 
 
 #define nil ((id)0)
@@ -24,16 +21,10 @@
 - (void)foo:(Param *)param {
   // FIXME: Why do we not emit the warning under ARC?
   [super foo:param];
-#if __has_feature(objc_arc)
-  // expected-warning@-2{{nil passed to a callee that requires a non-null 1st 
parameter}}
-  // expected-note@-3   {{nil passed to a callee that requires a non-null 1st 
parameter}}
-#endif
 
   [self foo:nil];
-#if __has_feature(objc_arc)
-  // expected-note@-2{{Calling 'foo:'}}
-  // expected-note@-3{{Passing nil object reference via 1st parameter 'param'}}
-#endif
+  // expected-warning@-1{{nil passed to a callee that requires a non-null 1st 
parameter}}
+  // expected-note@-2   {{nil passed to a callee that requires a non-null 1st 
parameter}}
 }
 @end
 
Index: clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
@@ -767,7 +767,7 @@
 Nullability RequiredNullability =
 getNullabilityAnnotation(Param->getType());
 Nullability ArgExprTypeLevelNullability =
-getNullabilityAnnotation(ArgExpr->getType());
+getNullabilityAnnotation(lookThroughImplicitCasts(ArgExpr)->getType());
 
 unsigned ParamIdx = Param->getFunctionScopeIndex() + 1;
 


Index: clang/test/Analysis/nullability.mm
===
--- clang/test/Analysis/nullability.mm
+++ clang/test/Analysis/nullability.mm
@@ -69,6 +69,7 @@
 void takesNonnull(Dummy *_Nonnull);
 void takesUnspecified(Dummy *);
 void takesNonnullBlock(void (^ _Nonnull)(void));
+void takesNonnullObject(NSObject *_Nonnull);
 
 Dummy *_Nullable returnsNullable();
 Dummy *_Nonnull returnsNonnull();
@@ -264,6 +265,17 @@
   return (Dummy * _Nonnull)0; // no-warning
 }
 
+void testImplicitCastNilToNonnull() {
+  id obj = nil;
+  takesNonnullObject(obj); // expected-warning {{nil passed to a callee that requires a non-null 1st parameter}}
+}
+
+void testImplicitCastNullableArgToNonnull(TestObject *_Nullable obj) {
+  if (!obj) {
+takesNonnullObject(obj); // expected-warning {{nil passed to a callee that requires a non-null 1st parameter}}
+  }
+}
+
 void testIndirectCastNilToNonnullAndPass() {
   Dummy *p = (Dummy * _Nonnull)0;
   // FIXME: Ideally the cast above would suppress this warning.
Index: clang/test/Analysis/nullability-arc.mm
===
--- clang/test/Analysis/nullability-arc.mm
+++ clang/test/Analysis/nullability-arc.mm
@@ -3,9 +3,6 @@
 // RUN: %clang_analyze_cc1 -w -analyzer-checker=core,nullability\
 // RUN:   -analyzer-output=text -verify %s -fobjc-arc
 
-#if !__has_feature(objc_arc)
-// expected-no-diagnostics
-#endif
 
 
 #define nil ((id)0)
@@ -24,16 +21,10 @@
 - (void)foo:(Param *)param {
   // FIXME: Why do we not emit the warning under ARC?
   [super foo:param];
-#if __has_feature(objc_arc)
-  // expected-warning@-2{{nil passed to a callee that requires a non-null

[PATCH] D153273: [analyzer] Rework support for CFGScopeBegin, CFGScopeEnd, CFGLifetime elements

[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.

I'd appreciate some review on this, given that a lot of you would be affected 
by the changes of CFG.
By changes I mean, fixes for goto statements, properly calling destructors and 
stuff.

It's already in production for our CSA fork, and the results look good.

In two weeks we plan to land this unless anyone objects.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153273

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

[PATCH] D151325: [analyzer] Differentiate lifetime extended temporaries

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp:556
+  bool IsLocal =
+  isa_and_nonnull(MR) &&
+  isa(MR->getMemorySpace());

I think strictly speaking this might be "unsound" resulting in false positives 
in scenarios like:
```
void f(bool reset) {
  static T&& extended = getTemp();
  if (reset) {
extended = getTemp();
return;
  }
  consume(std::move(extended));
  f(true);
  extended.use();
}
```

In case the call to `f(true)` is not inlined (or in other cases there is some 
aliasing the analyzer does not know about), we might not realize that the 
object is reset and might falsely report a use after move.

But I think this is probably sufficiently rare that we do not care about those. 



Comment at: clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp:101
+QualType Ty = LER->getValueType().getLocalUnqualifiedType();
+os << "stack memory associated with temporary object of type '";
+Ty.print(os, Ctx.getPrintingPolicy());

Is this a good wording for static lifetime extended temporaries?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151325

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

[PATCH] D154325: [analyzer][NFC] Move away from using raw-for loops inside StaticAnalyzer

[Balázs Benics via Phabricator via cfe-commits]

steakhal added a comment.

Fixed remarks, cleaned up residual includes, rebased.
Should be read to land.
Final thoughts?




Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h:501
   public:
+const iterator &operator*() const { return *this; }
+

steakhal wrote:
> xazax.hun wrote:
> > Is this just a dummy to make sure this satisfies some concepts? I think I 
> > comment might be helpful in that case, people might find a noop dereference 
> > operator confusing. Same for some other places.
> Yes. I'll add a comment.
> This falls into the "weird" iterators category.
Added comments to both places.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h:749-750
 
   referenced_vars_iterator referenced_vars_begin() const;
   referenced_vars_iterator referenced_vars_end() const;
+  llvm::iterator_range referenced_vars() const;

steakhal wrote:
> xazax.hun wrote:
> > Do we need these with the addition of `referenced_vars`?
> I no longer remember where, but yes. But for sure I went through these to see 
> if I could remove them.
Now I remember.
The `referenced_vars` is implemented as 
`llvm::make_range(referenced_vars_begin(), referenced_vars_end())`, thus I need 
those functions as they do quite a bit of work if you check.
So, I kept them.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h:637-638
 
   region_iterator region_begin() const { return LiveRegionRoots.begin(); }
   region_iterator region_end() const { return LiveRegionRoots.end(); }
+  llvm::iterator_range regions() const {

steakhal wrote:
> xazax.hun wrote:
> > Should we remove these?
> I'm pretty sure we need these somewhere.
> One prominent place is where we dump these into JSONs. We do some fancy 
> iteration there. I'm not sure that is the reason for this particular case.
> I'll give it another look to confirm.
This was just an oversight. Removed. Thanks!



Comment at: clang/lib/StaticAnalyzer/Checkers/TrustNonnullChecker.cpp:72
 
-for (auto B=CondS->symbol_begin(), E=CondS->symbol_end(); B != E; ++B) {
-  const SymbolRef Antecedent = *B;
+for (SymbolRef Antecedent : CondS->symbols()) {
   State = addImplication(Antecedent, State, true);

Oh, I didn't know this is a word :D



Comment at: clang/lib/StaticAnalyzer/Core/Environment.cpp:29
 #include "llvm/ADT/ImmutableMap.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallPtrSet.h"

Removed this extra include.



Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp:173-177
+} else {
 // The left-hand side may bind to a different value then the
 // computation type.
 LHSVal = svalBuilder.evalCast(Result, LTy, CTy);
+}

steakhal wrote:
> xazax.hun wrote:
> > Might be an issue with Phab, but indent looks strange to me here.
> Thanks for catching this. Indeed. I'll have a look. Probably I messed 
> something up.
Clang-format went crazy, and I'm using clang-format on save, thus messed this 
up.
Fixed up manually. Should be good now.



Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp:1190
   CXXRecordDecl::field_iterator CurField = LE->getLambdaClass()->field_begin();
-  for (LambdaExpr::const_capture_init_iterator i = LE->capture_init_begin(),
-   e = LE->capture_init_end();
-   i != e; ++i, ++CurField, ++Idx) {
+  for (auto i = LE->capture_init_begin(), e = LE->capture_init_end(); i != e;
+   ++i, ++CurField, ++Idx) {

steakhal wrote:
> xazax.hun wrote:
> > Ha about using `capture_inits`?
> I would count this as a "fancy" iteration as we increment/advance more 
> logical sequences together.
I had a better idea and llvm::zip all these together.
Check it out, and tell me if you like it more ;)
I certainly do.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154325

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

[PATCH] D153296: [AST] Stop evaluate constant expression if the condition expression which in switch statement contains errors

[Yurong via Phabricator via cfe-commits]

yronglin added inline comments.



Comment at: clang/lib/AST/ExprConstant.cpp:4921
+  // value is.
+  if (isa(E))
+return false;

yronglin wrote:
> aaron.ballman wrote:
> > yronglin wrote:
> > > hokein wrote:
> > > > The constant evaluator is not aware of the "error" concept, it is only 
> > > > aware of value-dependent -- the general idea behind is that we treat 
> > > > the dependent-on-error and dependent-on-template-parameters cases the 
> > > > same, they are potentially constant (if we see an expression contains 
> > > > errors, it could be constant depending on how the error is resolved), 
> > > > this will give us nice recovery and avoid bogus following diagnostics.
> > > > 
> > > > So, a `RecoveryExpr` should not result in failure when checking for a 
> > > > potential constant expression.
> > > > 
> > > > I think the right fix is to remove the conditional check `if 
> > > > (!EvaluateDependentExpr(SS->getCond(), Info))` in `EvaluateSwitch`, and 
> > > > return `ESR_Failed` unconditionally (we don't know its value, any 
> > > > switch-case anwser will be wrong in some cases). We already do this for 
> > > > return-statment, do-statement etc.
> > > > 
> > > > 
> > > Do you mean?
> > > ```
> > > if (SS->getCond()->isValueDependent()) {
> > > EvaluateDependentExpr(SS->getCond(), Info);
> > > return ESR_Failed;
> > > }
> > > ```
> > > the general idea behind is that we treat the dependent-on-error and 
> > > dependent-on-template-parameters cases the same, they are potentially 
> > > constant (if we see an expression contains errors, it could be constant 
> > > depending on how the error is resolved), this will give us nice recovery 
> > > and avoid bogus following diagnostics.
> > 
> > I could use some further education on why this is the correct approach. For 
> > a dependent-on-template-parameters case, this makes sense -- either the 
> > template will be instantiated (at which point we'll know if it's a constant 
> > expression) or it won't be (at which point it's constant expression-ness 
> > doesn't matter). But for error recovery, we will *never* get a valid 
> > constant expression.
> > 
> > I worry about the performance overhead of continuing on with constant 
> > expression evaluation in the error case. We use these code paths not only 
> > to get a value but to say "is this a constant expression at all?".
> > 
> > I don't see why the fix should be localized to just the switch statement 
> > condition; it seems like *any* attempt to get a dependent value from an 
> > error recovery expression is a point at which we can definitively say "this 
> > is not a constant expression" and move on.
> I understand that continuing to perform constant evaluation when an error 
> occurs can bring more additional diagnostic information (such as jumping to 
> the default branch to continue calculation when the condition expression 
> evaluation of switch-statement fails), but the additional diagnostic message 
> that is emitted is in some cases doesn't usually useful, and as Aaron said 
> may affect performance of clang. I don't have enough experience to make a 
> tradeoff between the two. BTW 
> https://github.com/llvm/llvm-project/blob/843ff7581408a02e852c0f1f7ebf176cabbc7527/clang/lib/Parse/ParseStmt.cpp#L1894-L1909
>  I don't quite understand why a RecoveryExpr is not created here, which 
> caused to the whole do statement disappears on the 
> AST(https://godbolt.org/z/PsPb31YKP), should we fix this? 
Thanks a lot for your comments! @aaron.ballman 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153296

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

[PATCH] D154221: [analyzer] Fix false negative when pass implicit cast nil to nonnull

[tripleCC via Phabricator via cfe-commits]

tripleCC added inline comments.



Comment at: clang/test/Analysis/nullability-arc.mm:26
   [self foo:nil];
-#if __has_feature(objc_arc)
-  // expected-note@-2{{Calling 'foo:'}}
-  // expected-note@-3{{Passing nil object reference via 1st parameter 'param'}}
-#endif
+  // expected-warning@-1{{nil passed to a callee that requires a non-null 1st 
parameter}}
+  // expected-note@-2   {{nil passed to a callee that requires a non-null 1st 
parameter}}

I'm not sure if this change fixes the issue in [[ 
https://reviews.llvm.org/D54017?id=172274#inline-477102 | D54017 ]] , and may 
require some further discussion


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154221

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

[PATCH] D154325: [analyzer][NFC] Move away from using raw-for loops inside StaticAnalyzer

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun accepted this revision.
xazax.hun added a comment.

I love it, I think we can land this as is. If there are further comments, we 
can address those in follow-up PRs.




Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp:1190
   CXXRecordDecl::field_iterator CurField = LE->getLambdaClass()->field_begin();
-  for (LambdaExpr::const_capture_init_iterator i = LE->capture_init_begin(),
-   e = LE->capture_init_end();
-   i != e; ++i, ++CurField, ++Idx) {
+  for (auto i = LE->capture_init_begin(), e = LE->capture_init_end(); i != e;
+   ++i, ++CurField, ++Idx) {

steakhal wrote:
> steakhal wrote:
> > xazax.hun wrote:
> > > Ha about using `capture_inits`?
> > I would count this as a "fancy" iteration as we increment/advance more 
> > logical sequences together.
> I had a better idea and llvm::zip all these together.
> Check it out, and tell me if you like it more ;)
> I certainly do.
Nice!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154325

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

[PATCH] D151325: [analyzer] Differentiate lifetime extended temporaries

[Tomasz Kamiński via Phabricator via cfe-commits]

tomasz-kaminski-sonarsource marked an inline comment as done.
tomasz-kaminski-sonarsource added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp:556
+  bool IsLocal =
+  isa_and_nonnull(MR) &&
+  isa(MR->getMemorySpace());

xazax.hun wrote:
> I think strictly speaking this might be "unsound" resulting in false 
> positives in scenarios like:
> ```
> void f(bool reset) {
>   static T&& extended = getTemp();
>   if (reset) {
> extended = getTemp();
> return;
>   }
>   consume(std::move(extended));
>   f(true);
>   extended.use();
> }
> ```
> 
> In case the call to `f(true)` is not inlined (or in other cases there is some 
> aliasing the analyzer does not know about), we might not realize that the 
> object is reset and might falsely report a use after move.
> 
> But I think this is probably sufficiently rare that we do not care about 
> those. 
Aren't such case already excluded by the 
`isa(MR->getMemorySpace())`, in a same way as we do for 
variables?
In such case, we should be creating `getCXXStaticLifetimeExtendedObjectRegion` 
so it will not be located on stack.



Comment at: clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp:101
+QualType Ty = LER->getValueType().getLocalUnqualifiedType();
+os << "stack memory associated with temporary object of type '";
+Ty.print(os, Ctx.getPrintingPolicy());

xazax.hun wrote:
> Is this a good wording for static lifetime extended temporaries?
Lifetime extended static temporaries should never enter here, as they are not 
in `StackSpaceRegion`. 
Previously we have had `CXXStaticTempObjects` and now they are turned into 
`CXXStaticLifetimeExtendedObjectRegion`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151325

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

[clang] df5213c - [clang][Interp][NFC] Return a const pointer from Pointer::getRecord()

[Timm Bäder via cfe-commits]

Author: Timm Bäder
Date: 2023-07-04T17:21:51+02:00
New Revision: df5213c4420e40dace6506d39bba28459a91c2a4

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

LOG: [clang][Interp][NFC] Return a const pointer from Pointer::getRecord()

Added: 


Modified: 
clang/lib/AST/Interp/EvalEmitter.cpp
clang/lib/AST/Interp/Interp.cpp
clang/lib/AST/Interp/Pointer.h
clang/lib/AST/Interp/Record.h

Removed: 




diff  --git a/clang/lib/AST/Interp/EvalEmitter.cpp 
b/clang/lib/AST/Interp/EvalEmitter.cpp
index 6d89bc407aad8d..f22cca90d4f412 100644
--- a/clang/lib/AST/Interp/EvalEmitter.cpp
+++ b/clang/lib/AST/Interp/EvalEmitter.cpp
@@ -123,7 +123,7 @@ bool EvalEmitter::emitRetValue(const SourceInfo &Info) {
   Ty = AT->getValueType();
 
 if (auto *RT = Ty->getAs()) {
-  auto *Record = Ptr.getRecord();
+  const auto *Record = Ptr.getRecord();
   assert(Record && "Missing record descriptor");
 
   bool Ok = true;

diff  --git a/clang/lib/AST/Interp/Interp.cpp b/clang/lib/AST/Interp/Interp.cpp
index e906f65c371c27..c3503f4a078958 100644
--- a/clang/lib/AST/Interp/Interp.cpp
+++ b/clang/lib/AST/Interp/Interp.cpp
@@ -68,7 +68,7 @@ static bool CheckActive(InterpState &S, CodePtr OpPC, const 
Pointer &Ptr,
   }
 
   // Find the active field of the union.
-  Record *R = U.getRecord();
+  const Record *R = U.getRecord();
   assert(R && R->isUnion() && "Not a union");
   const FieldDecl *ActiveField = nullptr;
   for (unsigned I = 0, N = R->getNumFields(); I < N; ++I) {

diff  --git a/clang/lib/AST/Interp/Pointer.h b/clang/lib/AST/Interp/Pointer.h
index ab196beb93aa36..7d9e45a0a5a206 100644
--- a/clang/lib/AST/Interp/Pointer.h
+++ b/clang/lib/AST/Interp/Pointer.h
@@ -247,9 +247,11 @@ class Pointer {
   }
 
   /// Returns the record descriptor of a class.
-  Record *getRecord() const { return getFieldDesc()->ElemRecord; }
+  const Record *getRecord() const { return getFieldDesc()->ElemRecord; }
   /// Returns the element record type, if this is a non-primive array.
-  Record *getElemRecord() const { return getFieldDesc()->ElemDesc->ElemRecord; 
}
+  const Record *getElemRecord() const {
+return getFieldDesc()->ElemDesc->ElemRecord;
+  }
   /// Returns the field information.
   const FieldDecl *getField() const { return getFieldDesc()->asFieldDecl(); }
 

diff  --git a/clang/lib/AST/Interp/Record.h b/clang/lib/AST/Interp/Record.h
index 52173fffa54474..24092f57c0d941 100644
--- a/clang/lib/AST/Interp/Record.h
+++ b/clang/lib/AST/Interp/Record.h
@@ -87,7 +87,7 @@ class Record final {
   }
 
   unsigned getNumBases() const { return Bases.size(); }
-  Base *getBase(unsigned I) { return &Bases[I]; }
+  const Base *getBase(unsigned I) const { return &Bases[I]; }
 
   using const_virtual_iter = VirtualBaseList::const_iterator;
   llvm::iterator_range virtual_bases() const {
@@ -95,7 +95,7 @@ class Record final {
   }
 
   unsigned getNumVirtualBases() const { return VirtualBases.size(); }
-  Base *getVirtualBase(unsigned I) { return &VirtualBases[I]; }
+  const Base *getVirtualBase(unsigned I) const { return &VirtualBases[I]; }
 
 private:
   /// Constructor used by Program to create record descriptors.



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

[PATCH] D151325: [analyzer] Differentiate lifetime extended temporaries

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp:556
+  bool IsLocal =
+  isa_and_nonnull(MR) &&
+  isa(MR->getMemorySpace());

tomasz-kaminski-sonarsource wrote:
> xazax.hun wrote:
> > I think strictly speaking this might be "unsound" resulting in false 
> > positives in scenarios like:
> > ```
> > void f(bool reset) {
> >   static T&& extended = getTemp();
> >   if (reset) {
> > extended = getTemp();
> > return;
> >   }
> >   consume(std::move(extended));
> >   f(true);
> >   extended.use();
> > }
> > ```
> > 
> > In case the call to `f(true)` is not inlined (or in other cases there is 
> > some aliasing the analyzer does not know about), we might not realize that 
> > the object is reset and might falsely report a use after move.
> > 
> > But I think this is probably sufficiently rare that we do not care about 
> > those. 
> Aren't such case already excluded by the 
> `isa(MR->getMemorySpace())`, in a same way as we do for 
> variables?
> In such case, we should be creating 
> `getCXXStaticLifetimeExtendedObjectRegion` so it will not be located on stack.
Whoops, indeed, sorry. 



Comment at: clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp:101
+QualType Ty = LER->getValueType().getLocalUnqualifiedType();
+os << "stack memory associated with temporary object of type '";
+Ty.print(os, Ctx.getPrintingPolicy());

tomasz-kaminski-sonarsource wrote:
> xazax.hun wrote:
> > Is this a good wording for static lifetime extended temporaries?
> Lifetime extended static temporaries should never enter here, as they are not 
> in `StackSpaceRegion`. 
> Previously we have had `CXXStaticTempObjects` and now they are turned into 
> `CXXStaticLifetimeExtendedObjectRegion`.
Ah, my bad.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151325

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

[PATCH] D154437: [analyzer][NFC] Make AnalyzerConfig round-tripping deterministic

[Balázs Benics via Phabricator via cfe-commits]

steakhal abandoned this revision.
steakhal added a comment.

Ah, D142861  is only on `main`, that's why we 
don't have that. I'll cherry-pick that then.
@jansvoboda11 Thanks for the xref!

Confirmed, that fixes the issue. Abandoning this one.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154437

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

[PATCH] D154221: [analyzer] Fix false negative when pass implicit cast nil to nonnull

[tripleCC via Phabricator via cfe-commits]

tripleCC added inline comments.



Comment at: clang/test/Analysis/nullability-arc.mm:25
 
   [self foo:nil];
+  // expected-warning@-1{{nil passed to a callee that requires a non-null 1st 
parameter}}

I think there should be a warning when we call the foo: method


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154221

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

[clang] 74514e8 - [clang][Interp][NFC] Fix GetFnPtr signature

[Timm Bäder via cfe-commits]

Author: Timm Bäder
Date: 2023-07-04T17:35:28+02:00
New Revision: 74514e8713bfcd31faeb6f4cfd5e29824413e1c1

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

LOG: [clang][Interp][NFC] Fix GetFnPtr signature

Added: 


Modified: 
clang/lib/AST/Interp/Interp.h

Removed: 




diff  --git a/clang/lib/AST/Interp/Interp.h b/clang/lib/AST/Interp/Interp.h
index dbe8b3889849cd..f179c0486bdbdb 100644
--- a/clang/lib/AST/Interp/Interp.h
+++ b/clang/lib/AST/Interp/Interp.h
@@ -1700,7 +1700,7 @@ inline bool CallPtr(InterpState &S, CodePtr OpPC) {
   return Call(S, OpPC, F);
 }
 
-inline bool GetFnPtr(InterpState &S, CodePtr &PC, const Function *Func) {
+inline bool GetFnPtr(InterpState &S, CodePtr OpPC, const Function *Func) {
   assert(Func);
   S.Stk.push(Func);
   return true;



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

[clang] a936357 - [clang][Interp][NFC] Return integer from Boolean::bitWidth()

[Timm Bäder via cfe-commits]

Author: Timm Bäder
Date: 2023-07-04T17:35:28+02:00
New Revision: a93635794813ab26a751f72ccb78534e288390b2

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

LOG: [clang][Interp][NFC] Return integer from Boolean::bitWidth()

Added: 


Modified: 
clang/lib/AST/Interp/Boolean.h

Removed: 




diff  --git a/clang/lib/AST/Interp/Boolean.h b/clang/lib/AST/Interp/Boolean.h
index e496f70eb4117a..579842ce46aa04 100644
--- a/clang/lib/AST/Interp/Boolean.h
+++ b/clang/lib/AST/Interp/Boolean.h
@@ -64,7 +64,7 @@ class Boolean final {
 
   Boolean toUnsigned() const { return *this; }
 
-  constexpr static unsigned bitWidth() { return true; }
+  constexpr static unsigned bitWidth() { return 1; }
   bool isZero() const { return !V; }
   bool isMin() const { return isZero(); }
 



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

[clang] 0d40973 - [clang][Interp][NFC] Move CastFP to Interp.h

[Timm Bäder via cfe-commits]

Author: Timm Bäder
Date: 2023-07-04T17:35:28+02:00
New Revision: 0d40973644ba7f3efe666735306646579a7ffe5e

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

LOG: [clang][Interp][NFC] Move CastFP to Interp.h

It's not a Check* function, so try to stay consistent and move this to
the header.

Added: 


Modified: 
clang/lib/AST/Interp/Interp.cpp
clang/lib/AST/Interp/Interp.h

Removed: 




diff  --git a/clang/lib/AST/Interp/Interp.cpp b/clang/lib/AST/Interp/Interp.cpp
index c3503f4a078958..cf9e5b8ea6bd29 100644
--- a/clang/lib/AST/Interp/Interp.cpp
+++ b/clang/lib/AST/Interp/Interp.cpp
@@ -496,14 +496,6 @@ bool CheckFloatResult(InterpState &S, CodePtr OpPC, 
APFloat::opStatus Status) {
   return true;
 }
 
-bool CastFP(InterpState &S, CodePtr OpPC, const llvm::fltSemantics *Sem,
-llvm::RoundingMode RM) {
-  Floating F = S.Stk.pop();
-  Floating Result = F.toSemantics(Sem, RM);
-  S.Stk.push(Result);
-  return true;
-}
-
 bool Interpret(InterpState &S, APValue &Result) {
   // The current stack frame when we started Interpret().
   // This is being used by the ops to determine wheter

diff  --git a/clang/lib/AST/Interp/Interp.h b/clang/lib/AST/Interp/Interp.h
index f179c0486bdbdb..d70b732f6fd865 100644
--- a/clang/lib/AST/Interp/Interp.h
+++ b/clang/lib/AST/Interp/Interp.h
@@ -1421,9 +1421,13 @@ template  bool 
Cast(InterpState &S, CodePtr OpPC) {
 
 /// 1) Pops a Floating from the stack.
 /// 2) Pushes a new floating on the stack that uses the given semantics.
-/// Not templated, so implemented in Interp.cpp.
-bool CastFP(InterpState &S, CodePtr OpPC, const llvm::fltSemantics *Sem,
-llvm::RoundingMode RM);
+inline bool CastFP(InterpState &S, CodePtr OpPC, const llvm::fltSemantics *Sem,
+llvm::RoundingMode RM) {
+  Floating F = S.Stk.pop();
+  Floating Result = F.toSemantics(Sem, RM);
+  S.Stk.push(Result);
+  return true;
+}
 
 template ::T>
 bool CastIntegralFloating(InterpState &S, CodePtr OpPC,



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

[PATCH] D154450: [clangd][c++20] Drop first template argument in code completion in some contexts.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Cool! This makes sense, and is much cheaper than actually working out how to 
render the abbreviated arg list and store it in the index.

Nice that TopLevel gives us some of these contexts, but I suspect it's not all. 
(If not, it's fine to handle just this case for now and leave the others as 
tests showing the bad behavior with a FIXME)




Comment at: clang-tools-extra/clangd/CodeComplete.cpp:321
+// If Signature only contains a single argument an empty string is returned.
+std::string RemoveFirstTemplateArg(const std::string &Signature) {
+  if (Signature.empty() || Signature.front() != '<' || Signature.back() != '>')

RemoveFirst => removeFirst



Comment at: clang-tools-extra/clangd/CodeComplete.cpp:321
+// If Signature only contains a single argument an empty string is returned.
+std::string RemoveFirstTemplateArg(const std::string &Signature) {
+  if (Signature.empty() || Signature.front() != '<' || Signature.back() != '>')

sammccall wrote:
> RemoveFirst => removeFirst
(if this function takes StringRef instead you get nicer APIs like split, and 
less copying)



Comment at: clang-tools-extra/clangd/CodeComplete.cpp:327
+return "";
+  return "<" + Signature.substr(FirstComma + 2);
+}

I don't love the arithmetic, the repetition of 2 as the length of the string, 
and the reliance on the space after the comma.

Maybe: (assuming Signature is a StringRef)

```
[First, Rest] = Signature.split(",");
if (Rest.empty()) // no comma => one argument => no list needed
  return "";
return ("<" + Rest.ltrim()).str();
```



Comment at: clang-tools-extra/clangd/CodeComplete.cpp:481
+
+// Hack to remove first template argument in some special cases.
+if (IsConcept && ContextKind == CodeCompletionContext::CCC_TopLevel) {

more important to say why rather than what: which cases?

```
/// When a concept is used as a type-constraint (e.g. `Iterator auto x`),
/// and in some other contexts, its first type argument is not written.
/// Drop the parameter from the signature.
```



Comment at: clang-tools-extra/clangd/CodeComplete.cpp:484
+  S.Signature = RemoveFirstTemplateArg(S.Signature);
+  S.SnippetSuffix = RemoveFirstTemplateArg(S.SnippetSuffix);
+}

maybe leave a comment:

// dropping the first placeholder from the suffix will leave a `$2` with no 
`$1`.
// However, editors appear to deal with this OK.

(assuming you've tested this in vscode)



Comment at: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp:3964
 
+template
+int bar() requires $other^A;

this doesn't look valid - what is U here?
shouldn't `requires A` be `requires A<...>?

(generally if we're mixing completion cases in one file it's important that 
they're valid so subsequent ones parse correctly - also to not confuse the 
reader)



Comment at: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp:3968
 template
-concept b = $other^A && $other^sizeof(T) % 2 == 0 || $other^A && 
sizeof(T) == 1;
+concept b = $other^A && $other^sizeof(T) % 2 == 0 || $other^A && sizeof(T) 
== 1;
 

I think it might be worth renaming "other" to "expr", it seems to more clearly 
describe all the situations here



Comment at: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp:3970
 
-$other^A auto i = 19;
+$toplevel^A auto i = 19;
   )cpp");

can you also add tests for
```
void abbreviated(A auto x) {}

template int constrainedNTTP();
```



Comment at: clang-tools-extra/clangd/unittests/TestIndex.h:38
 // Create a C++20 concept symbol.
-Symbol conceptSym(llvm::StringRef Name);
+Symbol conceptSym(llvm::StringRef Name, llvm::StringRef Signature = "",
+  llvm::StringRef CompletionSnippetSuffix = "");

this diverges from how the other helpers work: they're mostly creating minimal 
skeletons and encapsulate the mess of synthesizing USRs.

Instead of extending the function here, we can assign the appropriate fields on 
the returned symbol at the callsite (or add a local helper to do so)

(objcSym is also out-of-line with the pattern here, but we shouldn't follow it 
off the rails)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154450

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

[PATCH] D86993: Document Clang's expectations of the C standard library.

[Ralf via Phabricator via cfe-commits]

RalfJung added a comment.
Herald added a subscriber: StephenFan.
Herald added a project: All.

What is the status of this patch? It has not been committed as far as I can 
tell.

I was looking into this coming from Rust, where we do worry a bit about LLVM 
calling `memcpy`, `memmove` or `memset` in ways that are not allowed by the C 
standard. This here would at least give us something to point to, though it 
doesn't answer the question "which C standard libraries have officially 
committed to guaranteeing these properties". Since this affects all compilers 
using LLVM, not just Clang, should this be put somewhere in the LLVM docs?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86993

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

[PATCH] D154417: [libc++] Disable tree invariant check in asserts mode

[Louis Dionne via Phabricator via cfe-commits]

ldionne added a comment.

LGTM, but as a matter of process, please try to wait for someone from the 
libc++ review group before submitting. Thanks for the fix!




Comment at: libcxx/include/__tree:379-380
 _LIBCPP_ASSERT_UNCATEGORIZED(__z != nullptr, "The node to remove should 
not be null");
-_LIBCPP_ASSERT_UNCATEGORIZED(std::__tree_invariant(__root), "The tree 
invariants should hold");
+// TODO: Use in the new debug mode:
+// _LIBCPP_DEBUG_ASSERT(std::__tree_invariant(__root), "The tree 
invariants should hold");
 // __z will be removed from the tree.  Client still needs to 
destruct/deallocate it

CC @var-const in case you want to annotate your TODOs for the debug mode in 
some special way to find them later.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154417

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

[PATCH] D154221: [analyzer] Fix false negative when pass implicit cast nil to nonnull

[Balázs Benics via Phabricator via cfe-commits]

steakhal added a comment.

@tripleCC Could you clarify the status. Do you need some help or review? Do you 
have concerns?
You made some inline comments that I couldn't put into context.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154221

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

[PATCH] D153600: Implement -frecord-command-line for XCOFF

[Jake Egan via Phabricator via cfe-commits]

Jake-Egan updated this revision to Diff 537156.
Jake-Egan edited the summary of this revision.
Jake-Egan added a comment.

Updated to use both a newline and null byte to separate command lines.


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

https://reviews.llvm.org/D153600

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  llvm/include/llvm/CodeGen/AsmPrinter.h
  llvm/include/llvm/MC/MCStreamer.h
  llvm/include/llvm/MC/MCXCOFFStreamer.h
  llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
  llvm/lib/MC/MCAsmStreamer.cpp
  llvm/lib/MC/MCStreamer.cpp
  llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp
  llvm/test/CodeGen/PowerPC/aix-command-line-metadata.ll

Index: llvm/test/CodeGen/PowerPC/aix-command-line-metadata.ll
===
--- /dev/null
+++ llvm/test/CodeGen/PowerPC/aix-command-line-metadata.ll
@@ -0,0 +1,23 @@
+; RUN: llc -mtriple powerpc-ibm-aix-xcoff < %s | \
+; RUN: FileCheck --check-prefix=ASM %s
+; RUN: llc -mtriple powerpc64-ibm-aix-xcoff < %s | \
+; RUN: FileCheck --check-prefix=ASM %s
+
+; RUN: not --crash llc -mtriple powerpc-ibm-aix-xcoff -filetype=obj  < %s 2>&1 | \
+; RUN: FileCheck --check-prefix=OBJ %s
+; RUN: not --crash llc -mtriple powerpc64-ibm-aix-xcoff -filetype=obj  < %s 2>&1 | \
+; RUN: FileCheck --check-prefix=OBJ %s
+
+; Verify that llvm.commandline metadata is emitted to .info sections and that the
+; metadata is padded if necessary.
+
+; OBJ: LLVM ERROR: emitXCOFFCInfoSym is not implemented yet on object generation path
+
+; ASM: .info ".GCC.command.line", 0x003a,
+; ASM: .info , 0x40282329, 0x6f707420, 0x636c616e, 0x67202d63, 0x6f6d6d61, 0x6e64202d
+; ASM: .info , 0x6c696e65, 0x0a004028, 0x23296f70, 0x7420736f, 0x6d657468, 0x696e6720
+; ASM: .info , 0x656c7365, 0x20313233, 0x0a00
+
+!llvm.commandline = !{!0, !1}
+!0 = !{!"clang -command -line"}
+!1 = !{!"something else 123"}
Index: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp
===
--- llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp
+++ llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp
@@ -290,6 +290,8 @@
   bool doFinalization(Module &M) override;
 
   void emitTTypeReference(const GlobalValue *GV, unsigned Encoding) override;
+
+  void emitModuleCommandLines(Module &M) override;
 };
 
 } // end anonymous namespace
@@ -2954,6 +2956,26 @@
   return new PPCLinuxAsmPrinter(tm, std::move(Streamer));
 }
 
+void PPCAIXAsmPrinter::emitModuleCommandLines(Module &M) {
+  const NamedMDNode *NMD = M.getNamedMetadata("llvm.commandline");
+  if (!NMD || !NMD->getNumOperands())
+return;
+
+  std::string S;
+  raw_string_ostream RSOS(S);
+  for (unsigned i = 0, e = NMD->getNumOperands(); i != e; ++i) {
+const MDNode *N = NMD->getOperand(i);
+assert(N->getNumOperands() == 1 &&
+   "llvm.commandline metadata entry can have only one operand");
+const MDString *MDS = cast(N->getOperand(0));
+// Add "@(#)" to support retrieving the command line information with the
+// AIX "what" command
+RSOS << "@(#)opt " << MDS->getString() << "\n";
+RSOS.write('\0');
+  }
+  OutStreamer->emitXCOFFCInfoSym(".GCC.command.line", RSOS.str());
+}
+
 // Force static initialization.
 extern "C" LLVM_EXTERNAL_VISIBILITY void LLVMInitializePowerPCAsmPrinter() {
   TargetRegistry::RegisterAsmPrinter(getThePPC32Target(),
Index: llvm/lib/MC/MCStreamer.cpp
===
--- llvm/lib/MC/MCStreamer.cpp
+++ llvm/lib/MC/MCStreamer.cpp
@@ -1208,6 +1208,11 @@
  "XCOFF targets");
 }
 
+void MCStreamer::emitXCOFFCInfoSym(StringRef Name, StringRef Metadata) {
+  llvm_unreachable("emitXCOFFCInfoSym is only supported on"
+   "XCOFF targets");
+}
+
 void MCStreamer::emitELFSize(MCSymbol *Symbol, const MCExpr *Value) {}
 void MCStreamer::emitELFSymverDirective(const MCSymbol *OriginalSym,
 StringRef Name, bool KeepOriginalSym) {}
Index: llvm/lib/MC/MCAsmStreamer.cpp
===
--- llvm/lib/MC/MCAsmStreamer.cpp
+++ llvm/lib/MC/MCAsmStreamer.cpp
@@ -37,6 +37,7 @@
 #include "llvm/Support/MathExtras.h"
 #include "llvm/Support/Path.h"
 #include 
+#include 
 
 using namespace llvm;
 
@@ -200,6 +201,7 @@
 const MCSymbol *Trap,
 unsigned Lang, unsigned Reason,
 unsigned FunctionSize, bool hasDebug) override;
+  void emitXCOFFCInfoSym(StringRef Name, StringRef Metadata) override;
 
   void emitELFSize(MCSymbol *Symbol, const MCExpr *Value) override;
   void emitCommonSymbol(MCSymbol *Symbol, uint64_t Size,
@@ -966,6 +968,65 @@
   EmitEOL();
 }
 
+void MCAsmStreamer::emitXCOFFCInfoSym(StringRef Name, StringRef Metadata) {
+  const char *InfoDirective = "\t.info ";
+  const char *Separator = ", ";
+  constexpr int WordSize = sizeof(uint32_t);
+
+  // Start by em

[PATCH] D153600: Implement -frecord-command-line for XCOFF

[Jake Egan via Phabricator via cfe-commits]

Jake-Egan updated this revision to Diff 537160.
Jake-Egan added a comment.

Fix formatting


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

https://reviews.llvm.org/D153600

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  llvm/include/llvm/CodeGen/AsmPrinter.h
  llvm/include/llvm/MC/MCStreamer.h
  llvm/include/llvm/MC/MCXCOFFStreamer.h
  llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
  llvm/lib/MC/MCAsmStreamer.cpp
  llvm/lib/MC/MCStreamer.cpp
  llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp
  llvm/test/CodeGen/PowerPC/aix-command-line-metadata.ll

Index: llvm/test/CodeGen/PowerPC/aix-command-line-metadata.ll
===
--- /dev/null
+++ llvm/test/CodeGen/PowerPC/aix-command-line-metadata.ll
@@ -0,0 +1,23 @@
+; RUN: llc -mtriple powerpc-ibm-aix-xcoff < %s | \
+; RUN: FileCheck --check-prefix=ASM %s
+; RUN: llc -mtriple powerpc64-ibm-aix-xcoff < %s | \
+; RUN: FileCheck --check-prefix=ASM %s
+
+; RUN: not --crash llc -mtriple powerpc-ibm-aix-xcoff -filetype=obj  < %s 2>&1 | \
+; RUN: FileCheck --check-prefix=OBJ %s
+; RUN: not --crash llc -mtriple powerpc64-ibm-aix-xcoff -filetype=obj  < %s 2>&1 | \
+; RUN: FileCheck --check-prefix=OBJ %s
+
+; Verify that llvm.commandline metadata is emitted to .info sections and that the
+; metadata is padded if necessary.
+
+; OBJ: LLVM ERROR: emitXCOFFCInfoSym is not implemented yet on object generation path
+
+; ASM: .info ".GCC.command.line", 0x003a,
+; ASM: .info , 0x40282329, 0x6f707420, 0x636c616e, 0x67202d63, 0x6f6d6d61, 0x6e64202d
+; ASM: .info , 0x6c696e65, 0x0a004028, 0x23296f70, 0x7420736f, 0x6d657468, 0x696e6720
+; ASM: .info , 0x656c7365, 0x20313233, 0x0a00
+
+!llvm.commandline = !{!0, !1}
+!0 = !{!"clang -command -line"}
+!1 = !{!"something else 123"}
Index: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp
===
--- llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp
+++ llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp
@@ -290,6 +290,8 @@
   bool doFinalization(Module &M) override;
 
   void emitTTypeReference(const GlobalValue *GV, unsigned Encoding) override;
+
+  void emitModuleCommandLines(Module &M) override;
 };
 
 } // end anonymous namespace
@@ -2954,6 +2956,26 @@
   return new PPCLinuxAsmPrinter(tm, std::move(Streamer));
 }
 
+void PPCAIXAsmPrinter::emitModuleCommandLines(Module &M) {
+  const NamedMDNode *NMD = M.getNamedMetadata("llvm.commandline");
+  if (!NMD || !NMD->getNumOperands())
+return;
+
+  std::string S;
+  raw_string_ostream RSOS(S);
+  for (unsigned i = 0, e = NMD->getNumOperands(); i != e; ++i) {
+const MDNode *N = NMD->getOperand(i);
+assert(N->getNumOperands() == 1 &&
+   "llvm.commandline metadata entry can have only one operand");
+const MDString *MDS = cast(N->getOperand(0));
+// Add "@(#)" to support retrieving the command line information with the
+// AIX "what" command
+RSOS << "@(#)opt " << MDS->getString() << "\n";
+RSOS.write('\0');
+  }
+  OutStreamer->emitXCOFFCInfoSym(".GCC.command.line", RSOS.str());
+}
+
 // Force static initialization.
 extern "C" LLVM_EXTERNAL_VISIBILITY void LLVMInitializePowerPCAsmPrinter() {
   TargetRegistry::RegisterAsmPrinter(getThePPC32Target(),
Index: llvm/lib/MC/MCStreamer.cpp
===
--- llvm/lib/MC/MCStreamer.cpp
+++ llvm/lib/MC/MCStreamer.cpp
@@ -1208,6 +1208,11 @@
  "XCOFF targets");
 }
 
+void MCStreamer::emitXCOFFCInfoSym(StringRef Name, StringRef Metadata) {
+  llvm_unreachable("emitXCOFFCInfoSym is only supported on"
+   "XCOFF targets");
+}
+
 void MCStreamer::emitELFSize(MCSymbol *Symbol, const MCExpr *Value) {}
 void MCStreamer::emitELFSymverDirective(const MCSymbol *OriginalSym,
 StringRef Name, bool KeepOriginalSym) {}
Index: llvm/lib/MC/MCAsmStreamer.cpp
===
--- llvm/lib/MC/MCAsmStreamer.cpp
+++ llvm/lib/MC/MCAsmStreamer.cpp
@@ -36,6 +36,7 @@
 #include "llvm/Support/LEB128.h"
 #include "llvm/Support/MathExtras.h"
 #include "llvm/Support/Path.h"
+#include 
 #include 
 
 using namespace llvm;
@@ -200,6 +201,7 @@
 const MCSymbol *Trap,
 unsigned Lang, unsigned Reason,
 unsigned FunctionSize, bool hasDebug) override;
+  void emitXCOFFCInfoSym(StringRef Name, StringRef Metadata) override;
 
   void emitELFSize(MCSymbol *Symbol, const MCExpr *Value) override;
   void emitCommonSymbol(MCSymbol *Symbol, uint64_t Size,
@@ -966,6 +968,66 @@
   EmitEOL();
 }
 
+void MCAsmStreamer::emitXCOFFCInfoSym(StringRef Name, StringRef Metadata) {
+  const char *InfoDirective = "\t.info ";
+  const char *Separator = ", ";
+  constexpr int WordSize = sizeof(uint32_t);
+
+  // Start by emitting the .info pseudo-op and C_INFO symbol name.
+  OS << InfoDirecti

[PATCH] D154450: [clangd][c++20] Drop first template argument in code completion in some contexts.

[Jens Massberg via Phabricator via cfe-commits]

massberg updated this revision to Diff 537161.
massberg added a comment.

Update code by resolving comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154450

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

Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -3961,26 +3961,51 @@
 template<$tparam^A U>
 int foo();
 
+template
+int bar(T t) requires $expr^A;
+
 template
-concept b = $other^A && $other^sizeof(T) % 2 == 0 || $other^A && sizeof(T) == 1;
+concept b = $expr^A && $expr^sizeof(T) % 2 == 0 || $expr^A && sizeof(T) == 1;
+
+$toplevel^A auto i = 19;
 
-$other^A auto i = 19;
+template<$toplevel^A auto i> void constrainedNTTP();
   )cpp");
   TestTU TU;
   TU.Code = Code.code().str();
   TU.ExtraArgs = {"-std=c++20"};
 
-  std::vector Syms = {conceptSym("same_as")};
+  auto Sym = conceptSym("same_as");
+  Sym.Signature = "";
+  Sym.CompletionSnippetSuffix = "<${1:typename Tp}, ${2:typename Up}>";
+  std::vector Syms = {Sym};
   for (auto P : Code.points("tparam")) {
-ASSERT_THAT(completions(TU, P, Syms).Completions,
-AllOf(Contains(named("A")), Contains(named("same_as")),
-  Contains(named("class")), Contains(named("typename"
+ASSERT_THAT(
+completions(TU, P, Syms).Completions,
+AllOf(Contains(AllOf(named("A"), signature(""), snippetSuffix(""))),
+  Contains(AllOf(named("same_as"), signature(""),
+ snippetSuffix("<${2:typename Up}>"))),
+  Contains(named("class")), Contains(named("typename"
 << "Completing template parameter at position " << P;
   }
 
-  for (auto P : Code.points("other")) {
-EXPECT_THAT(completions(TU, P, Syms).Completions,
-AllOf(Contains(named("A")), Contains(named("same_as"
+  for (auto P : Code.points("toplevel")) {
+EXPECT_THAT(
+completions(TU, P, Syms).Completions,
+AllOf(Contains(AllOf(named("A"), signature(""), snippetSuffix(""))),
+  Contains(AllOf(named("same_as"), signature(""),
+ snippetSuffix("<${2:typename Up}>")
+<< "Completing 'requires' expression at position " << P;
+  }
+
+  for (auto P : Code.points("expr")) {
+EXPECT_THAT(
+completions(TU, P, Syms).Completions,
+AllOf(Contains(AllOf(named("A"), signature(""),
+ snippetSuffix("<${1:class T}>"))),
+  Contains(AllOf(
+  named("same_as"), signature(""),
+  snippetSuffix("<${1:typename Tp}, ${2:typename Up}>")
 << "Completing 'requires' expression at position " << P;
   }
 }
Index: clang-tools-extra/clangd/CodeComplete.cpp
===
--- clang-tools-extra/clangd/CodeComplete.cpp
+++ clang-tools-extra/clangd/CodeComplete.cpp
@@ -316,6 +316,15 @@
   }
 };
 
+// Remove the first template argument from Signature.
+// If Signature only contains a single argument an empty string is returned.
+std::string removeFirstTemplateArg(llvm::StringRef Signature) {
+  auto Rest = Signature.split(",").second;
+  if (Rest.empty())
+return "";
+  return ("<" + Rest.ltrim()).str();
+}
+
 // Assembles a code completion out of a bundle of >=1 completion candidates.
 // Many of the expensive strings are only computed at this point, once we know
 // the candidate bundle is going to be returned.
@@ -336,7 +345,7 @@
 EnableFunctionArgSnippets(Opts.EnableFunctionArgSnippets),
 IsUsingDeclaration(IsUsingDeclaration), NextTokenKind(NextTokenKind) {
 Completion.Deprecated = true; // cleared by any non-deprecated overload.
-add(C, SemaCCS);
+add(C, SemaCCS, ContextKind);
 if (C.SemaResult) {
   assert(ASTCtx);
   Completion.Origin |= SymbolOrigin::AST;
@@ -443,21 +452,40 @@
   });
   }
 
-  void add(const CompletionCandidate &C, CodeCompletionString *SemaCCS) {
+  void add(const CompletionCandidate &C, CodeCompletionString *SemaCCS,
+   CodeCompletionContext::Kind ContextKind) {
 assert(bool(C.SemaResult) == bool(SemaCCS));
 Bundled.emplace_back();
 BundledEntry &S = Bundled.back();
+bool IsConcept = false;
 if (C.SemaResult) {
   getSignature(*SemaCCS, &S.Signature, &S.SnippetSuffix, C.SemaResult->Kind,
C.SemaResult->CursorKind, &Completion.RequiredQualifier);
   if (!C.SemaResult->FunctionCanBeCall)
 S.SnippetSuffix.clear();
   S.ReturnType = getReturnType(*SemaCCS);
+  if (C.SemaResult->Kind == CodeCompletionResult::RK_Declaration)
+   

[PATCH] D154450: [clangd][c++20] Drop first template argument in code completion in some contexts.

[Jens Massberg via Phabricator via cfe-commits]

massberg marked 7 inline comments as done.
massberg added inline comments.



Comment at: clang-tools-extra/clangd/CodeComplete.cpp:327
+return "";
+  return "<" + Signature.substr(FirstComma + 2);
+}

sammccall wrote:
> I don't love the arithmetic, the repetition of 2 as the length of the string, 
> and the reliance on the space after the comma.
> 
> Maybe: (assuming Signature is a StringRef)
> 
> ```
> [First, Rest] = Signature.split(",");
> if (Rest.empty()) // no comma => one argument => no list needed
>   return "";
> return ("<" + Rest.ltrim()).str();
> ```
Thanks! I agree that the code with StringRefs looks much better.



Comment at: clang-tools-extra/clangd/CodeComplete.cpp:484
+  S.Signature = RemoveFirstTemplateArg(S.Signature);
+  S.SnippetSuffix = RemoveFirstTemplateArg(S.SnippetSuffix);
+}

sammccall wrote:
> maybe leave a comment:
> 
> // dropping the first placeholder from the suffix will leave a `$2` with no 
> `$1`.
> // However, editors appear to deal with this OK.
> 
> (assuming you've tested this in vscode)
Yes I've tested it with vscode and it looks fine. Why is the numbering of the 
parameters required?



Comment at: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp:3970
 
-$other^A auto i = 19;
+$toplevel^A auto i = 19;
   )cpp");

sammccall wrote:
> can you also add tests for
> ```
> void abbreviated(A auto x) {}
> 
> template int constrainedNTTP();
> ```
The 
`
void abbreviated(A auto x) {}
`

case doesn't work yet. I will fix it in a follow-up patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154450

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

[PATCH] D154459: [ODRHash] Stop hashing `ObjCMethodDecl::isOverriding` as it doesn't capture inherent method quality.

[Volodymyr Sapsai via Phabricator via cfe-commits]

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

`isOverriding` depends on the surrounding code and not on the method
itself. That's why it can be different in different modules. And
mismatches shouldn't be an error.

rdar://109481753


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D154459

Files:
  clang/lib/AST/ODRDiagsEmitter.cpp
  clang/lib/AST/ODRHash.cpp
  clang/test/Modules/compare-objc-nonisolated-methods.m


Index: clang/test/Modules/compare-objc-nonisolated-methods.m
===
--- /dev/null
+++ clang/test/Modules/compare-objc-nonisolated-methods.m
@@ -0,0 +1,54 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+
+// Test that different values of `ObjCMethodDecl::isOverriding` in different 
modules
+// is not an error because it depends on the surrounding code and not on the 
method itself.
+// RUN: %clang_cc1 -fsyntax-only -verify -I%t/include -fmodules 
-fimplicit-module-maps -fmodules-cache-path=%t/modules.cache 
-fmodule-name=Override %t/test-overriding.m
+
+//--- include/Common.h
+@interface NSObject
+@end
+
+//--- include/Indirection.h
+#import 
+
+//--- include/module.modulemap
+module Common {
+  header "Common.h"
+  export *
+}
+module Indirection {
+  header "Indirection.h"
+  export *
+}
+module Override {
+  header "Override.h"
+  export *
+}
+
+//--- include/Override.h
+#import 
+@interface SubClass: NSObject
+- (void)potentialOverride;
+@end
+
+//--- Override_Internal.h
+#import 
+@interface NSObject(InternalCategory)
+- (void)potentialOverride;
+@end
+
+//--- test-overriding.m
+//expected-no-diagnostics
+// Get non-modular version of `SubClass`, so that `-[SubClass 
potentialOverride]`
+// is an override of a method in `InternalCategory`.
+#import "Override_Internal.h"
+#import 
+
+// Get modular version of `SubClass` where `-[SubClass potentialOverride]` is
+// not an override because module "Override" doesn't know about 
Override_Internal.h.
+#import 
+
+void triggerOverrideCheck(SubClass *sc) {
+  [sc potentialOverride];
+}
Index: clang/lib/AST/ODRHash.cpp
===
--- clang/lib/AST/ODRHash.cpp
+++ clang/lib/AST/ODRHash.cpp
@@ -377,7 +377,6 @@
 Hash.AddBoolean(Method->isVariadic());
 Hash.AddBoolean(Method->isSynthesizedAccessorStub());
 Hash.AddBoolean(Method->isDefined());
-Hash.AddBoolean(Method->isOverriding());
 Hash.AddBoolean(Method->isDirectMethod());
 Hash.AddBoolean(Method->isThisDeclarationADesignatedInitializer());
 Hash.AddBoolean(Method->hasSkippedBody());
Index: clang/lib/AST/ODRDiagsEmitter.cpp
===
--- clang/lib/AST/ODRDiagsEmitter.cpp
+++ clang/lib/AST/ODRDiagsEmitter.cpp
@@ -2096,7 +2096,8 @@
   << FirstDecl->getSourceRange();
   Diag(SecondDecl->getLocation(),
diag::note_module_odr_violation_mismatch_decl_unknown)
-  << SecondModule << FirstDiffType << SecondDecl->getSourceRange();
+  << SecondModule.empty() << SecondModule << FirstDiffType
+  << SecondDecl->getSourceRange();
   return true;
 }
 


Index: clang/test/Modules/compare-objc-nonisolated-methods.m
===
--- /dev/null
+++ clang/test/Modules/compare-objc-nonisolated-methods.m
@@ -0,0 +1,54 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+
+// Test that different values of `ObjCMethodDecl::isOverriding` in different modules
+// is not an error because it depends on the surrounding code and not on the method itself.
+// RUN: %clang_cc1 -fsyntax-only -verify -I%t/include -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/modules.cache -fmodule-name=Override %t/test-overriding.m
+
+//--- include/Common.h
+@interface NSObject
+@end
+
+//--- include/Indirection.h
+#import 
+
+//--- include/module.modulemap
+module Common {
+  header "Common.h"
+  export *
+}
+module Indirection {
+  header "Indirection.h"
+  export *
+}
+module Override {
+  header "Override.h"
+  export *
+}
+
+//--- include/Override.h
+#import 
+@interface SubClass: NSObject
+- (void)potentialOverride;
+@end
+
+//--- Override_Internal.h
+#import 
+@interface NSObject(InternalCategory)
+- (void)potentialOverride;
+@end
+
+//--- test-overriding.m
+//expected-no-diagnostics
+// Get non-modular version of `SubClass`, so that `-[SubClass potentialOverride]`
+// is an override of a method in `InternalCategory`.
+#import "Override_Internal.h"
+#import 
+
+// Get modular version of `SubClass` where `-[SubClass potentialOverride]` is
+// not an override because module "Override" doesn't know about Override_Internal.h.
+#import 
+
+void triggerOverrideCheck(SubClass *sc) {
+  [sc potentialOverride];
+}
Index: clang/lib