[PATCH] D78442: Create a warning flag for 'warn_conv_*_not_used'

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

I suddenly discovered you unnoticed revision. I'd advise you to add more 
reviewers or subscribers here.


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

https://reviews.llvm.org/D78442



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

[PATCH] D75665: [analyzer] On-demand parsing capability for CTU

[Whisperity via Phabricator via cfe-commits]

whisperity resigned from this revision.
whisperity added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/lib/CrossTU/CrossTranslationUnit.cpp:26
+#include "llvm/ADT/Triple.h"
+#include "llvm/Option/ArgList.h"
 #include "llvm/Support/ErrorHandling.h"

Duped include in L34.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75665



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

[clang] d20bf5a - [DebugInfo] Upgrade DISubrange to support Fortran dynamic arrays

[Sourabh Singh Tomar via cfe-commits]

Author: Alok Kumar Sharma
Date: 2020-05-28T13:46:41+05:30
New Revision: d20bf5a7258d4b6a7f017a81b125275dac1aa166

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

LOG: [DebugInfo] Upgrade DISubrange to support Fortran dynamic arrays

This patch upgrades DISubrange to support fortran requirements.

Summary:
Below are the updates/addition of fields.
lowerBound - Now accepts signed integer or DIVariable or DIExpression,
earlier it accepted only signed integer.
upperBound - This field is now added and accepts signed interger or
DIVariable or DIExpression.
stride - This field is now added and accepts signed interger or
DIVariable or DIExpression.
This is required to describe bounds of array which are known at runtime.

Testing:
unit test cases added (hand-written)
check clang
check llvm
check debug-info

Reviewed By: aprantl

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

Added: 
llvm/test/Bitcode/fortranSubrange.ll
llvm/test/Bitcode/fortranSubrangeBackward.ll
llvm/test/Bitcode/fortranSubrangeBackward.ll.bc
llvm/test/DebugInfo/cDefaultLower.ll
llvm/test/DebugInfo/fortranDefaultLower.ll
llvm/test/DebugInfo/fortranSubrangeExpr.ll
llvm/test/DebugInfo/fortranSubrangeInt.ll
llvm/test/DebugInfo/fortranSubrangeVar.ll
llvm/test/Verifier/disubrange-count-upperBound.ll
llvm/test/Verifier/disubrange-missing-upperBound.ll
llvm/test/Verifier/invalid-disubrange-lowerBound.ll
llvm/test/Verifier/invalid-disubrange-stride.ll
llvm/test/Verifier/invalid-disubrange-upperBound.ll

Modified: 
clang/lib/CodeGen/CGDebugInfo.cpp
llvm/include/llvm/IR/DIBuilder.h
llvm/include/llvm/IR/DebugInfoMetadata.h
llvm/lib/AsmParser/LLParser.cpp
llvm/lib/Bitcode/Reader/MetadataLoader.cpp
llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
llvm/lib/IR/AsmWriter.cpp
llvm/lib/IR/DIBuilder.cpp
llvm/lib/IR/DebugInfoMetadata.cpp
llvm/lib/IR/LLVMContextImpl.h
llvm/lib/IR/Verifier.cpp
llvm/test/Assembler/debug-info.ll
llvm/test/Assembler/disubrange-empty-array.ll
llvm/test/Assembler/invalid-disubrange-count-missing.ll
llvm/test/Bindings/llvm-c/debug_info.ll
llvm/test/DebugInfo/X86/default-subrange-array.ll
llvm/test/DebugInfo/X86/nondefault-subrange-array.ll
llvm/unittests/IR/MetadataTest.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGDebugInfo.cpp 
b/clang/lib/CodeGen/CGDebugInfo.cpp
index 7ec792ca0e1f..4e0b6aa0dca6 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -2732,9 +2732,17 @@ llvm::DIType *CGDebugInfo::CreateType(const VectorType 
*Ty,
   QualType QTy(Ty, 0);
   auto SizeExpr = SizeExprCache.find(QTy);
   if (SizeExpr != SizeExprCache.end())
-Subscript = DBuilder.getOrCreateSubrange(0, SizeExpr->getSecond());
-  else
-Subscript = DBuilder.getOrCreateSubrange(0, Count ? Count : -1);
+Subscript = DBuilder.getOrCreateSubrange(
+SizeExpr->getSecond() /*count*/, nullptr /*lowerBound*/,
+nullptr /*upperBound*/, nullptr /*stride*/);
+  else {
+auto *CountNode =
+llvm::ConstantAsMetadata::get(llvm::ConstantInt::getSigned(
+llvm::Type::getInt64Ty(CGM.getLLVMContext()), Count ? Count : -1));
+Subscript = DBuilder.getOrCreateSubrange(
+CountNode /*count*/, nullptr /*lowerBound*/, nullptr /*upperBound*/,
+nullptr /*stride*/);
+  }
   llvm::DINodeArray SubscriptArray = DBuilder.getOrCreateArray(Subscript);
 
   uint64_t Size = CGM.getContext().getTypeSize(Ty);
@@ -2754,8 +2762,18 @@ llvm::DIType *CGDebugInfo::CreateType(const 
ConstantMatrixType *Ty,
 
   // Create ranges for both dimensions.
   llvm::SmallVector Subscripts;
-  Subscripts.push_back(DBuilder.getOrCreateSubrange(0, Ty->getNumColumns()));
-  Subscripts.push_back(DBuilder.getOrCreateSubrange(0, Ty->getNumRows()));
+  auto *ColumnCountNode =
+  llvm::ConstantAsMetadata::get(llvm::ConstantInt::getSigned(
+  llvm::Type::getInt64Ty(CGM.getLLVMContext()), Ty->getNumColumns()));
+  auto *RowCountNode =
+  llvm::ConstantAsMetadata::get(llvm::ConstantInt::getSigned(
+  llvm::Type::getInt64Ty(CGM.getLLVMContext()), Ty->getNumRows()));
+  Subscripts.push_back(DBuilder.getOrCreateSubrange(
+  ColumnCountNode /*count*/, nullptr /*lowerBound*/, nullptr 
/*upperBound*/,
+  nullptr /*stride*/));
+  Subscripts.push_back(DBuilder.getOrCreateSubrange(
+  RowCountNode /*count*/, nullptr /*lowerBound*/, nullptr /*upperBound*/,
+  nullptr /*stride*/));
   llvm::DINodeArray SubscriptArray = DBuilder.getOrCreateArray(Subscripts);
   return DBuilder.createArrayType(Size, Align,

[PATCH] D80023: [clang-tidy] Add abseil-string-find-str-contains checker.

[Nathan James via Phabricator via cfe-commits]

njames93 added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/abseil/StringFindStrContainsCheck.cpp:40-44
+  // FIXME(tdl-g): These options are being parsed redundantly with the
+  // constructor because TransformerClangTidyCheck forces us to provide 
MakeRule
+  // before "this" is fully constructed, but StoreOptions requires us to store
+  // the parsed options in "this".  We need to fix TransformerClangTidyCheck 
and
+  // then we can clean this up.

I've put in a patch to hopefully address this shortcoming D80697. Don't worry 
about this right now, but If that lands then it will make this a lot cleaner.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80023



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

[PATCH] D80697: [clang-tidy] Reworked TransformerClangTidyCheck to simplify usage of Options

[Nathan James via Phabricator via cfe-commits]

njames93 created this revision.
njames93 added reviewers: aaron.ballman, gribozavr2, alexfh.
Herald added subscribers: cfe-commits, xazax.hun.
Herald added a project: clang.

Changed `TransformerClangTidyCheck` to have a virtual method generate the rule. 
This has the advantages of making handling of any check options much simpler.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D80697

Files:
  clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp
  clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp

Index: clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp
@@ -10,6 +10,7 @@
 #include "ClangTidyTest.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/Tooling/Transformer/RangeSelector.h"
+#include "clang/Tooling/Transformer/RewriteRule.h"
 #include "clang/Tooling/Transformer/Stencil.h"
 #include "clang/Tooling/Transformer/Transformer.h"
 #include "gtest/gtest.h"
@@ -28,23 +29,23 @@
 using transformer::statement;
 
 // Invert the code of an if-statement, while maintaining its semantics.
-RewriteRule invertIf() {
-  StringRef C = "C", T = "T", E = "E";
-  RewriteRule Rule = tooling::makeRule(
-  ifStmt(hasCondition(expr().bind(C)), hasThen(stmt().bind(T)),
- hasElse(stmt().bind(E))),
-  change(statement(std::string(RewriteRule::RootID)),
- cat("if(!(", node(std::string(C)), ")) ",
- statement(std::string(E)), " else ",
- statement(std::string(T,
-  cat("negate condition and reverse `then` and `else` branches"));
-  return Rule;
-}
-
 class IfInverterCheck : public TransformerClangTidyCheck {
 public:
   IfInverterCheck(StringRef Name, ClangTidyContext *Context)
-  : TransformerClangTidyCheck(invertIf(), Name, Context) {}
+  : TransformerClangTidyCheck(Name, Context) {}
+
+  Optional makeRule() const override {
+StringRef C = "C", T = "T", E = "E";
+RewriteRule Rule = tooling::makeRule(
+ifStmt(hasCondition(expr().bind(C)), hasThen(stmt().bind(T)),
+   hasElse(stmt().bind(E))),
+change(statement(std::string(RewriteRule::RootID)),
+   cat("if(!(", node(std::string(C)), ")) ",
+   statement(std::string(E)), " else ",
+   statement(std::string(T,
+cat("negate condition and reverse `then` and `else` branches"));
+return Rule;
+  }
 };
 
 // Basic test of using a rewrite rule as a ClangTidy.
@@ -70,10 +71,12 @@
 class IntLitCheck : public TransformerClangTidyCheck {
 public:
   IntLitCheck(StringRef Name, ClangTidyContext *Context)
-  : TransformerClangTidyCheck(tooling::makeRule(integerLiteral(),
-change(cat("LIT")),
-cat("no message")),
-  Name, Context) {}
+  : TransformerClangTidyCheck(Name, Context) {}
+
+  Optional makeRule() const override {
+return tooling::makeRule(integerLiteral(), change(cat("LIT")),
+ cat("no message"));
+  }
 };
 
 // Tests that two changes in a single macro expansion do not lead to conflicts
@@ -94,11 +97,13 @@
 class BinOpCheck : public TransformerClangTidyCheck {
 public:
   BinOpCheck(StringRef Name, ClangTidyContext *Context)
-  : TransformerClangTidyCheck(
-tooling::makeRule(
-binaryOperator(hasOperatorName("+"), hasRHS(expr().bind("r"))),
-change(node("r"), cat("RIGHT")), cat("no message")),
-Name, Context) {}
+  : TransformerClangTidyCheck(Name, Context) {}
+
+  Optional makeRule() const override {
+return tooling::makeRule(
+binaryOperator(hasOperatorName("+"), hasRHS(expr().bind("r"))),
+change(node("r"), cat("RIGHT")), cat("no message"));
+  }
 };
 
 // Tests case where the rule's match spans both source from the macro and its
@@ -118,18 +123,20 @@
 }
 
 // A trivial rewrite-rule generator that requires Objective-C code.
-Optional needsObjC(const LangOptions &LangOpts,
-const ClangTidyCheck::OptionsView &Options) {
-  if (!LangOpts.ObjC)
-return None;
-  return tooling::makeRule(clang::ast_matchers::functionDecl(),
-   change(cat("void changed() {}")), cat("no message"));
-}
-
 class NeedsObjCCheck : public TransformerClangTidyCheck {
 public:
   NeedsObjCCheck(StringRef Name, ClangTidyContext *Context)
-  : TransformerClangTidyCheck(needsObjC, Name, Context) {}
+  : TransformerClangTidyCheck(Name, Context) {}
+
+  bool isLanguageVersionSupported(const LangOptions &LangOpts

[PATCH] D80293: [clangd] Run PreambleThread in async mode behind a flag

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clang-tools-extra/clangd/TUScheduler.cpp:1138
+  if (!wait(Lock, RequestsCV, Timeout,
+[&] { return Requests.empty() && !CurrentRequest; }))
+return false;

I'd consider pulling out an IsIdle lambda (checking for all 3 conditions) and 
using it both here and at the bottom.

PreambleRequests will never be (solely) full here, and Requests will always be 
empty at the bottom, but it's harmless and I think easier to read.



Comment at: clang-tools-extra/clangd/TUScheduler.cpp:1142
+  // Now that ASTWorker processed all requests, ensure PreamblePeer has served
+  // all update requests.
+  if (!PreamblePeer.blockUntilIdle(Timeout))

Maybe explicitly: "This might create new PreambleRequests for the ASTWorker."



Comment at: clang-tools-extra/clangd/TUScheduler.cpp:1146
+  Lock.lock();
+  assert(Requests.empty() && "Received new requests during blockUntilIdle");
+  // Finally make sure ASTWorker has processed all of the preamble updates.

sammccall wrote:
> um... isn't that allowed?
> If new traffic means the queue doesn't go idle, I think we're supposed to 
> return false, rather than crash.
OK, I somehowe forgot that ASTWorker isn't threadsafe so we only have the 
current thread and the worker thread to worry about.

Suggest a slightly expanded message like: No new normal tasks can be scheduled 
concurrently with blockUntilIdle(): ASTWorker isn't threadsafe



Comment at: clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp:97
+  static std::unique_ptr
+  capturePreamble(PreambleCallback CB) {
+class CapturePreamble : public ParsingCallbacks {

Consider inlining this class until we have a second user.
It's not clear that adapting between lambda and an interface clarifies the test 
enough to justify the indirection.



Comment at: clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp:97
+  static std::unique_ptr
+  capturePreamble(PreambleCallback CB) {
+class CapturePreamble : public ParsingCallbacks {

sammccall wrote:
> Consider inlining this class until we have a second user.
> It's not clear that adapting between lambda and an interface clarifies the 
> test enough to justify the indirection.
capturePreamble isn't the right name for this function, unlike captureDiags it 
doesn't actually send the preamble anywhere.



Comment at: clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp:466
   updateWithDiags(
-  S, File, Inputs, WantDiagnostics::Auto,
+  S, File, Inputs, WantDiagnostics::Yes,
   [File, Nonce, &Mut, &TotalUpdates](std::vector) {

As you've pointed out, Yes isn't used in production. So if the behaviour of 
Auto has changed we should test that rather than stop using it.

My understanding of the change:
1. we've asserted that every update yields diagnostics
2. this was true because we were calling runWithAST, this forced an AST build 
at each snapshot, and we'd publish the diagnostics
3. now we only publish diagnostics once the version has been through the 
preamble thread and back
4. these requests get coalesced in the preamble thread if the AST worker is 
pushing them faster than the preamble worker is servicing them
5. so we no longer publish diagnostics for every version even if we have a 
suitable AST

Is this right?
If so, I'd assert that TotalUpdates is at least FilesCount and at most 
FilesCount * UpdatesPerFile, and also that the latest UpdateI for diags (for 
any file) is equal to UpdatesPerFile - 1.
And maybe add a brief explanation (we drop diagnostics for some snapshots as 
they get coalesced in the preamble worker).



Comment at: clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp:999
+  TUScheduler S(CDB, optsForTest(), capturePreamble([&] {
+  if (PreambleBuildCount > 0)
+Ready.wait();

This seems like a dubious use of atomic (lack of synchronization between 
accesses) - there's actually only one thread here, right?

It would be much clearer IMO to just test the passed-in `Version` directly.



Comment at: clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp:1020
+ASSERT_TRUE(bool(AST));
+EXPECT_THAT(AST->Inputs.Version, PI.Version);
+RunASTAction.notify();

nit: spell out "blocking" explicitly



Comment at: clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp:1020
+ASSERT_TRUE(bool(AST));
+EXPECT_THAT(AST->Inputs.Version, PI.Version);
+RunASTAction.notify();

sammccall wrote:
> nit: spell out "blocking" explicitly
if you expose preambleVersion() from ParsedAST, you could assert on it 
directly. I think this would be much clearer than the stuff with 
PreambleBuildCount.

(I don't see any reason not to expo

[PATCH] D80668: [Clang][Sanitizers] Expect test failure on {arm,thumb}v7

[Alexander Potapenko via Phabricator via cfe-commits]

glider added inline comments.



Comment at: clang/test/CodeGen/sanitize-coverage.c:8
+//
+// XFAIL: armv7, thumbv7
 

Is there a Bugzilla entry for this? Please add a link to the code and to the 
patch description.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80668



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

[PATCH] D80197: [DebugInfo] Upgrade DISubrange to support Fortran dynamic arrays

[Sourabh Singh Tomar via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGd20bf5a7258d: [DebugInfo] Upgrade DISubrange to support 
Fortran dynamic arrays (authored by alok, committed by SouraVX).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D80197?vs=266597&id=266770#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80197

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  llvm/include/llvm/IR/DIBuilder.h
  llvm/include/llvm/IR/DebugInfoMetadata.h
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/Bitcode/Reader/MetadataLoader.cpp
  llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
  llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
  llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
  llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
  llvm/lib/IR/AsmWriter.cpp
  llvm/lib/IR/DIBuilder.cpp
  llvm/lib/IR/DebugInfoMetadata.cpp
  llvm/lib/IR/LLVMContextImpl.h
  llvm/lib/IR/Verifier.cpp
  llvm/test/Assembler/debug-info.ll
  llvm/test/Assembler/disubrange-empty-array.ll
  llvm/test/Assembler/invalid-disubrange-count-missing.ll
  llvm/test/Bindings/llvm-c/debug_info.ll
  llvm/test/Bitcode/fortranSubrange.ll
  llvm/test/Bitcode/fortranSubrangeBackward.ll
  llvm/test/Bitcode/fortranSubrangeBackward.ll.bc
  llvm/test/DebugInfo/X86/default-subrange-array.ll
  llvm/test/DebugInfo/X86/nondefault-subrange-array.ll
  llvm/test/DebugInfo/cDefaultLower.ll
  llvm/test/DebugInfo/fortranDefaultLower.ll
  llvm/test/DebugInfo/fortranSubrangeExpr.ll
  llvm/test/DebugInfo/fortranSubrangeInt.ll
  llvm/test/DebugInfo/fortranSubrangeVar.ll
  llvm/test/Verifier/disubrange-count-upperBound.ll
  llvm/test/Verifier/disubrange-missing-upperBound.ll
  llvm/test/Verifier/invalid-disubrange-lowerBound.ll
  llvm/test/Verifier/invalid-disubrange-stride.ll
  llvm/test/Verifier/invalid-disubrange-upperBound.ll
  llvm/unittests/IR/MetadataTest.cpp

Index: llvm/unittests/IR/MetadataTest.cpp
===
--- llvm/unittests/IR/MetadataTest.cpp
+++ llvm/unittests/IR/MetadataTest.cpp
@@ -1139,11 +1139,12 @@
 TEST_F(DISubrangeTest, get) {
   auto *N = DISubrange::get(Context, 5, 7);
   auto Count = N->getCount();
+  auto Lower = N->getLowerBound();
   EXPECT_EQ(dwarf::DW_TAG_subrange_type, N->getTag());
   ASSERT_TRUE(Count);
   ASSERT_TRUE(Count.is());
   EXPECT_EQ(5, Count.get()->getSExtValue());
-  EXPECT_EQ(7, N->getLowerBound());
+  EXPECT_EQ(7, Lower.get()->getSExtValue());
   EXPECT_EQ(N, DISubrange::get(Context, 5, 7));
   EXPECT_EQ(DISubrange::get(Context, 5, 0), DISubrange::get(Context, 5));
 
@@ -1154,11 +1155,12 @@
 TEST_F(DISubrangeTest, getEmptyArray) {
   auto *N = DISubrange::get(Context, -1, 0);
   auto Count = N->getCount();
+  auto Lower = N->getLowerBound();
   EXPECT_EQ(dwarf::DW_TAG_subrange_type, N->getTag());
   ASSERT_TRUE(Count);
   ASSERT_TRUE(Count.is());
   EXPECT_EQ(-1, Count.get()->getSExtValue());
-  EXPECT_EQ(0, N->getLowerBound());
+  EXPECT_EQ(0, Lower.get()->getSExtValue());
   EXPECT_EQ(N, DISubrange::get(Context, -1, 0));
 }
 
@@ -1172,15 +1174,146 @@
 
   auto *N = DISubrange::get(Context, VlaExpr, 0);
   auto Count = N->getCount();
+  auto Lower = N->getLowerBound();
   ASSERT_TRUE(Count);
   ASSERT_TRUE(Count.is());
   EXPECT_EQ(VlaExpr, Count.get());
   ASSERT_TRUE(isa(N->getRawCountNode()));
-  EXPECT_EQ(0, N->getLowerBound());
+  EXPECT_EQ(0, Lower.get()->getSExtValue());
   EXPECT_EQ("vla_expr", Count.get()->getName());
   EXPECT_EQ(N, DISubrange::get(Context, VlaExpr, 0));
 }
 
+TEST_F(DISubrangeTest, fortranAllocatableInt) {
+  DILocalScope *Scope = getSubprogram();
+  DIFile *File = getFile();
+  DIType *Type = getDerivedType();
+  DINode::DIFlags Flags = static_cast(7);
+  auto *LI = ConstantAsMetadata::get(
+  ConstantInt::getSigned(Type::getInt64Ty(Context), -10));
+  auto *UI = ConstantAsMetadata::get(
+  ConstantInt::getSigned(Type::getInt64Ty(Context), 10));
+  auto *SI = ConstantAsMetadata::get(
+  ConstantInt::getSigned(Type::getInt64Ty(Context), 4));
+  auto *UIother = ConstantAsMetadata::get(
+  ConstantInt::getSigned(Type::getInt64Ty(Context), 20));
+  auto *UVother = DILocalVariable::get(Context, Scope, "ubother", File, 8, Type,
+   2, Flags, 8);
+  auto *UEother = DIExpression::get(Context, {5, 6});
+  auto *LIZero = ConstantAsMetadata::get(
+  ConstantInt::getSigned(Type::getInt64Ty(Context), 0));
+  auto *UIZero = ConstantAsMetadata::get(
+  ConstantInt::getSigned(Type::getInt64Ty(Context), 0));
+
+  auto *N = DISubrange::get(Context, nullptr, LI, UI, SI);
+
+  auto Lower = N->getLowerBound();
+  ASSERT_TRUE(Lower);
+  ASSERT_TRUE(Lower.is());
+  EXPECT_EQ(cast(LI->getValue()), Lower.get());
+
+  auto Upper = N->getUpperBound();
+  ASSERT_TRUE(Upper);
+  ASSERT_TRUE(Upper.is());
+  EXPECT_EQ(cast(UI->getValue()), Upper.get());

[PATCH] D80369: [DebugInfo][CallSites] Remove decl subprograms from 'retainedTypes:'

[Djordje Todorovic via Phabricator via cfe-commits]

djtodoro added a comment.

In D80369#2057866 , @dblaikie wrote:

> Not sure I follow - why was it a problem that there was no DISubprogram at 
> all?


Actually, since the DISubprograms from the retained types don't affect the 
final DWARF, it's not a problem at all. :)


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

https://reviews.llvm.org/D80369



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

[PATCH] D80697: [clang-tidy] Reworked TransformerClangTidyCheck to simplify usage of Options

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr2 added a reviewer: ymandel.
gribozavr2 added a subscriber: ymandel.
gribozavr2 added a comment.

@ymandel owns transformers.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80697



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

[PATCH] D80685: [ASTMatchers] Add traversal-kind support to `DynTypedMatcher`

[Dmitri Gribenko via Phabricator via cfe-commits]

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



Comment at: clang/lib/ASTMatchers/ASTMatchersInternal.cpp:234
+DynTypedMatcher::constructWithTraversalKind(DynTypedMatcher InnerMatcher,
+ast_type_traits::TraversalKind TK) 
{
+  InnerMatcher.Implementation =

It might read better as an instance method on `DynTypedMatcher`: 
`DynTypedMatcher::withTraversalKind()`. It is not unprecedented, see 
`dynCastTo()`.



Comment at: clang/unittests/ASTMatchers/ASTMatchersInternalTest.cpp:182
+  EXPECT_TRUE(TK.hasValue());
+  EXPECT_EQ(*TK, TK_AsIs);
+}

Please use `EXPECT_THAT(M.getTraversalKind(), llvm::ValueIs(TK_AsIs));` (also 
in tests below).

You'll need to include `llvm/Testing/Support/SupportHelpers.h`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80685



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

[PATCH] D80668: [Clang][Sanitizers] Expect test failure on {arm,thumb}v7

[Marco Elver via Phabricator via cfe-commits]

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

Add link to bug.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80668

Files:
  clang/test/CodeGen/sanitize-coverage.c


Index: clang/test/CodeGen/sanitize-coverage.c
===
--- clang/test/CodeGen/sanitize-coverage.c
+++ clang/test/CodeGen/sanitize-coverage.c
@@ -4,6 +4,9 @@
 // RUN: %clang %s -target x86_64-unknown-linux-gnu -emit-llvm -S 
-fsanitize=memory -fsanitize-coverage=trace-pc,trace-cmp -o - | FileCheck 
%s --check-prefixes=CHECK,MSAN
 // RUN: %clang %s -target x86_64-unknown-linux-gnu -emit-llvm -S 
-fsanitize=thread -fsanitize-coverage=trace-pc,trace-cmp -o - | FileCheck 
%s --check-prefixes=CHECK,TSAN
 // RUN: %clang %s -target x86_64-unknown-linux-gnu -emit-llvm -S 
-fsanitize=undefined  -fsanitize-coverage=trace-pc,trace-cmp -o - | FileCheck 
%s --check-prefixes=CHECK,UBSAN
+//
+// Host armv7 is currently unsupported: 
https://bugs.llvm.org/show_bug.cgi?id=46117
+// XFAIL: armv7, thumbv7
 
 int x[10];
 


Index: clang/test/CodeGen/sanitize-coverage.c
===
--- clang/test/CodeGen/sanitize-coverage.c
+++ clang/test/CodeGen/sanitize-coverage.c
@@ -4,6 +4,9 @@
 // RUN: %clang %s -target x86_64-unknown-linux-gnu -emit-llvm -S -fsanitize=memory -fsanitize-coverage=trace-pc,trace-cmp -o - | FileCheck %s --check-prefixes=CHECK,MSAN
 // RUN: %clang %s -target x86_64-unknown-linux-gnu -emit-llvm -S -fsanitize=thread -fsanitize-coverage=trace-pc,trace-cmp -o - | FileCheck %s --check-prefixes=CHECK,TSAN
 // RUN: %clang %s -target x86_64-unknown-linux-gnu -emit-llvm -S -fsanitize=undefined  -fsanitize-coverage=trace-pc,trace-cmp -o - | FileCheck %s --check-prefixes=CHECK,UBSAN
+//
+// Host armv7 is currently unsupported: https://bugs.llvm.org/show_bug.cgi?id=46117
+// XFAIL: armv7, thumbv7
 
 int x[10];
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D80369: [DebugInfo][CallSites] Remove decl subprograms from 'retainedTypes:'

[Djordje Todorovic via Phabricator via cfe-commits]

djtodoro updated this revision to Diff 266776.
djtodoro added a comment.

-Tests clean up


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

https://reviews.llvm.org/D80369

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/test/CodeGen/debug-info-extern-call.c
  clang/test/Modules/DebugInfoTransitiveImport.m
  clang/test/Modules/ModuleDebugInfo.cpp
  clang/test/Modules/ModuleDebugInfo.m

Index: clang/test/Modules/ModuleDebugInfo.m
===
--- clang/test/Modules/ModuleDebugInfo.m
+++ clang/test/Modules/ModuleDebugInfo.m
@@ -36,19 +36,13 @@
 // CHECK-NOT:  name:
 // CHECK-SAME: elements:
 
-// CHECK: !DISubprogram(name: "+[ObjCClass classMethod]",
-// CHECK-SAME:  scope: ![[MODULE]],
+// CHECK: !DICompositeType(tag: DW_TAG_structure_type, name: "FwdDecl",
+// CHECK-SAME: scope: ![[MODULE]],
 
 // CHECK: !DICompositeType(tag: DW_TAG_structure_type, name: "ObjCClass",
 // CHECK-SAME: scope: ![[MODULE]],
 // CHECK-SAME: elements
 
-// The forward declaration should not be in the module scope.
-// CHECK: !DICompositeType(tag: DW_TAG_structure_type, name: "OpaqueData", file
-
-// CHECK: !DICompositeType(tag: DW_TAG_structure_type, name: "FwdDecl",
-// CHECK-SAME: scope: ![[MODULE]],
-
 // CHECK: !DICompositeType(tag: DW_TAG_structure_type, name: "ObjCClassWithPrivateIVars",
 // CHECK-SAME: scope: ![[MODULE]],
 // CHECK-SAME: elements
@@ -85,11 +79,8 @@
 // The output order is sublty different for module vs. pch,
 // so these are checked separately:
 //
-// CHECK2: !DISubprogram(name: "+[ObjCClass classMethod]"
-// CHECK2: !DISubprogram(name: "-[ObjCClass instanceMethodWithInt:]"
+// CHECK2: !DICompositeType(tag: DW_TAG_structure_type, name: "FwdDecl",
 // CHECK2: !DICompositeType(tag: DW_TAG_structure_type, name: "ObjCClass",
 // CHECK2: !DIObjCProperty(name: "property",
 // CHECK2: !DIDerivedType(tag: DW_TAG_member, name: "ivar"
-// CHECK2: !DISubprogram(name: "-[Category(Category) categoryMethod]"
-// CHECK2: !DICompositeType(tag: DW_TAG_structure_type, name: "FwdDecl",
 // CHECK2: !DIDerivedType(tag: DW_TAG_typedef, name: "InnerEnum"
Index: clang/test/Modules/ModuleDebugInfo.cpp
===
--- clang/test/Modules/ModuleDebugInfo.cpp
+++ clang/test/Modules/ModuleDebugInfo.cpp
@@ -51,15 +51,6 @@
 // CHECK-SAME: )
 // CHECK: !DIEnumerator(name: "e5", value: 5, isUnsigned: true)
 
-// CHECK: !DIDerivedType(tag: DW_TAG_typedef, name: "B",
-// no mangled name here yet.
-
-// This type is anchored by a function parameter.
-// CHECK: !DICompositeType(tag: DW_TAG_class_type, name: "A"
-// CHECK-SAME: elements:
-// CHECK-SAME: templateParams:
-// CHECK-SAME: identifier: "_ZTSN8DebugCXX1AIJvEEE")
-
 // CHECK: !DICompositeType(tag: DW_TAG_structure_type, name: "Struct"
 // CHECK-SAME: identifier: "_ZTSN8DebugCXX6StructE")
 
@@ -85,6 +76,12 @@
 // CHECK-SAME: templateParams:
 // CHECK-SAME: identifier: "_ZTSN8DebugCXX8TemplateIlNS_6traitsIl")
 
+// This type is anchored by a function parameter.
+// CHECK: !DICompositeType(tag: DW_TAG_class_type, name: "A"
+// CHECK-SAME: elements:
+// CHECK-SAME: templateParams:
+// CHECK-SAME: identifier: "_ZTSN8DebugCXX1AIJvEEE")
+
 // CHECK: !DIDerivedType(tag: DW_TAG_typedef, name: "FloatInstantiation"
 // no mangled name here yet.
 
@@ -93,6 +90,9 @@
 // CHECK-SAME: flags: DIFlagFwdDecl
 // CHECK-SAME: identifier: "_ZTSN8DebugCXX8TemplateIfNS_6traitsIf")
 
+// CHECK: !DIDerivedType(tag: DW_TAG_typedef, name: "B",
+// no mangled name here yet.
+
 // CHECK: !DICompositeType(tag: DW_TAG_structure_type, name: "Virtual"
 // CHECK-SAME: elements:
 // CHECK-SAME: identifier: "_ZTS7Virtual")
Index: clang/test/Modules/DebugInfoTransitiveImport.m
===
--- clang/test/Modules/DebugInfoTransitiveImport.m
+++ clang/test/Modules/DebugInfoTransitiveImport.m
@@ -12,10 +12,10 @@
 
 // Definition of left:
 // CHECK: !DICompileUnit({{.*}}dwoId:
-// CHECK: ![[LEFT:[0-9]+]] = !DIFile({{.*}}diamond_left.h
 // CHECK: !DIImportedEntity(tag: DW_TAG_imported_declaration,
-// CHECK-SAME:  entity: ![[MODULE:.*]], file: ![[LEFT]], line: 3)
+// CHECK-SAME:  entity: ![[MODULE:.*]], file: ![[LEFT:.*]], line: 3)
 // CHECK: ![[MODULE]] = !DIModule(scope: null, name: "diamond_top"
+// CHECK: ![[LEFT]] = !DIFile({{.*}}diamond_left.h
 
 // Skeleton for top:
 // CHECK: !DICompileUnit({{.*}}splitDebugFilename: {{.*}}diamond_top{{.*}}dwoId:
Index: clang/test/CodeGen/debug-info-extern-call.c
===
--- clang/test/CodeGen/debug-info-extern-call.c
+++ clang/test/CodeGen/debug

[PATCH] D80699: [Analyzer][StreamChecker] Add check for pointer escape.

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

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

After an escaped FILE* stream handle it is not possible to make
reliable checks on it because any function call can have effect
on it.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D80699

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

Index: clang/test/Analysis/stream.c
===
--- clang/test/Analysis/stream.c
+++ clang/test/Analysis/stream.c
@@ -192,4 +192,51 @@
 rewind(f1); // expected-warning {{Stream might be invalid}}
 fclose(f1);
   }
-}
\ No newline at end of file
+}
+
+extern FILE *GlobalF;
+extern void takeFile(FILE *);
+
+void check_escape1() {
+  FILE *F = tmpfile();
+  if (!F)
+return;
+  fwrite("1", 1, 1, F); // may fail
+  GlobalF = F;
+  fwrite("1", 1, 1, F); // no warning
+}
+
+void check_escape2() {
+  FILE *F = tmpfile();
+  if (!F)
+return;
+  fwrite("1", 1, 1, F); // may fail
+  takeFile(F);
+  fwrite("1", 1, 1, F); // no warning
+}
+
+void check_escape3() {
+  FILE *F = tmpfile();
+  if (!F)
+return;
+  takeFile(F);
+  F = freopen(0, "w", F);
+  if (!F)
+return;
+  fwrite("1", 1, 1, F); // may fail
+  fwrite("1", 1, 1, F); // no warning
+}
+
+void check_escape4() {
+  FILE *F = tmpfile();
+  if (!F)
+return;
+  fwrite("1", 1, 1, F); // may fail
+
+  // no escape at (non-StreamChecker-handled) system call
+  // FIXME: all such calls should be handled by the checker
+  fprintf(F, "0");
+
+  fwrite("1", 1, 1, F); // expected-warning {{might be 'indeterminate'}}
+  fclose(F);
+}
Index: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -193,8 +193,8 @@
   return State;
 }
 
-class StreamChecker
-: public Checker {
+class StreamChecker : public Checker {
   mutable std::unique_ptr BT_nullfp, BT_illegalwhence,
   BT_UseAfterClose, BT_UseAfterOpenFailed, BT_ResourceLeak, BT_StreamEof,
   BT_IndeterminatePosition;
@@ -203,6 +203,10 @@
   void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
   bool evalCall(const CallEvent &Call, CheckerContext &C) const;
   void checkDeadSymbols(SymbolReaper &SymReaper, CheckerContext &C) const;
+  ProgramStateRef checkPointerEscape(ProgramStateRef State,
+ const InvalidatedSymbols &Escaped,
+ const CallEvent *Call,
+ PointerEscapeKind Kind) const;
 
   /// If true, evaluate special testing stream functions.
   bool TestMode = false;
@@ -428,10 +432,14 @@
 
   SymbolRef StreamSym = StreamVal->getAsSymbol();
   // Do not care about concrete values for stream ("(FILE *)0x12345"?).
-  // FIXME: Are stdin, stdout, stderr such values?
+  // FIXME: Can be stdin, stdout, stderr such values?
   if (!StreamSym)
 return;
 
+  // Do not handle untracked stream. It is probably escaped.
+  if (!State->get(StreamSym))
+return;
+
   // Generate state for non-failed case.
   // Return value is the passed stream pointer.
   // According to the documentations, the stream is closed first
@@ -932,6 +940,32 @@
   }
 }
 
+ProgramStateRef StreamChecker::checkPointerEscape(
+ProgramStateRef State, const InvalidatedSymbols &Escaped,
+const CallEvent *Call, PointerEscapeKind Kind) const {
+  // Check for file-handling system call that is not handled by the checker.
+  // FIXME: The checker should be updated to handle all system calls that take
+  // 'FILE*' argument. These are now ignored.
+  if (Kind == PSK_DirectEscapeOnCall && Call->isInSystemHeader())
+return State;
+
+  for (InvalidatedSymbols::const_iterator I = Escaped.begin(),
+  E = Escaped.end();
+   I != E; ++I) {
+SymbolRef Sym = *I;
+
+// The symbol escaped.
+// From now the stream can be manipulated in unknown way to the checker,
+// it is not possible to handle it any more.
+// Optimistically, assume that the corresponding file handle will be closed
+// somewhere else.
+// Remove symbol from state so the following stream calls on this symbol are
+// not handled by the checker.
+State = State->remove(Sym);
+  }
+  return State;
+}
+
 void ento::registerStreamChecker(CheckerManager &Mgr) {
   Mgr.registerChecker();
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D80606: [libTooling] Fix Transformer to work with ambient traversal kinds

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr2 accepted this revision.
gribozavr2 added inline comments.



Comment at: clang/lib/Tooling/Transformer/RewriteRule.cpp:154
+const RewriteRule &Rule,
+ast_type_traits::TraversalKind DefaultTraversalKind) {
   // Map the cases into buckets of matchers -- one for each "root" AST kind,

I don't see any callers using the new argument. Do we need this flexibility?



Comment at: clang/unittests/Tooling/TransformerTest.cpp:574
 
+// Verifies that a rule with a matcher for an implicit node (like
+// `implicitCastExpr`) does not change the code, when the traversal kind is not

a matcher => a top-level matcher



Comment at: clang/unittests/Tooling/TransformerTest.cpp:575
+// Verifies that a rule with a matcher for an implicit node (like
+// `implicitCastExpr`) does not change the code, when the traversal kind is not
+// explicitly set ti TK_AsIs). In this test, only the rule with the

I'd say "when the AST traversal skips implicit nodes".



Comment at: clang/unittests/Tooling/TransformerTest.cpp:576
+// `implicitCastExpr`) does not change the code, when the traversal kind is not
+// explicitly set ti TK_AsIs). In this test, only the rule with the
+// explicit-node matcher will fire.

ti => to

AsIs) => AsIs


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80606



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

[clang] 69935d8 - [Clang][Sanitizers] Expect test failure on {arm, thumb}v7

[Marco Elver via cfe-commits]

Author: Marco Elver
Date: 2020-05-28T11:33:32+02:00
New Revision: 69935d86aed1b691c5f33a2141f15cb3aaee1af6

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

LOG: [Clang][Sanitizers] Expect test failure on {arm,thumb}v7

Summary:
Versions of LLVM built on {arm,thumb}v7 appear to have differently
configured pass managers, which causes restrictions on which sanitizers
we may use.

As such, expect failure of the recently added "sanitize-coverage.c" test
on these architectures until we can investigate armv7's restrictions.

Bugzilla: https://bugs.llvm.org/show_bug.cgi?id=46117

Reviewers: vitalybuka, glider

Reviewed By: glider

Subscribers: glider, kristof.beyls, danielkiss, cfe-commits, vvereschaka

Tags: #clang

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

Added: 


Modified: 
clang/test/CodeGen/sanitize-coverage.c

Removed: 




diff  --git a/clang/test/CodeGen/sanitize-coverage.c 
b/clang/test/CodeGen/sanitize-coverage.c
index 6fc8e39354d4..ea4ac9296b48 100644
--- a/clang/test/CodeGen/sanitize-coverage.c
+++ b/clang/test/CodeGen/sanitize-coverage.c
@@ -4,6 +4,9 @@
 // RUN: %clang %s -target x86_64-unknown-linux-gnu -emit-llvm -S 
-fsanitize=memory -fsanitize-coverage=trace-pc,trace-cmp -o - | FileCheck 
%s --check-prefixes=CHECK,MSAN
 // RUN: %clang %s -target x86_64-unknown-linux-gnu -emit-llvm -S 
-fsanitize=thread -fsanitize-coverage=trace-pc,trace-cmp -o - | FileCheck 
%s --check-prefixes=CHECK,TSAN
 // RUN: %clang %s -target x86_64-unknown-linux-gnu -emit-llvm -S 
-fsanitize=undefined  -fsanitize-coverage=trace-pc,trace-cmp -o - | FileCheck 
%s --check-prefixes=CHECK,UBSAN
+//
+// Host armv7 is currently unsupported: 
https://bugs.llvm.org/show_bug.cgi?id=46117
+// XFAIL: armv7, thumbv7
 
 int x[10];
 



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

[PATCH] D80117: [analyzer] Introduce reasoning about symbolic remainder operator

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko added a comment.

Performance-wise, I've investigated huge slowdowns on `tmux` and `pytorch`.

- `pytorch` build produces a lot of warnings and simply trashed my terminal.  I 
guess one time it had more troubles with displaying all that than the other.  
Here is a table with new times:

|   | pytorch |
| - | --- |
| Time (before) | 2h21m33s|
| Time (after)  | 2h19m23s|
| Delta | {icon info-circle color=blue} -1.5% |
|

As you can see, these numbers are //way// smaller than the original ones.

- `tmux` is a much smaller project, so I decided to run it **20** times for 
each case.

F12018491: boxplot.png 
//After// consistently shows slower runtimes, but the overall difference (for 
median times) is only **+3%**.

I believe that as of now we can submit these modifications as is and explore 
performance optimizations later if needed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80117



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

[PATCH] D80668: [Clang][Sanitizers] Expect test failure on {arm,thumb}v7

[Marco Elver via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG69935d86aed1: [Clang][Sanitizers] Expect test failure on 
{arm,thumb}v7 (authored by melver).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80668

Files:
  clang/test/CodeGen/sanitize-coverage.c


Index: clang/test/CodeGen/sanitize-coverage.c
===
--- clang/test/CodeGen/sanitize-coverage.c
+++ clang/test/CodeGen/sanitize-coverage.c
@@ -4,6 +4,9 @@
 // RUN: %clang %s -target x86_64-unknown-linux-gnu -emit-llvm -S 
-fsanitize=memory -fsanitize-coverage=trace-pc,trace-cmp -o - | FileCheck 
%s --check-prefixes=CHECK,MSAN
 // RUN: %clang %s -target x86_64-unknown-linux-gnu -emit-llvm -S 
-fsanitize=thread -fsanitize-coverage=trace-pc,trace-cmp -o - | FileCheck 
%s --check-prefixes=CHECK,TSAN
 // RUN: %clang %s -target x86_64-unknown-linux-gnu -emit-llvm -S 
-fsanitize=undefined  -fsanitize-coverage=trace-pc,trace-cmp -o - | FileCheck 
%s --check-prefixes=CHECK,UBSAN
+//
+// Host armv7 is currently unsupported: 
https://bugs.llvm.org/show_bug.cgi?id=46117
+// XFAIL: armv7, thumbv7
 
 int x[10];
 


Index: clang/test/CodeGen/sanitize-coverage.c
===
--- clang/test/CodeGen/sanitize-coverage.c
+++ clang/test/CodeGen/sanitize-coverage.c
@@ -4,6 +4,9 @@
 // RUN: %clang %s -target x86_64-unknown-linux-gnu -emit-llvm -S -fsanitize=memory -fsanitize-coverage=trace-pc,trace-cmp -o - | FileCheck %s --check-prefixes=CHECK,MSAN
 // RUN: %clang %s -target x86_64-unknown-linux-gnu -emit-llvm -S -fsanitize=thread -fsanitize-coverage=trace-pc,trace-cmp -o - | FileCheck %s --check-prefixes=CHECK,TSAN
 // RUN: %clang %s -target x86_64-unknown-linux-gnu -emit-llvm -S -fsanitize=undefined  -fsanitize-coverage=trace-pc,trace-cmp -o - | FileCheck %s --check-prefixes=CHECK,UBSAN
+//
+// Host armv7 is currently unsupported: https://bugs.llvm.org/show_bug.cgi?id=46117
+// XFAIL: armv7, thumbv7
 
 int x[10];
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D79411: [VE] Clang toolchain for VE

[Simon Moll via Phabricator via cfe-commits]

simoll added a comment.

Ping. Anything i can do to get this reviewed?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79411



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

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

[Gabor Marton via Phabricator via cfe-commits]

martong added inline comments.



Comment at: 
clang/unittests/StaticAnalyzer/TestReturnValueUnderConstruction.cpp:24
+public:
+  void checkPostCall(const CallEvent &Call, CheckerContext &C) const {
+if (!Call.getOriginExpr())

I assume this tests this call expression: `returnC(1)` . But this is absolutely 
not trivial from the test code, could you please make this cleaner?


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

https://reviews.llvm.org/D80366



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

[PATCH] D80704: Remove WrapperMatcherInterface

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr created this revision.
Herald added subscribers: cfe-commits, arichardson.
Herald added a project: clang.
gribozavr2 added a reviewer: ymandel.

WrapperMatcherInterface is an abstraction over a member variable -- in
other words, not much of an abstraction at all. I think it makes code
harder to read more than in helps with deduplication. Not to even
mention the questionable usage of the ~Interface suffix for a type with
state.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D80704

Files:
  clang/include/clang/ASTMatchers/ASTMatchersInternal.h

Index: clang/include/clang/ASTMatchers/ASTMatchersInternal.h
===
--- clang/include/clang/ASTMatchers/ASTMatchersInternal.h
+++ clang/include/clang/ASTMatchers/ASTMatchersInternal.h
@@ -475,19 +475,6 @@
   IntrusiveRefCntPtr Implementation;
 };
 
-/// Wrapper base class for a wrapping matcher.
-///
-/// This is just a container for a DynTypedMatcher that can be used as a base
-/// class for another matcher.
-template 
-class WrapperMatcherInterface : public MatcherInterface {
-protected:
-  explicit WrapperMatcherInterface(DynTypedMatcher &&InnerMatcher)
-  : InnerMatcher(std::move(InnerMatcher)) {}
-
-  const DynTypedMatcher InnerMatcher;
-};
-
 /// Wrapper of a MatcherInterface *that allows copying.
 ///
 /// A Matcher can be used anywhere a Matcher is
@@ -558,10 +545,12 @@
   /// does only matches in the absence of qualifiers, or not, i.e. simply
   /// ignores any qualifiers.
   template 
-  class TypeToQualType : public WrapperMatcherInterface {
+  class TypeToQualType : public MatcherInterface {
+const DynTypedMatcher InnerMatcher;
+
   public:
 TypeToQualType(const Matcher &InnerMatcher)
-: TypeToQualType::WrapperMatcherInterface(InnerMatcher) {}
+: InnerMatcher(InnerMatcher) {}
 
 bool matches(const QualType &Node, ASTMatchFinder *Finder,
  BoundNodesTreeBuilder *Builder) const override {
@@ -750,13 +739,15 @@
 /// Type argument DeclMatcherT is required by PolymorphicMatcherWithParam1 but
 /// not actually used.
 template 
-class HasDeclarationMatcher : public WrapperMatcherInterface {
+class HasDeclarationMatcher : public MatcherInterface {
   static_assert(std::is_same>::value,
 "instantiated with wrong types");
 
+  const DynTypedMatcher InnerMatcher;
+
 public:
   explicit HasDeclarationMatcher(const Matcher &InnerMatcher)
-  : HasDeclarationMatcher::WrapperMatcherInterface(InnerMatcher) {}
+  : InnerMatcher(InnerMatcher) {}
 
   bool matches(const T &Node, ASTMatchFinder *Finder,
BoundNodesTreeBuilder *Builder) const override {
@@ -1167,14 +1158,14 @@
   }
 };
 
-template 
-class TraversalMatcher : public WrapperMatcherInterface {
+template  class TraversalMatcher : public MatcherInterface {
+  const DynTypedMatcher InnerMatcher;
   clang::TraversalKind Traversal;
 
 public:
-  explicit TraversalMatcher(clang::TraversalKind TK, const Matcher &ChildMatcher)
-  : TraversalMatcher::WrapperMatcherInterface(ChildMatcher), Traversal(TK) {
-  }
+  explicit TraversalMatcher(clang::TraversalKind TK,
+const Matcher &InnerMatcher)
+  : InnerMatcher(InnerMatcher), Traversal(TK) {}
 
   bool matches(const T &Node, ASTMatchFinder *Finder,
BoundNodesTreeBuilder *Builder) const override {
@@ -1323,10 +1314,12 @@
 ///
 /// ChildT must be an AST base type.
 template 
-class HasMatcher : public WrapperMatcherInterface {
+class HasMatcher : public MatcherInterface {
+  const DynTypedMatcher InnerMatcher;
+
 public:
-  explicit HasMatcher(const Matcher &ChildMatcher)
-  : HasMatcher::WrapperMatcherInterface(ChildMatcher) {}
+  explicit HasMatcher(const Matcher &InnerMatcher)
+  : InnerMatcher(InnerMatcher) {}
 
   bool matches(const T &Node, ASTMatchFinder *Finder,
BoundNodesTreeBuilder *Builder) const override {
@@ -1342,16 +1335,18 @@
 /// As opposed to the HasMatcher, the ForEachMatcher will produce a match
 /// for each child that matches.
 template 
-class ForEachMatcher : public WrapperMatcherInterface {
+class ForEachMatcher : public MatcherInterface {
   static_assert(IsBaseType::value,
 "for each only accepts base type matcher");
 
- public:
-   explicit ForEachMatcher(const Matcher &ChildMatcher)
-   : ForEachMatcher::WrapperMatcherInterface(ChildMatcher) {}
+  const DynTypedMatcher InnerMatcher;
 
-  bool matches(const T& Node, ASTMatchFinder* Finder,
-   BoundNodesTreeBuilder* Builder) const override {
+public:
+  explicit ForEachMatcher(const Matcher &InnerMatcher)
+  : InnerMatcher(InnerMatcher) {}
+
+  bool matches(const T &Node, ASTMatchFinder *Finder,
+   BoundNodesTreeBuilder *Builder) const override {
 return Finder->matchesChildOf(
 Node, this->InnerMatcher, Builder,
 TraversalKind::TK_IgnoreImplicitCastsAndParentheses,
@@ -1455,17 +1450

[PATCH] D78990: [analyzer] Allow bindings of the CompoundLiteralRegion

[Artem Dergachev via Phabricator via cfe-commits]

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

Great, thanks!




Comment at: clang/unittests/StaticAnalyzer/StoreTest.cpp:66
 
-Store StInit = StMgr.getInitialStore(SFC).getStore();
-SVal Zero = SVB.makeZeroVal(ACtx.IntTy);
-SVal One = SVB.makeIntVal(1, ACtx.IntTy);
-SVal NarrowZero = SVB.makeZeroVal(ACtx.CharTy);
+Store StInit = SManager.getInitialStore(SFC).getStore();
+SVal Zero = Builder.makeZeroVal(ASTCtxt.IntTy);

The reason i usually specify `S` to `St` is i want to avoid confusion with 
`SourceManager`. It still causes confusion with [Program]`StateManager` but 
those two rarely appear in the same code because `ProgramStateManager` provides 
wrappers for most of the `StoreManager`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78990



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

[PATCH] D80508: [AST] Fix the source range for TagDecl if there is no matched } brace.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Argh I never submitted these comments, sorry.




Comment at: clang/lib/AST/Decl.cpp:4129
 SourceRange TagDecl::getSourceRange() const {
-  SourceLocation RBraceLoc = BraceRange.getEnd();
-  SourceLocation E = RBraceLoc.isValid() ? RBraceLoc : getLocation();
+  SourceLocation E = BraceRange.getBegin().isValid() ?
+ BraceRange.getEnd() : getLocation();

I'm confused about this change.

We were using BR.end if it was valid, now we're using it if BR.start is valid. 
So this changes behavior in two cases:
 - BR.start is valid, BR.end is invalid: old was {getOuterLocStart(), 
getLocation()}. new is {getOuterLocStart(), invalid}
 - BR.end is valid, BR.start is invalid: old was {getOuterLocStart(), 
BraceRange.getEnd()}. new is {getOuterLocStart(), getLocation()}

These both seem worse to me, what am I missing?

Patch description refers to the first case, I wonder if this is just a bug in 
clangd cancelling out a bad AST (does it just mark tokens until EOF if the end 
location is invalid?)



Comment at: clang/test/AST/ast-dump-invalid-brace.cpp:6
+  int abc;
+// }

or `// missing: }`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80508



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

[PATCH] D79400: [CMAKE] Fix build failure when source directory is read only

[Pushpinder Singh via Phabricator via cfe-commits]

pdhaliwal updated this revision to Diff 266794.
pdhaliwal marked an inline comment as done.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79400

Files:
  llvm/cmake/modules/AddLLVM.cmake
  llvm/include/llvm/Support/CMakeLists.txt


Index: llvm/include/llvm/Support/CMakeLists.txt
===
--- llvm/include/llvm/Support/CMakeLists.txt
+++ llvm/include/llvm/Support/CMakeLists.txt
@@ -5,12 +5,16 @@
 
 set(generate_vcs_version_script 
"${LLVM_CMAKE_PATH}/GenerateVersionFromVCS.cmake")
 
-if(llvm_vc AND LLVM_APPEND_VC_REV)
+if(LLVM_APPEND_VC_REV)
   set(llvm_source_dir ${LLVM_MAIN_SRC_DIR})
+
+  if (NOT llvm_vc)
+set(fake_version_inc "${CMAKE_CURRENT_BINARY_DIR}/__FakeVCSRevision.h")
+  endif()
 endif()
 
 # Create custom target to generate the VC revision include.
-add_custom_command(OUTPUT "${version_inc}"
+add_custom_command(OUTPUT "${version_inc}" "${fake_version_inc}"
   DEPENDS "${llvm_vc}" "${generate_vcs_version_script}"
   COMMAND ${CMAKE_COMMAND} "-DNAMES=LLVM"
"-DLLVM_SOURCE_DIR=${llvm_source_dir}"
@@ -22,5 +26,5 @@
   PROPERTIES GENERATED TRUE
  HEADER_FILE_ONLY TRUE)
 
-add_custom_target(llvm_vcsrevision_h DEPENDS "${version_inc}")
+add_custom_target(llvm_vcsrevision_h ALL DEPENDS "${version_inc}" 
"${fake_version_inc}")
 set_target_properties(llvm_vcsrevision_h PROPERTIES FOLDER "Misc")
Index: llvm/cmake/modules/AddLLVM.cmake
===
--- llvm/cmake/modules/AddLLVM.cmake
+++ llvm/cmake/modules/AddLLVM.cmake
@@ -1913,34 +1913,27 @@
   if(NOT EXISTS "${path}")
 return()
   endif()
-  if(EXISTS "${path}/.svn")
-set(svn_files
-  "${path}/.svn/wc.db"   # SVN 1.7
-  "${path}/.svn/entries" # SVN 1.6
-)
-foreach(file IN LISTS svn_files)
-  if(EXISTS "${file}")
-set(${out_var} "${file}" PARENT_SCOPE)
-return()
-  endif()
-endforeach()
-  else()
-find_package(Git)
-if(GIT_FOUND)
-  execute_process(COMMAND ${GIT_EXECUTABLE} rev-parse --git-dir
-WORKING_DIRECTORY ${path}
-RESULT_VARIABLE git_result
-OUTPUT_VARIABLE git_output
-ERROR_QUIET)
-  if(git_result EQUAL 0)
-string(STRIP "${git_output}" git_output)
-get_filename_component(git_dir ${git_output} ABSOLUTE BASE_DIR ${path})
-# Some branchless cases (e.g. 'repo') may not yet have .git/logs/HEAD
-if (NOT EXISTS "${git_dir}/logs/HEAD")
-  file(WRITE "${git_dir}/logs/HEAD" "")
+  find_package(Git)
+  if(GIT_FOUND)
+execute_process(COMMAND ${GIT_EXECUTABLE} rev-parse --git-dir
+  WORKING_DIRECTORY ${path}
+  RESULT_VARIABLE git_result
+  OUTPUT_VARIABLE git_output
+  ERROR_QUIET)
+if(git_result EQUAL 0)
+  string(STRIP "${git_output}" git_output)
+  get_filename_component(git_dir ${git_output} ABSOLUTE BASE_DIR ${path})
+  # Some branchless cases (e.g. 'repo') may not yet have .git/logs/HEAD
+  if (NOT EXISTS "${git_dir}/logs/HEAD")
+execute_process(COMMAND ${CMAKE_COMMAND} -E touch HEAD
+  WORKING_DIRECTORY "${git_dir}/logs"
+  RESULT_VARIABLE touch_head_result
+  ERROR_QUIET)
+if (NOT touch_head_result EQUAL 0)
+  return()
 endif()
-set(${out_var} "${git_dir}/logs/HEAD" PARENT_SCOPE)
   endif()
+  set(${out_var} "${git_dir}/logs/HEAD" PARENT_SCOPE)
 endif()
   endif()
 endfunction()


Index: llvm/include/llvm/Support/CMakeLists.txt
===
--- llvm/include/llvm/Support/CMakeLists.txt
+++ llvm/include/llvm/Support/CMakeLists.txt
@@ -5,12 +5,16 @@
 
 set(generate_vcs_version_script "${LLVM_CMAKE_PATH}/GenerateVersionFromVCS.cmake")
 
-if(llvm_vc AND LLVM_APPEND_VC_REV)
+if(LLVM_APPEND_VC_REV)
   set(llvm_source_dir ${LLVM_MAIN_SRC_DIR})
+
+  if (NOT llvm_vc)
+set(fake_version_inc "${CMAKE_CURRENT_BINARY_DIR}/__FakeVCSRevision.h")
+  endif()
 endif()
 
 # Create custom target to generate the VC revision include.
-add_custom_command(OUTPUT "${version_inc}"
+add_custom_command(OUTPUT "${version_inc}" "${fake_version_inc}"
   DEPENDS "${llvm_vc}" "${generate_vcs_version_script}"
   COMMAND ${CMAKE_COMMAND} "-DNAMES=LLVM"
"-DLLVM_SOURCE_DIR=${llvm_source_dir}"
@@ -22,5 +26,5 @@
   PROPERTIES GENERATED TRUE
  HEADER_FILE_ONLY TRUE)
 
-add_custom_target(llvm_vcsrevision_h DEPENDS "${version_inc}")
+add_custom_target(llvm_vcsrevision_h ALL DEPENDS "${version_inc}" "${fake_version_inc}")
 set_target_properties(llvm_vcsrevision_h PROPERTIES FOLDER "Misc")
Index: llvm/cmake/modules/AddLLVM.cmake
===
--- llvm/cmake/modules/AddLLVM.cmake
+++ llvm/cmake/modules/AddLLVM.cmake
@@ -1913,34 +1913,27 @@
   if(NOT EXISTS "

[PATCH] D79117: [clang] [Darwin] Add reverse mappings for aarch64/aarch64_32 to darwin arch names

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a reviewer: rjmccall.
mstorsjo added a comment.

Adding some more Apple reviewers on this one - who can give it a look?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79117



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

[PATCH] D79400: [CMAKE] Fix build failure when source directory is read only

[Pushpinder Singh via Phabricator via cfe-commits]

pdhaliwal added inline comments.



Comment at: llvm/cmake/modules/AddLLVM.cmake:1945
+  if (NOT touch_head_result EQUAL 0)
+return()
+  endif()

scott.linder wrote:
> This seems to implement the behavior that when the log is not present and is 
> not writable the VCS header target is never re-build. Would it instead make 
> more sense to conservatively assume it should //always// be rebuilt? The 
> latter case is what is implemented by the stackoverflow snippet you mentioned 
> previously, right?
> 
> If the assumption is that the failure to write the logs file implies this 
> source is read-only, then it may make sense to assume it never needs to be 
> rebuilt. However if the sources are changed in another context (e.g. `sudo -u 
> user-with-write-privs git checkout rev`) this won't trigger a rebuild until 
> the user re-runs the CMake config step manually.
> 
> I think I would lean towards the conservative approach of always rebuilding, 
> but this patch as it stands is still an improvement over just falling over in 
> this case.
To retrigger the build when `.git/logs/HEAD` is not writable, I have added 
solution from first answer of the linked stackoverflow. Given my limited 
knowledge of cmake, please let me know if that is correct.

Right now, the old behaviour is preserved as it is for read-write context.

Also, ninja output on nth build for same branch/tag/commit for read-only source 
with missing `.git/logs/HEAD`,
```
root@8c167cb7d5b9:/src/build# ninja -j8
[1/93] Generating VCSRevision.h, __FakeVCSRevision.h
-- Found Git: /usr/bin/git (found version "2.17.1") 
root@8c167cb7d5b9:/src/build#
```



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79400



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

[clang] d283fc4 - [DebugInfo] Use SplitTemplateClosers (foo >) in DWARF too

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2020-05-28T12:30:38+02:00
New Revision: d283fc4f9d07a5f3334fe682ccabfc16e8d2933b

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

LOG: [DebugInfo] Use SplitTemplateClosers (foo >) in DWARF too

Summary:
D76801 caused some regressions in debuginfo compatibility by changing how
certain functions were named.

For CodeView we try to mirror MSVC exactly: this was fixed in a549c0d00486
For DWARF the situation is murkier. Per David Blaikie:
> In general DWARF doesn't specify this at all.
> [...]
> This isn't the only naming divergence between GCC and Clang

Nevertheless, including the space seems to provide better compatibility with
GCC and GDB. E.g. cpexprs.cc in the GDB testsuite requires this formatting.
And there was no particular desire to change the printing of names in debug
info in the first place (just in diagnostics and other more user-facing text).

Fixes PR46052

Reviewers: dblaikie, labath

Subscribers: aprantl, cfe-commits, dyung

Tags: #clang

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

Added: 


Modified: 
clang/lib/CodeGen/CGDebugInfo.cpp
clang/test/CodeGenCXX/debug-info-template-explicit-specialization.cpp
clang/test/Modules/ExtDebugInfo.cpp
clang/test/Modules/ModuleDebugInfo.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGDebugInfo.cpp 
b/clang/lib/CodeGen/CGDebugInfo.cpp
index 4e0b6aa0dca6..31f8df243017 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -236,6 +236,10 @@ PrintingPolicy CGDebugInfo::getPrintingPolicy() const {
   if (CGM.getCodeGenOpts().EmitCodeView) {
 PP.MSVCFormatting = true;
 PP.SplitTemplateClosers = true;
+  } else {
+// For DWARF, printing rules are underspecified.
+// SplitTemplateClosers yields better interop with GCC and GDB (PR46052).
+PP.SplitTemplateClosers = true;
   }
 
   // Apply -fdebug-prefix-map.

diff  --git 
a/clang/test/CodeGenCXX/debug-info-template-explicit-specialization.cpp 
b/clang/test/CodeGenCXX/debug-info-template-explicit-specialization.cpp
index d97d82b76965..4e41c4092bf4 100644
--- a/clang/test/CodeGenCXX/debug-info-template-explicit-specialization.cpp
+++ b/clang/test/CodeGenCXX/debug-info-template-explicit-specialization.cpp
@@ -110,7 +110,7 @@ struct j_wrap {
 };
 j_wrap> j_wrap_j;
 // CHECK: DICompositeType(tag: DW_TAG_structure_type, name: "j"
-// CHECK: DICompositeType(tag: DW_TAG_structure_type, name: "j_wrap>"
+// CHECK: DICompositeType(tag: DW_TAG_structure_type, name: "j_wrap >"
 
 template 
 struct k {

diff  --git a/clang/test/Modules/ExtDebugInfo.cpp 
b/clang/test/Modules/ExtDebugInfo.cpp
index 6781810d592c..aff2953b4bb5 100644
--- a/clang/test/Modules/ExtDebugInfo.cpp
+++ b/clang/test/Modules/ExtDebugInfo.cpp
@@ -85,14 +85,14 @@ void foo() {
 
 // This type is not anchored in the module by an explicit template 
instantiation.
 // CHECK: !DICompositeType(tag: DW_TAG_class_type,
-// CHECK-SAME: name: "Template>",
+// CHECK-SAME: name: "Template >",
 // CHECK-SAME: scope: ![[NS]],
 // CHECK-SAME: elements:
 // CHECK-SAME: identifier: 
"_ZTSN8DebugCXX8TemplateIlNS_6traitsIl")
 
 // This type is anchored in the module by an explicit template instantiation.
 // CHECK: !DICompositeType(tag: DW_TAG_class_type,
-// CHECK-SAME: name: "Template>",
+// CHECK-SAME: name: "Template >",
 // CHECK-SAME: scope: ![[NS]],
 // CHECK-SAME: flags: DIFlagFwdDecl
 // CHECK-SAME: identifier: 
"_ZTSN8DebugCXX8TemplateIiNS_6traitsIi")
@@ -103,7 +103,7 @@ void foo() {
 
 // This one isn't.
 // CHECK: !DICompositeType(tag: DW_TAG_class_type,
-// CHECK-SAME: name: "Template>",
+// CHECK-SAME: name: "Template >",
 // CHECK-SAME: scope: ![[NS]],
 // CHECK-SAME: elements:
 // CHECK-SAME: templateParams:

diff  --git a/clang/test/Modules/ModuleDebugInfo.cpp 
b/clang/test/Modules/ModuleDebugInfo.cpp
index 26369c896058..e6e99ed4e537 100644
--- a/clang/test/Modules/ModuleDebugInfo.cpp
+++ b/clang/test/Modules/ModuleDebugInfo.cpp
@@ -65,7 +65,7 @@
 
 // This type is anchored by an explicit template instantiation.
 // CHECK: !DICompositeType(tag: DW_TAG_class_type,
-// CHECK-SAME: name: "Template>"
+// CHECK-SAME: name: "Template >"
 // CHECK-SAME: elements:
 // CHECK-SAME: templateParams:
 // CHECK-SAME: identifier: 
"_ZTSN8DebugCXX8TemplateIiNS_6traitsIi")
@@ -80,7 +80,7 @@
 // CHECK-SAME: identifier: "_ZTSN8DebugCXX6traitsIfEE")
 
 // CHECK: !DICompositeType(tag: DW_TAG_class_type,
-// CHECK-SAME: name: "Template>"
+// CHECK-SAME: name: "Temp

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

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

balazske added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp:112
 
+Optional ExprEngine::retrieveFromConstructionContext(
+ProgramStateRef State, const LocationContext *LCtx,

baloghadamsoftware wrote:
> baloghadamsoftware wrote:
> > baloghadamsoftware wrote:
> > > baloghadamsoftware wrote:
> > > > NoQ wrote:
> > > > > baloghadamsoftware wrote:
> > > > > > NoQ wrote:
> > > > > > > Please instead re-use the code that computes the object under 
> > > > > > > construction. That'll save you ~50 lines of code and will be more 
> > > > > > > future-proof (eg., standalone temporaries without destructor 
> > > > > > > technically have a construction context with 0 items so when we 
> > > > > > > implement them correctly your procedure will stop working).
> > > > > > That was so my first thought. However, 
> > > > > > `handleConstructionContext()` is private and non-static. Now I 
> > > > > > tried to merge the two methods: if the value is already in the 
> > > > > > construction context, we return it, if not then we add it. Is this 
> > > > > > what you suggest? Or did I misunderstand you? At the very beginning 
> > > > > > I tried to simply use `handleConstructionContext()`, but it 
> > > > > > asserted because the value was already in the map.
> > > > > I'd like to preserve the assertion that objects-under-construction 
> > > > > are never filled twice; it's a very useful sanity check. What you 
> > > > > need in your checker is a function that computes 
> > > > > object-under-construction but doesn't put it into the 
> > > > > objects-under-construction map. So you have to separate the 
> > > > > computation from filling in the state.
> > > > OK, so I (fortunately) misundertood you. Thus I should refactor this 
> > > > function to a calculation and a storing part?
> > > OK, I see what you are speaking about, but I have no idea how to do it 
> > > properly. The problem is that the control logic of filling in the state 
> > > also depends on the kind of the construction context. For some kinds we 
> > > do not fill at all. Every way I try it becomes more complex and less 
> > > correct:
> > > 
> > > 1) `NewAllocatedObjectKind`: we do not add this to the state, we only 
> > > retrieve the original.
> > > 2) `SimpleReturnedValueKind` and `CXX17ElidedCopyReturnedValueKind`: 
> > > depending on whether we are in top frame we handle this case recursively 
> > > or we do not fill at all, just return the value. What is the construction 
> > > context item here? Maybe the `ReturnStmt`?
> > > 3) `ElidedTemporaryObjectKind`: this is the most problematic: we first 
> > > handle it recursively for the construction context after elision, then we 
> > > also fill it for the elided temporary object construction context as well.
> > > 
> > > The only thing I can (maybe) do is to retrieve the construction context 
> > > item. But then the switch is still duplicated for filling, because of the 
> > > different control logic for different construction context kinds.
> > > The only thing I can (maybe) do is to retrieve the construction context 
> > > item.
> > 
> > This does not help even for retrieving the value, because its control logic 
> > also depends on the construction context kind. If I do it, it will be code 
> > triplication instead of duplication and results in a code that is worse to 
> > understand than the current one.
> > 
> > 
> > Please instead re-use the code that computes the object under construction. 
> > That'll save you ~50 lines of code and will be more future-proof (eg., 
> > standalone temporaries without destructor technically have a construction 
> > context with 0 items so when we implement them correctly your procedure 
> > will stop working).
> 
> Any solution I can come up with rather adds 100 to 150 lines to the code 
> instead of saving 50. For retrieving we only have to determine the 
> construction context item (the key). For storing we also have to calculate 
> the value. It sounds good to calculate these things in separate functions and 
> then have a simple retriever and store function but there are lots of 
> recursions, double strores, non-stores, retrieving in the store function that 
> make it too complicated.
> 
> Also retrieving is more complex than just determining the item and getting 
> the value from the context: if you look at `SimpleReturnedValueKind` and 
> `CXX17ElidedCopyReturnedValueKind` you can see that we do not use the 
> construction context we got in the parameter (the construction context of the 
> return value) but the construction context of the call instead. For 
> `ElidedTemporaryObjectKind` we take the construction context before the 
> elusion.
> 
> Future proofness: I agree, this is a problem to solve. In cases where the 
> construction context is empty maybe we can move the calculation of the values 
> to a separate function that is called by both the "handler" and the 
> "retriever".
Do I thin

[PATCH] D69764: [clang-format] Add East/West Const fixer capability

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay updated this revision to Diff 266799.
MyDeveloperDay added a comment.

Minor change for the simpler review comments before refactoring some of the 
more involved ones


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

https://reviews.llvm.org/D69764

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/CMakeLists.txt
  clang/lib/Format/EastWestConstFixer.cpp
  clang/lib/Format/EastWestConstFixer.h
  clang/lib/Format/Format.cpp
  clang/tools/clang-format/ClangFormat.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -13628,6 +13628,13 @@
   CHECK_PARSE("ContinuationIndentWidth: 11", ContinuationIndentWidth, 11u);
   CHECK_PARSE("CommentPragmas: '// abc$'", CommentPragmas, "// abc$");
 
+  Style.ConstPlacement = FormatStyle::CS_West;
+  CHECK_PARSE("ConstPlacement: Leave", ConstPlacement, FormatStyle::CS_Leave);
+  CHECK_PARSE("ConstPlacement: East", ConstPlacement, FormatStyle::CS_East);
+  CHECK_PARSE("ConstPlacement: West", ConstPlacement, FormatStyle::CS_West);
+  CHECK_PARSE("ConstPlacement: Right", ConstPlacement, FormatStyle::CS_East);
+  CHECK_PARSE("ConstPlacement: Left", ConstPlacement, FormatStyle::CS_West);
+
   Style.PointerAlignment = FormatStyle::PAS_Middle;
   CHECK_PARSE("PointerAlignment: Left", PointerAlignment,
   FormatStyle::PAS_Left);
@@ -16572,6 +16579,326 @@
"}",
Style);
 }
+
+TEST_F(FormatTest, EastWestConst) {
+  FormatStyle Style = getLLVMStyle();
+
+  // keep the const style unaltered
+  verifyFormat("const int a;", Style);
+  verifyFormat("const int *a;", Style);
+  verifyFormat("const int &a;", Style);
+  verifyFormat("const int &&a;", Style);
+  verifyFormat("int const b;", Style);
+  verifyFormat("int const *b;", Style);
+  verifyFormat("int const &b;", Style);
+  verifyFormat("int const &&b;", Style);
+  verifyFormat("int const *b const;", Style);
+  verifyFormat("int *const c;", Style);
+
+  verifyFormat("const Foo a;", Style);
+  verifyFormat("const Foo *a;", Style);
+  verifyFormat("const Foo &a;", Style);
+  verifyFormat("const Foo &&a;", Style);
+  verifyFormat("Foo const b;", Style);
+  verifyFormat("Foo const *b;", Style);
+  verifyFormat("Foo const &b;", Style);
+  verifyFormat("Foo const &&b;", Style);
+  verifyFormat("Foo const *b const;", Style);
+
+  verifyFormat("LLVM_NODISCARD const int &Foo();", Style);
+  verifyFormat("LLVM_NODISCARD int const &Foo();", Style);
+
+  verifyFormat("volatile const int *restrict;", Style);
+  verifyFormat("const volatile int *restrict;", Style);
+  verifyFormat("const int volatile *restrict;", Style);
+}
+
+TEST_F(FormatTest, EastConst) {
+  FormatStyle Style = getLLVMStyle();
+  Style.ConstPlacement = FormatStyle::CS_East;
+
+  
+  verifyFormat("int const a;", Style);
+  verifyFormat("int const *a;", Style);
+  verifyFormat("int const &a;", Style);
+  verifyFormat("int const &&a;", Style);
+  verifyFormat("int const b;", Style);
+  verifyFormat("int const *b;", Style);
+  verifyFormat("int const &b;", Style);
+  verifyFormat("int const &&b;", Style);
+  verifyFormat("int const *b const;", Style);
+  verifyFormat("int *const c;", Style);
+
+  verifyFormat("Foo const a;", Style);
+  verifyFormat("Foo const *a;", Style);
+  verifyFormat("Foo const &a;", Style);
+  verifyFormat("Foo const &&a;", Style);
+  verifyFormat("Foo const b;", Style);
+  verifyFormat("Foo const *b;", Style);
+  verifyFormat("Foo const &b;", Style);
+  verifyFormat("Foo const &&b;", Style);
+  verifyFormat("Foo const *b const;", Style);
+  verifyFormat("Foo *const b;", Style);
+  verifyFormat("Foo const *const b;", Style);
+  verifyFormat("auto const v = get_value();", Style);
+  verifyFormat("long long const &a;", Style);
+  verifyFormat("unsigned char const *a;", Style);
+  verifyFormat("int main(int const argc, char const *const *const argv)",
+   Style);
+
+  verifyFormat("LLVM_NODISCARD int const &Foo();", Style);
+  verifyFormat("SourceRange getSourceRange() const override LLVM_READONLY",
+   Style);
+  verifyFormat("void foo() const override;", Style);
+  verifyFormat("void foo() const override LLVM_READONLY;", Style);
+  verifyFormat("void foo() const final;", Style);
+  verifyFormat("void foo() const final LLVM_READONLY;", Style);
+  verifyFormat("void foo() const LLVM_READONLY;", Style);
+
+  verifyFormat(
+  "template  explicit Action(Action const &action);",
+  Style);
+  verifyFormat(
+  "template  explicit Action(Action const &action);",
+  "template  explicit Action(const Action& action);",
+  Style);
+  verifyFormat(
+  "template  explicit Action(Action const &action);",
+  "template \nexplicit Action(const Action& action);",
+  Style);
+
+  v

[PATCH] D80554: [DebugInfo] Use SplitTemplateClosers (foo >) in DWARF too

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGd283fc4f9d07: [DebugInfo] Use SplitTemplateClosers 
(foo >) in DWARF too (authored by sammccall).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80554

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/test/CodeGenCXX/debug-info-template-explicit-specialization.cpp
  clang/test/Modules/ExtDebugInfo.cpp
  clang/test/Modules/ModuleDebugInfo.cpp


Index: clang/test/Modules/ModuleDebugInfo.cpp
===
--- clang/test/Modules/ModuleDebugInfo.cpp
+++ clang/test/Modules/ModuleDebugInfo.cpp
@@ -65,7 +65,7 @@
 
 // This type is anchored by an explicit template instantiation.
 // CHECK: !DICompositeType(tag: DW_TAG_class_type,
-// CHECK-SAME: name: "Template>"
+// CHECK-SAME: name: "Template >"
 // CHECK-SAME: elements:
 // CHECK-SAME: templateParams:
 // CHECK-SAME: identifier: 
"_ZTSN8DebugCXX8TemplateIiNS_6traitsIi")
@@ -80,7 +80,7 @@
 // CHECK-SAME: identifier: "_ZTSN8DebugCXX6traitsIfEE")
 
 // CHECK: !DICompositeType(tag: DW_TAG_class_type,
-// CHECK-SAME: name: "Template>"
+// CHECK-SAME: name: "Template >"
 // CHECK-SAME: elements:
 // CHECK-SAME: templateParams:
 // CHECK-SAME: identifier: 
"_ZTSN8DebugCXX8TemplateIlNS_6traitsIl")
@@ -89,7 +89,7 @@
 // no mangled name here yet.
 
 // CHECK: !DICompositeType(tag: DW_TAG_class_type,
-// CHECK-SAME: name: "Template>"
+// CHECK-SAME: name: "Template >"
 // CHECK-SAME: flags: DIFlagFwdDecl
 // CHECK-SAME: identifier: 
"_ZTSN8DebugCXX8TemplateIfNS_6traitsIf")
 
Index: clang/test/Modules/ExtDebugInfo.cpp
===
--- clang/test/Modules/ExtDebugInfo.cpp
+++ clang/test/Modules/ExtDebugInfo.cpp
@@ -85,14 +85,14 @@
 
 // This type is not anchored in the module by an explicit template 
instantiation.
 // CHECK: !DICompositeType(tag: DW_TAG_class_type,
-// CHECK-SAME: name: "Template>",
+// CHECK-SAME: name: "Template >",
 // CHECK-SAME: scope: ![[NS]],
 // CHECK-SAME: elements:
 // CHECK-SAME: identifier: 
"_ZTSN8DebugCXX8TemplateIlNS_6traitsIl")
 
 // This type is anchored in the module by an explicit template instantiation.
 // CHECK: !DICompositeType(tag: DW_TAG_class_type,
-// CHECK-SAME: name: "Template>",
+// CHECK-SAME: name: "Template >",
 // CHECK-SAME: scope: ![[NS]],
 // CHECK-SAME: flags: DIFlagFwdDecl
 // CHECK-SAME: identifier: 
"_ZTSN8DebugCXX8TemplateIiNS_6traitsIi")
@@ -103,7 +103,7 @@
 
 // This one isn't.
 // CHECK: !DICompositeType(tag: DW_TAG_class_type,
-// CHECK-SAME: name: "Template>",
+// CHECK-SAME: name: "Template >",
 // CHECK-SAME: scope: ![[NS]],
 // CHECK-SAME: elements:
 // CHECK-SAME: templateParams:
Index: clang/test/CodeGenCXX/debug-info-template-explicit-specialization.cpp
===
--- clang/test/CodeGenCXX/debug-info-template-explicit-specialization.cpp
+++ clang/test/CodeGenCXX/debug-info-template-explicit-specialization.cpp
@@ -110,7 +110,7 @@
 };
 j_wrap> j_wrap_j;
 // CHECK: DICompositeType(tag: DW_TAG_structure_type, name: "j"
-// CHECK: DICompositeType(tag: DW_TAG_structure_type, name: "j_wrap>"
+// CHECK: DICompositeType(tag: DW_TAG_structure_type, name: "j_wrap >"
 
 template 
 struct k {
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -236,6 +236,10 @@
   if (CGM.getCodeGenOpts().EmitCodeView) {
 PP.MSVCFormatting = true;
 PP.SplitTemplateClosers = true;
+  } else {
+// For DWARF, printing rules are underspecified.
+// SplitTemplateClosers yields better interop with GCC and GDB (PR46052).
+PP.SplitTemplateClosers = true;
   }
 
   // Apply -fdebug-prefix-map.


Index: clang/test/Modules/ModuleDebugInfo.cpp
===
--- clang/test/Modules/ModuleDebugInfo.cpp
+++ clang/test/Modules/ModuleDebugInfo.cpp
@@ -65,7 +65,7 @@
 
 // This type is anchored by an explicit template instantiation.
 // CHECK: !DICompositeType(tag: DW_TAG_class_type,
-// CHECK-SAME: name: "Template>"
+// CHECK-SAME: name: "Template >"
 // CHECK-SAME: elements:
 // CHECK-SAME: templateParams:
 // CHECK-SAME: identifier: "_ZTSN8DebugCXX8TemplateIiNS_6traitsIi")
@@ -80,7 +80,7 @@
 // CHECK-SAME: identifier: "_ZTSN8DebugCXX6traitsIfEE")
 
 /

[PATCH] D80117: [analyzer] Introduce reasoning about symbolic remainder operator

[Artem Dergachev via Phabricator via cfe-commits]

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

Aha, so performance regressions on real code weren't real, that's a relief :)

> I believe that as of now we can submit these modifications as is and explore 
> performance optimizations later if needed.

I still encourage you to explore the tests we have from our previous attempts 
to simplify expressions recursively without memoization 
(`test/Analysis/hangs.c`). I'm asking because these aren't all that artificial: 
this kind of code was previously reported by a frustrated user as "the analyzer 
started hanging on my code". Like, please replace a bunch of `+`es with 
`&`/`|`/`%` and see if this causes your code to perform exponentially over the 
size of the program. If so, i'd rather have us hurry up and implement 
memoization.

The math in this patch looks great, thanks!




Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:506-507
+  ///
+  /// where abs(Origin) is the maximal absolute value of any possible values
+  ///   from Origin, and min(T) is a minimal value for the type T.
+  ///

I suggest not trying to express signed types and unsigned types in a single 
formula, the reader will have to unwrap it back into the two cases anyway in 
order to understand what's going on.

The following would imho be easier to read: "If T is signed, return the 
smallest range `[-x..x]` that covers the original range, or `[-min(T), max(T)]` 
if the aforementioned symmetric range doesn't exist due to original range 
covering `min(T)`). If T is unsigned, return the smallest range `[0..x]` that 
covers the original range".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80117



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

[clang] bd06c41 - [analyzer] Allow bindings of the CompoundLiteralRegion

[Valeriy Savchenko via cfe-commits]

Author: Valeriy Savchenko
Date: 2020-05-28T14:11:57+03:00
New Revision: bd06c417e6c717cbe33b566d7bbaf27fb47e763a

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

LOG: [analyzer] Allow bindings of the CompoundLiteralRegion

Summary:
CompoundLiteralRegions have been properly modeled before, but
'getBindingForElement` was not changed to accommodate this change
properly.

rdar://problem/46144644

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

Added: 
clang/test/Analysis/retain-release-compound-literal.m

Modified: 
clang/lib/StaticAnalyzer/Core/RegionStore.cpp
clang/test/Analysis/compound-literals.c
clang/unittests/StaticAnalyzer/StoreTest.cpp

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Core/RegionStore.cpp 
b/clang/lib/StaticAnalyzer/Core/RegionStore.cpp
index 2a55c9964712..57fde32bc01d 100644
--- a/clang/lib/StaticAnalyzer/Core/RegionStore.cpp
+++ b/clang/lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -1628,10 +1628,6 @@ 
RegionStoreManager::findLazyBinding(RegionBindingsConstRef B,
 
 SVal RegionStoreManager::getBindingForElement(RegionBindingsConstRef B,
   const ElementRegion* R) {
-  // We do not currently model bindings of the CompoundLiteralregion.
-  if (isa(R->getBaseRegion()))
-return UnknownVal();
-
   // Check if the region has a binding.
   if (const Optional &V = B.getDirectBinding(R))
 return *V;

diff  --git a/clang/test/Analysis/compound-literals.c 
b/clang/test/Analysis/compound-literals.c
index f8b9121494c1..42e6a55a30c7 100644
--- a/clang/test/Analysis/compound-literals.c
+++ b/clang/test/Analysis/compound-literals.c
@@ -1,4 +1,7 @@
-// RUN: %clang_cc1 -triple=i386-apple-darwin10 -analyze 
-analyzer-checker=debug.ExprInspection -verify %s
+// RUN: %clang_cc1 -triple=i386-apple-darwin10 -verify %s -analyze \
+// RUN:   -analyzer-checker=debug.ExprInspection
+
+#define NULL 0
 void clang_analyzer_eval(int);
 
 // pr28449: Used to crash.
@@ -6,3 +9,15 @@ void foo(void) {
   static const unsigned short array[] = (const unsigned short[]){0x0F00};
   clang_analyzer_eval(array[0] == 0x0F00); // expected-warning{{TRUE}}
 }
+
+// check that we propagate info through compound literal regions
+void bar() {
+  int *integers = (int[]){1, 2, 3};
+  clang_analyzer_eval(integers[0] == 1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(integers[1] == 2); // expected-warning{{TRUE}}
+  clang_analyzer_eval(integers[2] == 3); // expected-warning{{TRUE}}
+
+  int **pointers = (int *[]){&integers[0], NULL};
+  clang_analyzer_eval(pointers[0] == NULL); // expected-warning{{FALSE}}
+  clang_analyzer_eval(pointers[1] == NULL); // expected-warning{{TRUE}}
+}

diff  --git a/clang/test/Analysis/retain-release-compound-literal.m 
b/clang/test/Analysis/retain-release-compound-literal.m
new file mode 100644
index ..29a125346363
--- /dev/null
+++ b/clang/test/Analysis/retain-release-compound-literal.m
@@ -0,0 +1,25 @@
+// RUN: %clang_analyze_cc1 -verify -Wno-objc-root-class %s \
+// RUN:   -analyzer-checker=core,osx.cocoa.RetainCount
+
+#define NULL 0
+#define CF_RETURNS_RETAINED __attribute__((cf_returns_retained))
+#define CF_CONSUMED __attribute__((cf_consumed))
+
+void clang_analyzer_eval(int);
+
+typedef const void *CFTypeRef;
+
+extern CFTypeRef CFCreate() CF_RETURNS_RETAINED;
+extern CFTypeRef CFRetain(CFTypeRef cf);
+extern void CFRelease(CFTypeRef cf);
+
+void bar(CFTypeRef *v) {}
+
+void test1() {
+  CFTypeRef *values = (CFTypeRef[]){
+  CFCreate(),  // no-warning
+  CFCreate(),  // expected-warning{{leak}}
+  CFCreate()}; // no-warning
+  CFRelease(values[0]);
+  CFRelease(values[2]);
+}

diff  --git a/clang/unittests/StaticAnalyzer/StoreTest.cpp 
b/clang/unittests/StaticAnalyzer/StoreTest.cpp
index c8b930bf3247..17b64ce622f8 100644
--- a/clang/unittests/StaticAnalyzer/StoreTest.cpp
+++ b/clang/unittests/StaticAnalyzer/StoreTest.cpp
@@ -15,89 +15,139 @@ namespace clang {
 namespace ento {
 namespace {
 
+class StoreTestConsumer : public ExprEngineConsumer {
+public:
+  StoreTestConsumer(CompilerInstance &C) : ExprEngineConsumer(C) {}
+
+  bool HandleTopLevelDecl(DeclGroupRef DG) override {
+for (const auto *D : DG)
+  performTest(D);
+return true;
+  }
+
+private:
+  virtual void performTest(const Decl *D) = 0;
+};
+
+template  class TestAction : public ASTFrontendAction {
+public:
+  std::unique_ptr CreateASTConsumer(CompilerInstance &Compiler,
+ StringRef File) override {
+return std::make_unique(Compiler);
+  }
+};
+
 // Test that we can put a value into an int-type variable and load it
 // back from that variable. Test what happens if default bindings are used.
-class VariableBindConsumer : public ExprEngineConsu

[PATCH] D80197: [DebugInfo] Upgrade DISubrange to support Fortran dynamic arrays

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

This breaks check-llvm on Windows: http://45.33.8.238/win/16214/step_11.txt

Please take a look and revert if it takes a while to investigate.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80197



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

[PATCH] D69764: [clang-format] Add East/West Const fixer capability

[MyDeveloperDay via Phabricator via cfe-commits]

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



Comment at: clang/lib/Format/EastWestConstFixer.cpp:130
+
+static void swapFourTokens(const SourceManager &SourceMgr,
+ tooling::Replacements &Fixes,

curdeius wrote:
> These functions seem a bit ugly... and very specific. And they both look like 
> rotate left/right. Couldn't it be a single function taking a 
> range/span/collection of FormatTokens?
This is something I'd also started to feel the same, now I'm starting to handle 
longer sets of qualifier, I can replace all these swap functions for one single 
rotate.

```
lang=c++
static void rotateTokens(const SourceManager &SourceMgr,
 tooling::Replacements &Fixes, const FormatToken *First,
 const FormatToken *Last, bool Left) {
  auto *End = Last;
  auto *Begin = First;
  if (!Left) {
End = Last->Next;
Begin = First->Next;
  }

  std::string NewText;
  // If we are rotating to the left we move the Last token to the front.
  if (Left) {
NewText += Last->TokenText;
NewText += " ";
  }

  // Then move through the other tokens.
  auto *Tok = Begin;
  while (Tok != End) {
if (!NewText.empty())
  NewText += " ";

NewText += Tok->TokenText;
Tok = Tok->Next;
  }

  // If we are rotating to the right we move the first token to the back.
  if (!Left) {
NewText += " ";
NewText += First->TokenText;
  }

  auto Range = CharSourceRange::getCharRange(First->getStartOfNonWhitespace(),
 Last->Tok.getEndLoc());
  auto Err = Fixes.add(tooling::Replacement(SourceMgr, Range, NewText));
  if (Err) {
llvm::errs() << "Error while rearranging const : "
 << llvm::toString(std::move(Err)) << "\n";
  }
}
```


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

https://reviews.llvm.org/D69764



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

[PATCH] D78990: [analyzer] Allow bindings of the CompoundLiteralRegion

[Valeriy Savchenko via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGbd06c417e6c7: [analyzer] Allow bindings of the 
CompoundLiteralRegion (authored by vsavchenko).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78990

Files:
  clang/lib/StaticAnalyzer/Core/RegionStore.cpp
  clang/test/Analysis/compound-literals.c
  clang/test/Analysis/retain-release-compound-literal.m
  clang/unittests/StaticAnalyzer/StoreTest.cpp

Index: clang/unittests/StaticAnalyzer/StoreTest.cpp
===
--- clang/unittests/StaticAnalyzer/StoreTest.cpp
+++ clang/unittests/StaticAnalyzer/StoreTest.cpp
@@ -15,89 +15,139 @@
 namespace ento {
 namespace {
 
+class StoreTestConsumer : public ExprEngineConsumer {
+public:
+  StoreTestConsumer(CompilerInstance &C) : ExprEngineConsumer(C) {}
+
+  bool HandleTopLevelDecl(DeclGroupRef DG) override {
+for (const auto *D : DG)
+  performTest(D);
+return true;
+  }
+
+private:
+  virtual void performTest(const Decl *D) = 0;
+};
+
+template  class TestAction : public ASTFrontendAction {
+public:
+  std::unique_ptr CreateASTConsumer(CompilerInstance &Compiler,
+ StringRef File) override {
+return std::make_unique(Compiler);
+  }
+};
+
 // Test that we can put a value into an int-type variable and load it
 // back from that variable. Test what happens if default bindings are used.
-class VariableBindConsumer : public ExprEngineConsumer {
-  void performTest(const Decl *D) {
-StoreManager &StMgr = Eng.getStoreManager();
-SValBuilder &SVB = Eng.getSValBuilder();
-MemRegionManager &MRMgr = StMgr.getRegionManager();
-const ASTContext &ACtx = Eng.getContext();
+class VariableBindConsumer : public StoreTestConsumer {
+  void performTest(const Decl *D) override {
+StoreManager &SManager = Eng.getStoreManager();
+SValBuilder &Builder = Eng.getSValBuilder();
+MemRegionManager &MRManager = SManager.getRegionManager();
+const ASTContext &ASTCtxt = Eng.getContext();
 
 const auto *VDX0 = findDeclByName(D, "x0");
 const auto *VDY0 = findDeclByName(D, "y0");
 const auto *VDZ0 = findDeclByName(D, "z0");
 const auto *VDX1 = findDeclByName(D, "x1");
 const auto *VDY1 = findDeclByName(D, "y1");
-assert(VDX0 && VDY0 && VDZ0 && VDX1 && VDY1);
+
+ASSERT_TRUE(VDX0 && VDY0 && VDZ0 && VDX1 && VDY1);
 
 const StackFrameContext *SFC =
 Eng.getAnalysisDeclContextManager().getStackFrame(D);
 
-Loc LX0 = loc::MemRegionVal(MRMgr.getVarRegion(VDX0, SFC));
-Loc LY0 = loc::MemRegionVal(MRMgr.getVarRegion(VDY0, SFC));
-Loc LZ0 = loc::MemRegionVal(MRMgr.getVarRegion(VDZ0, SFC));
-Loc LX1 = loc::MemRegionVal(MRMgr.getVarRegion(VDX1, SFC));
-Loc LY1 = loc::MemRegionVal(MRMgr.getVarRegion(VDY1, SFC));
+Loc LX0 = loc::MemRegionVal(MRManager.getVarRegion(VDX0, SFC));
+Loc LY0 = loc::MemRegionVal(MRManager.getVarRegion(VDY0, SFC));
+Loc LZ0 = loc::MemRegionVal(MRManager.getVarRegion(VDZ0, SFC));
+Loc LX1 = loc::MemRegionVal(MRManager.getVarRegion(VDX1, SFC));
+Loc LY1 = loc::MemRegionVal(MRManager.getVarRegion(VDY1, SFC));
 
-Store StInit = StMgr.getInitialStore(SFC).getStore();
-SVal Zero = SVB.makeZeroVal(ACtx.IntTy);
-SVal One = SVB.makeIntVal(1, ACtx.IntTy);
-SVal NarrowZero = SVB.makeZeroVal(ACtx.CharTy);
+Store StInit = SManager.getInitialStore(SFC).getStore();
+SVal Zero = Builder.makeZeroVal(ASTCtxt.IntTy);
+SVal One = Builder.makeIntVal(1, ASTCtxt.IntTy);
+SVal NarrowZero = Builder.makeZeroVal(ASTCtxt.CharTy);
 
 // Bind(Zero)
-Store StX0 =
-StMgr.Bind(StInit, LX0, Zero).getStore();
-ASSERT_EQ(Zero, StMgr.getBinding(StX0, LX0, ACtx.IntTy));
+Store StX0 = SManager.Bind(StInit, LX0, Zero).getStore();
+EXPECT_EQ(Zero, SManager.getBinding(StX0, LX0, ASTCtxt.IntTy));
 
 // BindDefaultInitial(Zero)
 Store StY0 =
-StMgr.BindDefaultInitial(StInit, LY0.getAsRegion(), Zero).getStore();
-ASSERT_EQ(Zero, StMgr.getBinding(StY0, LY0, ACtx.IntTy));
-ASSERT_EQ(Zero, *StMgr.getDefaultBinding(StY0, LY0.getAsRegion()));
+SManager.BindDefaultInitial(StInit, LY0.getAsRegion(), Zero).getStore();
+EXPECT_EQ(Zero, SManager.getBinding(StY0, LY0, ASTCtxt.IntTy));
+EXPECT_EQ(Zero, *SManager.getDefaultBinding(StY0, LY0.getAsRegion()));
 
 // BindDefaultZero()
-Store StZ0 =
-StMgr.BindDefaultZero(StInit, LZ0.getAsRegion()).getStore();
+Store StZ0 = SManager.BindDefaultZero(StInit, LZ0.getAsRegion()).getStore();
 // BindDefaultZero wipes the region with '0 S8b', not with out Zero.
 // Direct load, however, does give us back the object of the type
 // that we specify for loading.
-ASSERT_EQ(Zero, StMgr.getBinding(StZ0, LZ0, ACtx.IntTy));
-ASSERT_EQ(NarrowZero, *StMgr.getDefaultBinding(St

[clang-tools-extra] a56141b - [clangd] Highlight related control flow.

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2020-05-28T13:25:11+02:00
New Revision: a56141b8f9fea112c1ea078c974d91949b6e7a5c

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

LOG: [clangd] Highlight related control flow.

Summary:
This means e.g. highlighting "return" will show other returns/throws
from the same function, highlighting a case will show all the
return/breaks etc.

This is a bit of an abuse of textDocument/highlight, but seems useful.

Reviewers: adamcz

Subscribers: ilya-biryukov, MaskRay, jkorous, mgrang, arphaman, kadircet, 
usaxena95, cfe-commits

Tags: #clang

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

Added: 


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

Removed: 




diff  --git a/clang-tools-extra/clangd/XRefs.cpp 
b/clang-tools-extra/clangd/XRefs.cpp
index 1fc0e0348d09..7de1dc53596e 100644
--- a/clang-tools-extra/clangd/XRefs.cpp
+++ b/clang-tools-extra/clangd/XRefs.cpp
@@ -27,8 +27,12 @@
 #include "clang/AST/Attrs.inc"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclCXX.h"
+#include "clang/AST/DeclObjC.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/ExprCXX.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/Stmt.h"
+#include "clang/AST/StmtCXX.h"
 #include "clang/AST/Type.h"
 #include "clang/Basic/CharInfo.h"
 #include "clang/Basic/LLVM.h"
@@ -45,6 +49,7 @@
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/None.h"
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
@@ -712,35 +717,304 @@ findRefs(const std::vector &Decls, 
ParsedAST &AST) {
   return std::move(RefFinder).take();
 }
 
+const Stmt *getFunctionBody(DynTypedNode N) {
+  if (const auto *FD = N.get())
+return FD->getBody();
+  if (const auto *FD = N.get())
+return FD->getBody();
+  if (const auto *FD = N.get())
+return FD->getBody();
+  if (const auto *FD = N.get())
+return FD->getBody();
+  return nullptr;
+}
+
+const Stmt *getLoopBody(DynTypedNode N) {
+  if (const auto *LS = N.get())
+return LS->getBody();
+  if (const auto *LS = N.get())
+return LS->getBody();
+  if (const auto *LS = N.get())
+return LS->getBody();
+  if (const auto *LS = N.get())
+return LS->getBody();
+  return nullptr;
+}
+
+// AST traversal to highlight control flow statements under some root.
+// Once we hit further control flow we prune the tree (or at least restrict
+// what we highlight) so we capture e.g. breaks from the outer loop only.
+class FindControlFlow : public RecursiveASTVisitor {
+  // Types of control-flow statements we might highlight.
+  enum Target {
+Break = 1,
+Continue = 2,
+Return = 4,
+Case = 8,
+Throw = 16,
+Goto = 32,
+All = Break | Continue | Return | Case | Throw | Goto,
+  };
+  int Ignore = 0; // bitmask of Target - what are we *not* highlighting?
+  SourceRange Bounds; // Half-open, restricts reported targets.
+  std::vector &Result;
+  const SourceManager &SM;
+
+  // Masks out targets for a traversal into D.
+  // Traverses the subtree using Delegate() if any targets remain.
+  template 
+  bool filterAndTraverse(DynTypedNode D, const Func &Delegate) {
+auto RestoreIgnore = llvm::make_scope_exit(
+[OldIgnore(Ignore), this] { Ignore = OldIgnore; });
+if (getFunctionBody(D))
+  Ignore = All;
+else if (getLoopBody(D))
+  Ignore |= Continue | Break;
+else if (D.get())
+  Ignore |= Break | Case;
+// Prune tree if we're not looking for anything.
+return (Ignore == All) ? true : Delegate();
+  }
+
+  void found(Target T, SourceLocation Loc) {
+if (T & Ignore)
+  return;
+if (SM.isBeforeInTranslationUnit(Loc, Bounds.getBegin()) ||
+SM.isBeforeInTranslationUnit(Bounds.getEnd(), Loc))
+  return;
+Result.push_back(Loc);
+  }
+
+public:
+  FindControlFlow(SourceRange Bounds, std::vector &Result,
+  const SourceManager &SM)
+  : Bounds(Bounds), Result(Result), SM(SM) {}
+
+  // When traversing function or loops, limit targets to those that still
+  // refer to the original root.
+  bool TraverseDecl(Decl *D) {
+return !D || filterAndTraverse(DynTypedNode::create(*D), [&] {
+  return RecursiveASTVisitor::TraverseDecl(D);
+});
+  }
+  bool TraverseStmt(Stmt *S) {
+return !S || filterAndTraverse(DynTypedNode::create(*S), [&] {
+  return RecursiveASTVisitor::TraverseStmt(S);
+});
+  }
+
+  // Add leaves that we found and want.
+  bool VisitReturnStmt(ReturnStmt *R) {
+found(Return, R->getReturnLoc());
+return true;
+  }
+  bool VisitBreakStmt(BreakStmt *B) {
+found(Break, B->getBreakLoc());
+return true;
+  }

[PATCH] D80293: [clangd] Run PreambleThread in async mode behind a flag

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet marked 22 inline comments as done.
kadircet added inline comments.



Comment at: clang-tools-extra/clangd/TUScheduler.cpp:724
std::move(Req.WantDiags));
+// Set it after notifying ASTPeer about the preamble to prevent any races.
+BuiltFirst.notify();

sammccall wrote:
> hmm, we're notifying the ASTPeer, and then setting builtfirst which is being 
> waited on... by astpeer.
> This suggests to me the AST thread should own the notification, not the 
> preamble thread.
> And in fact it already has a notification for the first preamble being built! 
> why can't we use that one?
umm, it is a little bit more complicated.

The above call only inserts the "update request" onto ASTPeer's queue and 
doesn't update the preamble inside ASTPeer. This enables ASTPeer to access 
`LatestPreamble` without holding the lock, as it is the only writer. We can 
change that, update the `LatestPreamble` while queueing the request.



Comment at: clang-tools-extra/clangd/TUScheduler.cpp:1133
 
 bool ASTWorker::blockUntilIdle(Deadline Timeout) const {
   std::unique_lock Lock(Mutex);

sammccall wrote:
> I think this would be clearer as a loop... and maybe more correct.
as discussed offline, loop version isn't more clear keeping the straight 
version.



Comment at: clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp:466
   updateWithDiags(
-  S, File, Inputs, WantDiagnostics::Auto,
+  S, File, Inputs, WantDiagnostics::Yes,
   [File, Nonce, &Mut, &TotalUpdates](std::vector) {

sammccall wrote:
> As you've pointed out, Yes isn't used in production. So if the behaviour of 
> Auto has changed we should test that rather than stop using it.
> 
> My understanding of the change:
> 1. we've asserted that every update yields diagnostics
> 2. this was true because we were calling runWithAST, this forced an AST build 
> at each snapshot, and we'd publish the diagnostics
> 3. now we only publish diagnostics once the version has been through the 
> preamble thread and back
> 4. these requests get coalesced in the preamble thread if the AST worker is 
> pushing them faster than the preamble worker is servicing them
> 5. so we no longer publish diagnostics for every version even if we have a 
> suitable AST
> 
> Is this right?
> If so, I'd assert that TotalUpdates is at least FilesCount and at most 
> FilesCount * UpdatesPerFile, and also that the latest UpdateI for diags (for 
> any file) is equal to UpdatesPerFile - 1.
> And maybe add a brief explanation (we drop diagnostics for some snapshots as 
> they get coalesced in the preamble worker).
> Is this right?

yes. in addition to those, previously `Auto` promised a diagnostics release if 
it was followed by a read(that's how this test used to work). we no longer 
guarantee that for the reasons you've mentioned.


What about keeping `WantDiagnostics::Yes` at all? I believe it should be a 
separate patch none the less, but I would be happy with silently converting Yes 
to Auto. I would expect this to only break editors not using `Auto` at all and 
always forcing diags or not requesting them at all.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80293



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

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

[Gabor Marton via Phabricator via cfe-commits]

martong added inline comments.



Comment at: 
clang/unittests/StaticAnalyzer/TestReturnValueUnderConstruction.cpp:28
+
+Optional RetVal = Call.getReturnValueUnderConstruction(0);
+assert(RetVal);

How do we know that `0` is the proper block count?


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

https://reviews.llvm.org/D80366



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

[PATCH] D50078: clang-format: support aligned nested conditionals formatting

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir added a comment.

In D50078#2053324 , @krasimir wrote:

> I'm happy with this patch!
>
> We found a couple of rough edges:
>
> - alignment after `?:` and
> - new formatting of `_ ? _ ? _ : _ : _` patterns is bad
>
>   These are illustrated as examples D and E below (A, B and C look good to 
> me). `test.cc` is how I'd expect clang-format to behave with this patch with 
> `BreakBeforeTernaryOperators = true`, like in LLVM style.
> - alignment after `?:`: this is a GNU extension and we've seen it used in C++ 
> and ObjC: https://stackoverflow.com/questions/24538096/in-objective-c. I 
> think this is special enough for us to consider an occurrence of `?:` to 
> break a ternary operator chain for the purposes of this alignment. I'd expect 
> a +4 alignment of the last 2 lines of example D for consistency with A.
> - new formatting doesn't work for `_ ? _ ? _ : _ : _` patterns; old 
> formatting is better in that case. I think the new chained alignment works 
> very well for _ ? _ : _ ? _ : _ ... cases, but we should attempt it otherwise.
>
>
>
>   % cat test.cc
>   //-- column 80 
> V
>   // A
>   int i = aa 
>   ? bbb
>   : ;
>   
>   //-- column 80 
> V
>   // B
>   int i = aa ? bbb
>   :  ? 
>  : e;
>   
>   //-- column 80 
> V
>   // C
>   int i = aa
>   ?: bbb ? c
>  : d;
>   
>   //-- column 80 
> V
>   // D
>   int i = aa
>   ?: 
>  ? bbb
>  : ;
>   
>   //-- column 80 
> V
>   // E 
>   return temp[0] > temp[1] ? temp[0] > temp[2] ? 0 : 2
>: temp[1] > temp[2] ? 1 : 2;
>
>
>
>
>   % bin/clang-format -style=llvm test.cc
>   //-- column 80 
> V
>   // A
>   int i = aa
>   ? bbb
>   : ;
>   
>   //-- column 80 
> V
>   // B
>   int i = aa ? bbb
>   :  ? 
>  : e;
>   
>   //-- column 80 
> V
>   // C
>   int i = aa
>   ?: bbb ? c
>  : d;
>   
>   //-- column 80 
> V
>   // D
>   int i = aa
>   ?: 
>  ? bbb
>  : ;
>   
>   //-- column 80 
> V
>   // E
>   return temp[0] > temp[1]   ? temp[0] > temp[2] ? 0 : 2
>  : temp[1] > temp[2] ? 1
>  : 2;
>


@MyDeveloperDay , @Typz -- could you please take a look at these two issues and 
give your feedback? (E) looks pretty bad. I'm not familiar enough with this 
patch to judge how easy / hard would it be to mitigate these.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D50078



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

[PATCH] D80293: [clangd] Run PreambleThread in async mode behind a flag

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

- Address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80293

Files:
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/ParsedAST.cpp
  clang-tools-extra/clangd/ParsedAST.h
  clang-tools-extra/clangd/TUScheduler.cpp
  clang-tools-extra/clangd/TUScheduler.h
  clang-tools-extra/clangd/tool/ClangdMain.cpp
  clang-tools-extra/clangd/unittests/ClangdTests.cpp
  clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp

Index: clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
===
--- clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
+++ clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
@@ -21,14 +21,20 @@
 #include "support/Threading.h"
 #include "clang/Basic/DiagnosticDriver.h"
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/FunctionExtras.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/ScopeExit.h"
+#include "llvm/ADT/StringExtras.h"
+#include "llvm/ADT/StringMap.h"
+#include "llvm/ADT/StringRef.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include 
 #include 
 #include 
 #include 
+#include 
+#include 
 #include 
 
 namespace clang {
@@ -407,6 +413,7 @@
   int TotalASTReads = 0;
   int TotalPreambleReads = 0;
   int TotalUpdates = 0;
+  llvm::StringMap LatestDiagVersion;
 
   // Run TUScheduler and collect some stats.
   {
@@ -441,15 +448,23 @@
 auto Inputs = getInputs(File, Contents.str());
 {
   WithContextValue WithNonce(NonceKey, ++Nonce);
-  Inputs.Version = std::to_string(Nonce);
+  Inputs.Version = std::to_string(UpdateI);
   updateWithDiags(
   S, File, Inputs, WantDiagnostics::Auto,
-  [File, Nonce, &Mut, &TotalUpdates](std::vector) {
+  [File, Nonce, Version(Inputs.Version), &Mut, &TotalUpdates,
+   &LatestDiagVersion](std::vector) {
 EXPECT_THAT(Context::current().get(NonceKey), Pointee(Nonce));
 
 std::lock_guard Lock(Mut);
 ++TotalUpdates;
 EXPECT_EQ(File, *TUScheduler::getFileBeingProcessedInContext());
+// Make sure Diags are for a newer version.
+auto It = LatestDiagVersion.try_emplace(File, -1);
+const int PrevVersion = It.first->second;
+int CurVersion;
+ASSERT_TRUE(llvm::to_integer(Version, CurVersion, 10));
+EXPECT_LT(PrevVersion, CurVersion);
+It.first->getValue() = CurVersion;
   });
 }
 {
@@ -494,7 +509,13 @@
   } // TUScheduler destructor waits for all operations to finish.
 
   std::lock_guard Lock(Mut);
-  EXPECT_EQ(TotalUpdates, FilesCount * UpdatesPerFile);
+  // Updates might get coalesced in preamble thread and result in dropping
+  // diagnostics for intermediate snapshots.
+  EXPECT_GE(TotalUpdates, FilesCount);
+  EXPECT_LE(TotalUpdates, FilesCount * UpdatesPerFile);
+  // We should receive diags for last update.
+  for (const auto &Entry : LatestDiagVersion)
+EXPECT_EQ(Entry.second, UpdatesPerFile - 1);
   EXPECT_EQ(TotalASTReads, FilesCount * UpdatesPerFile);
   EXPECT_EQ(TotalPreambleReads, FilesCount * UpdatesPerFile);
 }
@@ -972,6 +993,57 @@
   EXPECT_NEAR(25, Compute({}), 0.01) << "no history -> max";
 }
 
+TEST_F(TUSchedulerTests, AsyncPreambleThread) {
+  // Blocks preamble thread while building preamble with \p BlockVersion until
+  // \p N is notified.
+  class BlockPreambleThread : public ParsingCallbacks {
+  public:
+BlockPreambleThread(llvm::StringRef BlockVersion, Notification &N)
+: BlockVersion(BlockVersion), N(N) {}
+void onPreambleAST(PathRef Path, llvm::StringRef Version, ASTContext &Ctx,
+   std::shared_ptr PP,
+   const CanonicalIncludes &) override {
+  if (Version == BlockVersion)
+N.wait();
+}
+
+  private:
+llvm::StringRef BlockVersion;
+Notification &N;
+  };
+
+  static constexpr llvm::StringLiteral InputsV0 = "v0";
+  static constexpr llvm::StringLiteral InputsV1 = "v1";
+  Notification Ready;
+  TUScheduler S(CDB, optsForTest(),
+std::make_unique(InputsV1, Ready));
+
+  Path File = testPath("foo.cpp");
+  auto PI = getInputs(File, "");
+  PI.Version = InputsV0.str();
+  S.update(File, PI, WantDiagnostics::Auto);
+  S.blockUntilIdle(timeoutSeconds(10));
+
+  // Block preamble builds.
+  PI.Version = InputsV1.str();
+  // Issue second update which will block preamble thread.
+  S.update(File, PI, WantDiagnostics::Auto);
+
+  Notification RunASTAction;
+  // Issue an AST read, which shouldn't be blocked and see latest version of the
+  // file.
+  S.runWithAST("test", File, [&](Expected

[PATCH] D78454: [clangd] Highlight related control flow.

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
sammccall marked an inline comment as done.
Closed by commit rGa56141b8f9fe: [clangd] Highlight related control flow. 
(authored by sammccall).

Changed prior to commit:
  https://reviews.llvm.org/D78454?vs=264734&id=266817#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78454

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

Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -116,6 +116,141 @@
   }
 }
 
+TEST(HighlightsTest, ControlFlow) {
+  const char *Tests[] = {
+  R"cpp(
+// Highlight same-function returns.
+int fib(unsigned n) {
+  if (n <= 1) [[ret^urn]] 1;
+  [[return]] fib(n - 1) + fib(n - 2);
+
+  // Returns from other functions not highlighted.
+  auto Lambda = [] { return; };
+  class LocalClass { void x() { return; } };
+}
+  )cpp",
+
+  R"cpp(
+#define FAIL() return false
+#define DO(x) { x; }
+bool foo(int n) {
+  if (n < 0) [[FAIL]]();
+  DO([[re^turn]] true)
+}
+  )cpp",
+
+  R"cpp(
+// Highlight loop control flow
+int magic() {
+  int counter = 0;
+  [[^for]] (char c : "fruit loops!") {
+if (c == ' ') [[continue]];
+counter += c;
+if (c == '!') [[break]];
+if (c == '?') [[return]] -1;
+  }
+  return counter;
+}
+  )cpp",
+
+  R"cpp(
+// Highlight loop and same-loop control flow
+void nonsense() {
+  [[while]] (true) {
+if (false) [[bre^ak]];
+switch (1) break;
+[[continue]];
+  }
+}
+  )cpp",
+
+  R"cpp(
+// Highlight switch for break (but not other breaks).
+void describe(unsigned n) {
+  [[switch]](n) {
+  case 0:
+break;
+  [[default]]:
+[[^break]];
+  }
+}
+  )cpp",
+
+  R"cpp(
+// Highlight case and exits for switch-break (but not other cases).
+void describe(unsigned n) {
+  [[switch]](n) {
+  case 0:
+break;
+  [[case]] 1:
+  [[default]]:
+[[return]];
+[[^break]];
+  }
+}
+  )cpp",
+
+  R"cpp(
+// Highlight exits and switch for case
+void describe(unsigned n) {
+  [[switch]](n) {
+  case 0:
+break;
+  [[case]] 1:
+  [[d^efault]]:
+[[return]];
+[[break]];
+  }
+}
+  )cpp",
+
+  R"cpp(
+// Highlight nothing for switch.
+void describe(unsigned n) {
+  s^witch(n) {
+  case 0:
+break;
+  case 1:
+  default:
+return;
+break;
+  }
+}
+  )cpp",
+
+  R"cpp(
+// FIXME: match exception type against catch blocks
+int catchy() {
+  try { // wrong: highlight try with matching catch
+try {   // correct: has no matching catch
+  [[thr^ow]] "oh no!";
+} catch (int) { }   // correct: catch doesn't match type
+[[return]] -1;  // correct: exits the matching catch
+  } catch (const char*) { } // wrong: highlight matching catch
+  [[return]] 42;// wrong: throw doesn't exit function
+}
+  )cpp",
+
+  R"cpp(
+// Loop highlights goto exiting the loop, but not jumping within it.
+void jumpy() {
+  [[wh^ile]](1) {
+up:
+if (0) [[goto]] out;
+goto up;
+  }
+  out: return;
+}
+  )cpp",
+  };
+  for (const char *Test : Tests) {
+Annotations T(Test);
+auto AST = TestTU::withCode(T.code()).build();
+EXPECT_THAT(findDocumentHighlights(AST, T.point()), HighlightsFrom(T))
+<< Test;
+  }
+}
+
 MATCHER_P3(Sym, Name, Decl, DefOrNone, "") {
   llvm::Optional Def = DefOrNone;
   if (Name != arg.Name) {
Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -27,8 +27,12 @@
 #include "clang/AST/Attrs.inc"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclCXX.h"
+#include "clang/AST/DeclObjC.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/ExprCXX.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/Stmt.h"
+#include "clang/AST/StmtCXX.h"
 #include "clang/AST/Type.h"
 #include "c

[clang] f9e94eb - [Clang] Enable _Complex __float128

[Nemanja Ivanovic via cfe-commits]

Author: Nemanja Ivanovic
Date: 2020-05-28T06:55:49-05:00
New Revision: f9e94eb8688d1fe1727360462e957fbbfb754e59

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

LOG: [Clang] Enable _Complex __float128

When I added __float128 a while ago, I neglected to add support for the complex
variant of the type. This patch just adds that.

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

Added: 


Modified: 
clang/lib/Sema/DeclSpec.cpp
clang/test/CodeGen/ppc64-complex-parms.c
clang/test/CodeGen/ppc64-complex-return.c

Removed: 




diff  --git a/clang/lib/Sema/DeclSpec.cpp b/clang/lib/Sema/DeclSpec.cpp
index 276e35a3497e..834e2533342d 100644
--- a/clang/lib/Sema/DeclSpec.cpp
+++ b/clang/lib/Sema/DeclSpec.cpp
@@ -1269,7 +1269,8 @@ void DeclSpec::Finish(Sema &S, const PrintingPolicy 
&Policy) {
   // Note that this intentionally doesn't include _Complex _Bool.
   if (!S.getLangOpts().CPlusPlus)
 S.Diag(TSTLoc, diag::ext_integer_complex);
-} else if (TypeSpecType != TST_float && TypeSpecType != TST_double) {
+} else if (TypeSpecType != TST_float && TypeSpecType != TST_double &&
+   TypeSpecType != TST_float128) {
   S.Diag(TSCLoc, diag::err_invalid_complex_spec)
 << getSpecifierName((TST)TypeSpecType, Policy);
   TypeSpecComplex = TSC_unspecified;

diff  --git a/clang/test/CodeGen/ppc64-complex-parms.c 
b/clang/test/CodeGen/ppc64-complex-parms.c
index c0e1794bf47c..1c8aa1d568cf 100644
--- a/clang/test/CodeGen/ppc64-complex-parms.c
+++ b/clang/test/CodeGen/ppc64-complex-parms.c
@@ -1,8 +1,19 @@
+// REQUIRES: powerpc-registered-target
 // RUN: %clang_cc1 -triple powerpc64-unknown-linux-gnu -emit-llvm -o - %s | 
FileCheck %s
+// RUN: %clang_cc1 -target-feature +float128 -DTEST_F128 -triple \
+// RUN:   powerpc64le-unknown-linux-gnu -emit-llvm -o - %s | FileCheck %s \
+// RUN:   --check-prefix CHECK-F128
 
 float crealf(_Complex float);
 double creal(_Complex double);
 long double creall(_Complex long double);
+#ifdef TEST_F128
+__float128 crealf128(_Complex __float128);
+__float128 foo_f128(_Complex __float128 x) {
+  return crealf128(x);
+}
+// CHECK-F128: define fp128 @foo_f128(fp128 {{[%A-Za-z0-9.]+}}, fp128 
{{[%A-Za-z0-9.]+}})
+#endif
 
 float foo_float(_Complex float x) {
   return crealf(x);

diff  --git a/clang/test/CodeGen/ppc64-complex-return.c 
b/clang/test/CodeGen/ppc64-complex-return.c
index 02bfe82d4efe..a27286d85b8f 100644
--- a/clang/test/CodeGen/ppc64-complex-return.c
+++ b/clang/test/CodeGen/ppc64-complex-return.c
@@ -1,9 +1,20 @@
 // REQUIRES: powerpc-registered-target
 // RUN: %clang_cc1 -triple powerpc64-unknown-linux-gnu -emit-llvm -o - %s | 
FileCheck %s
+// RUN: %clang_cc1 -target-feature +float128 -DTEST_F128 -triple \
+// RUN:   powerpc64le-unknown-linux-gnu -emit-llvm -o - %s | FileCheck %s \
+// RUN:   --check-prefix CHECK-F128
 
 float crealf(_Complex float);
 double creal(_Complex double);
 long double creall(_Complex long double);
+#ifdef TEST_F128
+__float128 crealf128(_Complex __float128);
+_Complex __float128 foo_f128(_Complex __float128 x) {
+  return x;
+}
+
+// CHECK-F128: define { fp128, fp128 } @foo_f128(fp128 {{[%A-Za-z0-9.]+}}, 
fp128 {{[%A-Za-z0-9.]+}}) [[NUW:#[0-9]+]] {
+#endif
 
 _Complex float foo_float(_Complex float x) {
   return x;
@@ -80,6 +91,17 @@ long double bar_long_double(void) {
 // CHECK: extractvalue { ppc_fp128, ppc_fp128 } [[VAR3]], 0
 // CHECK: extractvalue { ppc_fp128, ppc_fp128 } [[VAR3]], 1
 
+#ifdef TEST_F128
+__float128 bar_f128(void) {
+  return crealf128(foo_f128(2.0Q - 2.5Qi));
+}
+
+// CHECK-F128: define fp128 @bar_f128() [[NUW]] {
+// CHECK-F128: [[VAR3:[%A-Za-z0-9.]+]] = call { fp128, fp128 } @foo_f128
+// CHECK-F128: extractvalue { fp128, fp128 } [[VAR3]], 0
+// CHECK-F128: extractvalue { fp128, fp128 } [[VAR3]], 1
+#endif
+
 int bar_int(void) {
   return __real__(foo_int(2 - 3i));
 }



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

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

[Daniel Kiss via Phabricator via cfe-commits]

danielkiss added a comment.

Related ticket for gcc: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94891


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

https://reviews.llvm.org/D75044



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

[PATCH] D80374: [Clang] Enable KF and KC mode for [_Complex] __float128

[Nemanja Ivanovic via Phabricator via cfe-commits]

nemanjai updated this revision to Diff 266820.
nemanjai added a comment.

Handled invalid uses of `KI` as there is no corresponding integer mode and 
added testing for it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80374

Files:
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/Basic/TargetInfo.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/Basic/TargetInfo.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/Sema/attr-mode.c

Index: clang/test/Sema/attr-mode.c
===
--- clang/test/Sema/attr-mode.c
+++ clang/test/Sema/attr-mode.c
@@ -4,6 +4,8 @@
 // RUN:   -verify %s
 // RUN: %clang_cc1 -triple powerpc64-pc-linux-gnu -DTEST_64BIT_PPC64 -fsyntax-only \
 // RUN:   -verify %s
+// RUN: %clang_cc1 -triple powerpc64-pc-linux-gnu -DTEST_F128_PPC64 -fsyntax-only \
+// RUN:   -verify -target-feature +float128 %s
 // RUN: %clang_cc1 -triple x86_64-pc-linux-gnux32 -DTEST_64BIT_X86 -fsyntax-only \
 // RUN:   -verify %s
 // RUN: %clang_cc1 -triple mips-linux-gnu -DTEST_MIPS_32 -fsyntax-only \
@@ -90,6 +92,15 @@
 void f_ft128_complex_arg(_Complex long double *x);
 void test_TFtype(f128ibm *a) { f_ft128_arg (a); }
 void test_TCtype(c128ibm *a) { f_ft128_complex_arg (a); }
+#elif TEST_F128_PPC64
+typedef int invalid_7 __attribute((mode(KF))); // expected-error{{type of machine mode does not match type of base type}}
+typedef int invalid_8 __attribute((mode(KI))); // expected-error{{unknown machine mode}}
+typedef _Complex float cf128 __attribute__((mode(KC)));
+typedef float f128 __attribute__((mode(KF)));
+void f_f128_arg(__float128 *x);
+void f_f128_complex_arg(_Complex __float128 *x);
+void test_KFtype(f128 *a) { f_f128_arg(a); }
+void test_KCtype(cf128 *a) { f_f128_complex_arg(a); }
 #elif TEST_MIPS_32
 typedef unsigned int gcc_unwind_word __attribute__((mode(unwind_word)));
 int foo[sizeof(gcc_unwind_word) == 4 ? 1 : -1];
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -3942,7 +3942,8 @@
 /// parseModeAttrArg - Parses attribute mode string and returns parsed type
 /// attribute.
 static void parseModeAttrArg(Sema &S, StringRef Str, unsigned &DestWidth,
- bool &IntegerMode, bool &ComplexMode) {
+ bool &IntegerMode, bool &ComplexMode,
+ bool &ExplicitIEEE) {
   IntegerMode = true;
   ComplexMode = false;
   switch (Str.size()) {
@@ -3963,7 +3964,12 @@
 case 'X':
   DestWidth = 96;
   break;
+case 'K': // KFmode - IEEE quad precision (__float128)
+  ExplicitIEEE = true;
+  DestWidth = Str[1] == 'I' ? 0 : 128;
+  break;
 case 'T':
+  ExplicitIEEE = false;
   DestWidth = 128;
   break;
 }
@@ -4024,6 +4030,7 @@
   unsigned DestWidth = 0;
   bool IntegerMode = true;
   bool ComplexMode = false;
+  bool ExplicitIEEE = false;
   llvm::APInt VectorSize(64, 0);
   if (Str.size() >= 4 && Str[0] == 'V') {
 // Minimal length of vector mode is 4: 'V' + NUMBER(>=1) + TYPE(>=2).
@@ -4036,7 +4043,7 @@
 !Str.substr(1, VectorStringLength).getAsInteger(10, VectorSize) &&
 VectorSize.isPowerOf2()) {
   parseModeAttrArg(*this, Str.substr(VectorStringLength + 1), DestWidth,
-   IntegerMode, ComplexMode);
+   IntegerMode, ComplexMode, ExplicitIEEE);
   // Avoid duplicate warning from template instantiation.
   if (!InInstantiation)
 Diag(AttrLoc, diag::warn_vector_mode_deprecated);
@@ -4046,7 +4053,8 @@
   }
 
   if (!VectorSize)
-parseModeAttrArg(*this, Str, DestWidth, IntegerMode, ComplexMode);
+parseModeAttrArg(*this, Str, DestWidth, IntegerMode, ComplexMode,
+ ExplicitIEEE);
 
   // FIXME: Sync this with InitializePredefinedMacros; we need to match int8_t
   // and friends, at least with glibc.
@@ -4112,7 +4120,7 @@
 NewElemTy = Context.getIntTypeForBitwidth(DestWidth,
   OldElemTy->isSignedIntegerType());
   else
-NewElemTy = Context.getRealTypeForBitwidth(DestWidth);
+NewElemTy = Context.getRealTypeForBitwidth(DestWidth, ExplicitIEEE);
 
   if (NewElemTy.isNull()) {
 Diag(AttrLoc, diag::err_machine_mode) << 1 /*Unsupported*/ << Name;
Index: clang/lib/Basic/TargetInfo.cpp
===
--- clang/lib/Basic/TargetInfo.cpp
+++ clang/lib/Basic/TargetInfo.cpp
@@ -265,7 +265,8 @@
   return NoInt;
 }
 
-TargetInfo::RealType TargetInfo::getRealTypeByWidth(unsigned BitWidth) const {
+TargetInfo::RealType TargetInfo::getRealTypeByWidth(unsigned BitWidth,
+bool ExplicitIEEE) const {
   if (getFloatWidth() == BitWidth)
 return Float;
   if (getDoubleWidth() =

[PATCH] D80531: [clang-tidy]: Added modernize-replace-disallow-copy-and-assign-macro

[Konrad Wilhelm Kleine via Phabricator via cfe-commits]

kwk added a comment.

@njames93 and @Eugene.Zelenko do you guys think this patch is ready to be 
approved and land?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80531



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

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

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clang/lib/Frontend/PrecompiledPreamble.cpp:101
+
+  // We always receive FileNotFound followed by InclusionDirective.
+  // We want the former, so we're completely sure the file was missing.

kadircet wrote:
> is there a common recovery logic implemented in clang that makes this logic 
> worth it?
> because none of the existing clients seems to be implementing it.
> 
> I would rather check for `File == nullptr` in `InclusionDirective` and maybe 
> put a fixme saying that this might still be set by a "recovering ppcallback".
> 
> Up to you though.
Done. I think a fixme is disingenuous though if we deliberately un-fixed it :-)
Left a non-committal comment.



Comment at: clang/lib/Frontend/PrecompiledPreamble.cpp:590
+  for (const auto &F : MissingFiles) {
+// A missing file may be "provided" by an override buffer.
+if (OverridenFileBuffers.count(F.getKey()))

kadircet wrote:
> i believe it can also be provided by an overridefile. maybe also check this 
> above while going over `RemappedFiles`. i.e:
> 
> ```
> for(... &R : ..RemappedFiles) {
>if(!stat(R.second)) return false;
>// A missing file has been remapped into an existing file.
>if(MissingFiles.count(R.first)) return false;
>  
> }
> ```
Decided to just add a new set containing abs paths for everything that got 
overridden.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77942



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

[PATCH] D80374: [Clang] Enable KF and KC mode for [_Complex] __float128

[Nemanja Ivanovic via Phabricator via cfe-commits]

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



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:3970
+  DestWidth = 128;
+  break;
 case 'T':

rjmccall wrote:
> rjmccall wrote:
> > Are there interactions with the other mode specifiers?  For example, should 
> > this be allowed with integer modes?  If so, I think this needs more tests.
> I shouldn't have said "if so" — *either way*, this needs more tests.
Very good point. Thank you. Added.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80374



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

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

[Sam McCall via Phabricator via cfe-commits]

sammccall updated this revision to Diff 266822.
sammccall marked 6 inline comments as done.
sammccall added a comment.

Finally address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77942

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

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

[PATCH] D80533: [Clang] Enable _Complex __float

[Nemanja Ivanovic via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGf9e94eb8688d: [Clang] Enable _Complex __float128 (authored 
by nemanjai).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80533

Files:
  clang/lib/Sema/DeclSpec.cpp
  clang/test/CodeGen/ppc64-complex-parms.c
  clang/test/CodeGen/ppc64-complex-return.c


Index: clang/test/CodeGen/ppc64-complex-return.c
===
--- clang/test/CodeGen/ppc64-complex-return.c
+++ clang/test/CodeGen/ppc64-complex-return.c
@@ -1,9 +1,20 @@
 // REQUIRES: powerpc-registered-target
 // RUN: %clang_cc1 -triple powerpc64-unknown-linux-gnu -emit-llvm -o - %s | 
FileCheck %s
+// RUN: %clang_cc1 -target-feature +float128 -DTEST_F128 -triple \
+// RUN:   powerpc64le-unknown-linux-gnu -emit-llvm -o - %s | FileCheck %s \
+// RUN:   --check-prefix CHECK-F128
 
 float crealf(_Complex float);
 double creal(_Complex double);
 long double creall(_Complex long double);
+#ifdef TEST_F128
+__float128 crealf128(_Complex __float128);
+_Complex __float128 foo_f128(_Complex __float128 x) {
+  return x;
+}
+
+// CHECK-F128: define { fp128, fp128 } @foo_f128(fp128 {{[%A-Za-z0-9.]+}}, 
fp128 {{[%A-Za-z0-9.]+}}) [[NUW:#[0-9]+]] {
+#endif
 
 _Complex float foo_float(_Complex float x) {
   return x;
@@ -80,6 +91,17 @@
 // CHECK: extractvalue { ppc_fp128, ppc_fp128 } [[VAR3]], 0
 // CHECK: extractvalue { ppc_fp128, ppc_fp128 } [[VAR3]], 1
 
+#ifdef TEST_F128
+__float128 bar_f128(void) {
+  return crealf128(foo_f128(2.0Q - 2.5Qi));
+}
+
+// CHECK-F128: define fp128 @bar_f128() [[NUW]] {
+// CHECK-F128: [[VAR3:[%A-Za-z0-9.]+]] = call { fp128, fp128 } @foo_f128
+// CHECK-F128: extractvalue { fp128, fp128 } [[VAR3]], 0
+// CHECK-F128: extractvalue { fp128, fp128 } [[VAR3]], 1
+#endif
+
 int bar_int(void) {
   return __real__(foo_int(2 - 3i));
 }
Index: clang/test/CodeGen/ppc64-complex-parms.c
===
--- clang/test/CodeGen/ppc64-complex-parms.c
+++ clang/test/CodeGen/ppc64-complex-parms.c
@@ -1,8 +1,19 @@
+// REQUIRES: powerpc-registered-target
 // RUN: %clang_cc1 -triple powerpc64-unknown-linux-gnu -emit-llvm -o - %s | 
FileCheck %s
+// RUN: %clang_cc1 -target-feature +float128 -DTEST_F128 -triple \
+// RUN:   powerpc64le-unknown-linux-gnu -emit-llvm -o - %s | FileCheck %s \
+// RUN:   --check-prefix CHECK-F128
 
 float crealf(_Complex float);
 double creal(_Complex double);
 long double creall(_Complex long double);
+#ifdef TEST_F128
+__float128 crealf128(_Complex __float128);
+__float128 foo_f128(_Complex __float128 x) {
+  return crealf128(x);
+}
+// CHECK-F128: define fp128 @foo_f128(fp128 {{[%A-Za-z0-9.]+}}, fp128 
{{[%A-Za-z0-9.]+}})
+#endif
 
 float foo_float(_Complex float x) {
   return crealf(x);
Index: clang/lib/Sema/DeclSpec.cpp
===
--- clang/lib/Sema/DeclSpec.cpp
+++ clang/lib/Sema/DeclSpec.cpp
@@ -1269,7 +1269,8 @@
   // Note that this intentionally doesn't include _Complex _Bool.
   if (!S.getLangOpts().CPlusPlus)
 S.Diag(TSTLoc, diag::ext_integer_complex);
-} else if (TypeSpecType != TST_float && TypeSpecType != TST_double) {
+} else if (TypeSpecType != TST_float && TypeSpecType != TST_double &&
+   TypeSpecType != TST_float128) {
   S.Diag(TSCLoc, diag::err_invalid_complex_spec)
 << getSpecifierName((TST)TypeSpecType, Policy);
   TypeSpecComplex = TSC_unspecified;


Index: clang/test/CodeGen/ppc64-complex-return.c
===
--- clang/test/CodeGen/ppc64-complex-return.c
+++ clang/test/CodeGen/ppc64-complex-return.c
@@ -1,9 +1,20 @@
 // REQUIRES: powerpc-registered-target
 // RUN: %clang_cc1 -triple powerpc64-unknown-linux-gnu -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -target-feature +float128 -DTEST_F128 -triple \
+// RUN:   powerpc64le-unknown-linux-gnu -emit-llvm -o - %s | FileCheck %s \
+// RUN:   --check-prefix CHECK-F128
 
 float crealf(_Complex float);
 double creal(_Complex double);
 long double creall(_Complex long double);
+#ifdef TEST_F128
+__float128 crealf128(_Complex __float128);
+_Complex __float128 foo_f128(_Complex __float128 x) {
+  return x;
+}
+
+// CHECK-F128: define { fp128, fp128 } @foo_f128(fp128 {{[%A-Za-z0-9.]+}}, fp128 {{[%A-Za-z0-9.]+}}) [[NUW:#[0-9]+]] {
+#endif
 
 _Complex float foo_float(_Complex float x) {
   return x;
@@ -80,6 +91,17 @@
 // CHECK: extractvalue { ppc_fp128, ppc_fp128 } [[VAR3]], 0
 // CHECK: extractvalue { ppc_fp128, ppc_fp128 } [[VAR3]], 1
 
+#ifdef TEST_F128
+__float128 bar_f128(void) {
+  return crealf128(foo_f128(2.0Q - 2.5Qi));
+}
+
+// CHECK-F128: define fp128 @bar_f128() [[NUW]] {
+// CHECK-F128: [[VAR3:[%A-Za-z0-9.]+]] = call { fp128, fp128 } @foo_f128
+// CHECK-F128: extractvalue { fp128, fp128 } [[VAR3]], 0
+// CHE

[PATCH] D80492: Avoid linking libdl unless needed

[Tobias Hieta via Phabricator via cfe-commits]

thieta added a comment.

@mstorsjo mind landing this for me as well?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80492



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

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

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

In this version of the patch you are using `handleConstructionContext` to 
"retrieve" or read an SVal for the construction. However, it seems like that 
function was designed to store values in the GDM for individual construction 
context items. To mix a read-only functionality into a setter seems to be 
error-prone to me.

> Please instead re-use the code that computes the object under construction. 
> That'll save you ~50 lines of code and will be more future-proof

I am not sure that this is the best option here. In my viewpoint, 
ConstructionContext and ConstructionContextItem are both "variants" (i.e. 
tagged unions) and we perform operations on these. Each operation is actually a 
visitation on the different kind of values (i.e. a switch). One operation is 
transforming a ConstructionContext to a ConstructionContextItem. This is what 
is needed to be able to call the getObjectUnderConstruction function. And this 
operation - the reading - is way different than the other - the store -, see 
Adam's concerns. This yields to different functions for each op. But this is 
perfectly natural once we work with "variants". I mean we can easily extend the 
operations on a variant with a new operation as a form of a visitation of the 
kinds (contrary to traditional class hierarchies where adding a new op is hard, 
but adding a new type is easy).


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

https://reviews.llvm.org/D80366



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

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

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

In D80366#2059765 , @martong wrote:

> In this version of the patch you are using `handleConstructionContext` to 
> "retrieve" or read an SVal for the construction. However, it seems like that 
> function was designed to store values in the GDM for individual construction 
> context items. To mix a read-only functionality into a setter seems to be 
> error-prone to me.
>
> > Please instead re-use the code that computes the object under construction. 
> > That'll save you ~50 lines of code and will be more future-proof
>
> I am not sure that this is the best option here. In my viewpoint, 
> ConstructionContext and ConstructionContextItem are both "variants" (i.e. 
> tagged unions) and we perform operations on these. Each operation is actually 
> a visitation on the different kind of values (i.e. a switch). One operation 
> is transforming a ConstructionContext to a ConstructionContextItem. This is 
> what is needed to be able to call the getObjectUnderConstruction function. 
> And this operation - the reading - is way different than the other - the 
> store -, see Adam's concerns. This yields to different functions for each op. 
> But this is perfectly natural once we work with "variants". I mean we can 
> easily extend the operations on a variant with a new operation as a form of a 
> visitation of the kinds (contrary to traditional class hierarchies where 
> adding a new op is hard, but adding a new type is easy).


Just one more thing. It may turn out that the visitation should be done the 
same way in both cases, then we have to have a Visitor factored out. But, now 
it seems that even the visitation strategy is different for the individual 
operations.


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

https://reviews.llvm.org/D80366



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

[PATCH] D80685: [ASTMatchers] Add traversal-kind support to `DynTypedMatcher`

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel updated this revision to Diff 266828.
ymandel marked 2 inline comments as done.
ymandel added a comment.
Herald added a subscriber: mgorny.

updated per comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80685

Files:
  clang/include/clang/ASTMatchers/ASTMatchersInternal.h
  clang/lib/ASTMatchers/ASTMatchersInternal.cpp
  clang/unittests/ASTMatchers/ASTMatchersInternalTest.cpp
  clang/unittests/ASTMatchers/CMakeLists.txt

Index: clang/unittests/ASTMatchers/CMakeLists.txt
===
--- clang/unittests/ASTMatchers/CMakeLists.txt
+++ clang/unittests/ASTMatchers/CMakeLists.txt
@@ -30,4 +30,9 @@
   clangTooling
   )
 
+target_link_libraries(ASTMatchersTests
+  PRIVATE
+  LLVMTestingSupport
+)
+
 add_subdirectory(Dynamic)
Index: clang/unittests/ASTMatchers/ASTMatchersInternalTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersInternalTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersInternalTest.cpp
@@ -13,10 +13,12 @@
 #include "clang/Tooling/Tooling.h"
 #include "llvm/ADT/Triple.h"
 #include "llvm/Support/Host.h"
+#include "llvm/Testing/Support/SupportHelpers.h"
 #include "gtest/gtest.h"
 
 namespace clang {
 namespace ast_matchers {
+using internal::DynTypedMatcher;
 
 #if GTEST_HAS_DEATH_TEST
 TEST(HasNameDeathTest, DiesOnEmptyName) {
@@ -171,6 +173,26 @@
   EXPECT_NE(nullptr, PT);
 }
 
+TEST(DynTypedMatcherTest, TraversalKindForwardsToImpl) {
+  auto M = DynTypedMatcher(decl());
+  EXPECT_FALSE(M.getTraversalKind().hasValue());
+
+  M = DynTypedMatcher(traverse(TK_AsIs, decl()));
+  EXPECT_THAT(M.getTraversalKind(), llvm::ValueIs(TK_AsIs));
+}
+
+TEST(DynTypedMatcherTest, ConstructWithTraversalKindSetsTK) {
+  auto M = DynTypedMatcher(decl()).withTraversalKind(TK_AsIs);
+  EXPECT_THAT(M.getTraversalKind(), llvm::ValueIs(TK_AsIs));
+}
+
+TEST(DynTypedMatcherTest, ConstructWithTraversalKindOverridesNestedTK) {
+  auto M = DynTypedMatcher(decl()).withTraversalKind(TK_AsIs).withTraversalKind(
+  TK_IgnoreUnlessSpelledInSource);
+  EXPECT_THAT(M.getTraversalKind(),
+  llvm::ValueIs(TK_IgnoreUnlessSpelledInSource));
+}
+
 TEST(IsInlineMatcher, IsInline) {
   EXPECT_TRUE(matches("void g(); inline void f();",
   functionDecl(isInline(), hasName("f";
Index: clang/lib/ASTMatchers/ASTMatchersInternal.cpp
===
--- clang/lib/ASTMatchers/ASTMatchersInternal.cpp
+++ clang/lib/ASTMatchers/ASTMatchersInternal.cpp
@@ -136,6 +136,31 @@
   }
 };
 
+/// A matcher that specifies a particular \c TraversalKind.
+///
+/// The kind provided to the constructor overrides any kind that may be
+/// specified by the `InnerMatcher`.
+class DynTraversalMatcherImpl : public DynMatcherInterface {
+public:
+  explicit DynTraversalMatcherImpl(
+  clang::TraversalKind TK,
+  IntrusiveRefCntPtr InnerMatcher)
+  : TK(TK), InnerMatcher(std::move(InnerMatcher)) {}
+
+  bool dynMatches(const DynTypedNode &DynNode, ASTMatchFinder *Finder,
+  BoundNodesTreeBuilder *Builder) const override {
+return this->InnerMatcher->dynMatches(DynNode, Finder, Builder);
+  }
+
+  llvm::Optional TraversalKind() const override {
+return TK;
+  }
+
+private:
+  clang::TraversalKind TK;
+  IntrusiveRefCntPtr InnerMatcher;
+};
+
 } // namespace
 
 static llvm::ManagedStatic TrueMatcherInstance;
@@ -204,6 +229,14 @@
   return Copy;
 }
 
+DynTypedMatcher
+DynTypedMatcher::withTraversalKind(ast_type_traits::TraversalKind TK) {
+  auto Copy = *this;
+  Copy.Implementation =
+  new DynTraversalMatcherImpl(TK, std::move(Copy.Implementation));
+  return Copy;
+}
+
 DynTypedMatcher DynTypedMatcher::trueMatcher(ASTNodeKind NodeKind) {
   return DynTypedMatcher(NodeKind, NodeKind, &*TrueMatcherInstance);
 }
Index: clang/include/clang/ASTMatchers/ASTMatchersInternal.h
===
--- clang/include/clang/ASTMatchers/ASTMatchersInternal.h
+++ clang/include/clang/ASTMatchers/ASTMatchersInternal.h
@@ -395,6 +395,12 @@
   ///   restricts the node types for \p Kind.
   DynTypedMatcher dynCastTo(const ASTNodeKind Kind) const;
 
+  /// Return a matcher that that points to the same implementation, but sets the
+  ///   traversal kind.
+  ///
+  /// If the traversal kind is already set, then \c TK overrides it.
+  DynTypedMatcher withTraversalKind(TraversalKind TK);
+
   /// Returns true if the matcher matches the given \c DynNode.
   bool matches(const DynTypedNode &DynNode, ASTMatchFinder *Finder,
BoundNodesTreeBuilder *Builder) const;
@@ -458,6 +464,14 @@
   /// If it is not compatible, then this matcher will never match anything.
   template  Matcher unconditionalConvertTo() const;
 
+  /// Returns the \c TraversalKind respected by calls to `match()`, if

[PATCH] D80197: [DebugInfo] Upgrade DISubrange to support Fortran dynamic arrays

[Marco Elver via Phabricator via cfe-commits]

melver added inline comments.



Comment at: llvm/lib/Bitcode/Writer/BitcodeWriter.cpp:1522
 
 static uint64_t rotateSign(int64_t I) {
   uint64_t U = I;

rotateSign() is no longer used in this file. If there are no plans to use it 
again, please remove it.
Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80197



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

[PATCH] D80197: [DebugInfo] Upgrade DISubrange to support Fortran dynamic arrays

[Alok Kumar Sharma via Phabricator via cfe-commits]

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

In D80197#2059591 , @thakis wrote:

> This breaks check-llvm on Windows: http://45.33.8.238/win/16214/step_11.txt
>
> Please take a look and revert if it takes a while to investigate.


Fixed in 7716681cfd0ea2dadbddae6f1983e130c2fa4247 
 .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80197



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

[PATCH] D80685: [ASTMatchers] Add traversal-kind support to `DynTypedMatcher`

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

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



Comment at: clang/lib/ASTMatchers/ASTMatchersInternal.cpp:234
+DynTypedMatcher::constructWithTraversalKind(DynTypedMatcher InnerMatcher,
+ast_type_traits::TraversalKind TK) 
{
+  InnerMatcher.Implementation =

gribozavr2 wrote:
> It might read better as an instance method on `DynTypedMatcher`: 
> `DynTypedMatcher::withTraversalKind()`. It is not unprecedented, see 
> `dynCastTo()`.
Agreed, thanks for the suggestion.



Comment at: clang/unittests/ASTMatchers/ASTMatchersInternalTest.cpp:182
+  EXPECT_TRUE(TK.hasValue());
+  EXPECT_EQ(*TK, TK_AsIs);
+}

gribozavr2 wrote:
> Please use `EXPECT_THAT(M.getTraversalKind(), llvm::ValueIs(TK_AsIs));` (also 
> in tests below).
> 
> You'll need to include `llvm/Testing/Support/SupportHelpers.h`.
Thanks! This cleaned up the tests quite nicely.

I also updated the CMake file. Please let me know if that looks right to you. 
Cargo-culted from other cmake files...



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80685



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

[PATCH] D80117: [analyzer] Introduce reasoning about symbolic remainder operator

[Valeriy Savchenko via Phabricator via cfe-commits]

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



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:506-507
+  ///
+  /// where abs(Origin) is the maximal absolute value of any possible values
+  ///   from Origin, and min(T) is a minimal value for the type T.
+  ///

NoQ wrote:
> I suggest not trying to express signed types and unsigned types in a single 
> formula, the reader will have to unwrap it back into the two cases anyway in 
> order to understand what's going on.
> 
> The following would imho be easier to read: "If T is signed, return the 
> smallest range `[-x..x]` that covers the original range, or `[-min(T), 
> max(T)]` if the aforementioned symmetric range doesn't exist due to original 
> range covering `min(T)`). If T is unsigned, return the smallest range 
> `[0..x]` that covers the original range".
That is a perfect explanation, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80117



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

[PATCH] D80627: [clang-format] Create a python documentation tool to generate a summary of the clang-format status for the whole of the LLVM project

[Sylvestre Ledru via Phabricator via cfe-commits]

sylvestre.ledru added a comment.

Well done!

For Firefox, we decided to go ahead and reformat everything at once.
I has been way easier this way.

You should run flake8 & autopep8 on your python code. it isn't idomatic Python 
code (";" are useless on python for example)




Comment at: clang/docs/tools/generate_formatted_state.py:64
+stdout, err = cmd.communicate();
+if (err.find(': warning:') > 0):
+clean = False;

() are useless in python for if


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80627



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

cfe-commits@lists.llvm.org

[Luke Geeson via Phabricator via cfe-commits]

LukeGeeson created this revision.
LukeGeeson added reviewers: fpetrogalli, SjoerdMeijer.
Herald added subscribers: llvm-commits, cfe-commits, danielkiss, hiraditya, 
kristof.beyls.
Herald added projects: clang, LLVM.
LukeGeeson added a parent revision: D79869: [clang][BFloat] Add reinterpret 
cast intrinsics.

This patch upstreams support for ld / st variants of BFloat intrinsics
in from __bf16 to AArch64. This includes IR intrinsics. Unittests are
provided as needed.

This patch is part of a series implementing the Bfloat16 extension of
the
Armv8.6-a architecture, as detailed here:

https://community.arm.com/developer/ip-products/processors/b/processors-ip-blog/posts/arm-architecture-developments-armv8-6-a

The bfloat type, and its properties are specified in the Arm
Architecture
Reference Manual:

https://developer.arm.com/docs/ddi0487/latest/arm-architecture-reference-manual-armv8-for-armv8-a-architecture-profile

The following people contributed to this patch:

- Luke Geeson
- Momchil Velikov
- Luke Cheeseman


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D80716

Files:
  clang/include/clang/Basic/arm_neon.td
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/aarch64-bf16-ldst-intrinsics.c
  clang/test/Sema/aarch64-bf16-ldst-intrinsics.c
  llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp
  llvm/lib/Target/AArch64/AArch64InstrInfo.td

Index: llvm/lib/Target/AArch64/AArch64InstrInfo.td
===
--- llvm/lib/Target/AArch64/AArch64InstrInfo.td
+++ llvm/lib/Target/AArch64/AArch64InstrInfo.td
@@ -2233,6 +2233,7 @@
   defm : VecROLoadPat;
   defm : VecROLoadPat;
   defm : VecROLoadPat;
+  defm : VecROLoadPat;
 }
 
 defm : VecROLoadPat;
@@ -2247,6 +2248,7 @@
   defm : VecROLoadPat;
   defm : VecROLoadPat;
   defm : VecROLoadPat;
+  defm : VecROLoadPat;
   defm : VecROLoadPat;
 }
 } // AddedComplexity = 10
@@ -2382,6 +2384,8 @@
 (LDRDui GPR64sp:$Rn, uimm12s8:$offset)>;
   def : Pat<(v4f16 (load (am_indexed64 GPR64sp:$Rn, uimm12s8:$offset))),
 (LDRDui GPR64sp:$Rn, uimm12s8:$offset)>;
+  def : Pat<(v4bf16 (load (am_indexed64 GPR64sp:$Rn, uimm12s8:$offset))),
+(LDRDui GPR64sp:$Rn, uimm12s8:$offset)>;
 }
 def : Pat<(v1f64 (load (am_indexed64 GPR64sp:$Rn, uimm12s8:$offset))),
   (LDRDui GPR64sp:$Rn, uimm12s8:$offset)>;
@@ -2405,6 +2409,8 @@
 (LDRQui GPR64sp:$Rn, uimm12s16:$offset)>;
   def : Pat<(v8f16 (load (am_indexed128 GPR64sp:$Rn, uimm12s16:$offset))),
 (LDRQui GPR64sp:$Rn, uimm12s16:$offset)>;
+  def : Pat<(v8bf16 (load (am_indexed128 GPR64sp:$Rn, uimm12s16:$offset))),
+(LDRQui GPR64sp:$Rn, uimm12s16:$offset)>;
 }
 def : Pat<(f128  (load (am_indexed128 GPR64sp:$Rn, uimm12s16:$offset))),
   (LDRQui GPR64sp:$Rn, uimm12s16:$offset)>;
@@ -2903,6 +2909,7 @@
   defm : VecROStorePat;
   defm : VecROStorePat;
   defm : VecROStorePat;
+  defm : VecROStorePat;
 }
 
 defm : VecROStorePat;
@@ -2918,6 +2925,7 @@
   defm : VecROStorePat;
   defm : VecROStorePat;
   defm : VecROStorePat;
+  defm : VecROStorePat;
 }
 } // AddedComplexity = 10
 
@@ -3010,6 +3018,9 @@
   def : Pat<(store (v4f16 FPR64:$Rt),
(am_indexed64 GPR64sp:$Rn, uimm12s8:$offset)),
 (STRDui FPR64:$Rt, GPR64sp:$Rn, uimm12s8:$offset)>;
+  def : Pat<(store (v4bf16 FPR64:$Rt),
+   (am_indexed64 GPR64sp:$Rn, uimm12s8:$offset)),
+(STRDui FPR64:$Rt, GPR64sp:$Rn, uimm12s8:$offset)>;
 }
 
 // Match all store 128 bits width whose type is compatible with FPR128
@@ -3040,6 +3051,9 @@
   def : Pat<(store (v8f16 FPR128:$Rt),
(am_indexed128 GPR64sp:$Rn, uimm12s16:$offset)),
 (STRQui FPR128:$Rt, GPR64sp:$Rn, uimm12s16:$offset)>;
+  def : Pat<(store (v8bf16 FPR128:$Rt),
+   (am_indexed128 GPR64sp:$Rn, uimm12s16:$offset)),
+(STRQui FPR128:$Rt, GPR64sp:$Rn, uimm12s16:$offset)>;
 }
 
 // truncstore i64
@@ -3147,6 +3161,9 @@
   def : Pat<(store (v4f16 FPR64:$Rt),
(am_unscaled64 GPR64sp:$Rn, simm9:$offset)),
 (STURDi FPR64:$Rt, GPR64sp:$Rn, simm9:$offset)>;
+  def : Pat<(store (v4bf16 FPR64:$Rt),
+   (am_unscaled64 GPR64sp:$Rn, simm9:$offset)),
+(STURDi FPR64:$Rt, GPR64sp:$Rn, simm9:$offset)>;
 }
 
 // Match all store 128 bits width whose type is compatible with FPR128
@@ -3179,6 +3196,9 @@
   def : Pat<(store (v8f16 FPR128:$Rt),
(am_unscaled128 GPR64sp:$Rn, simm9:$offset)),
 (STURQi FPR128:$Rt, GPR64sp:$Rn, simm9:$offset)>;
+  def : Pat<(store (v8bf16 FPR128:$Rt),
+   (am_unscaled128 GPR64sp:$Rn, simm9:$offset)),
+(STURQi FPR128:$Rt, GPR64sp:$Rn, simm9:$offset)>;
 }
 
 } // AddedComplexity = 10
@@ -6316,6 +6336,10 @@
   (LD1Rv4h GPR64sp:$Rn)>;
 def : Pat<(v8f16 (AArch64dup (f16 (load GPR64sp:$Rn,
   (LD1Rv8h GPR64sp:$Rn)>;
+def : Pat<(v4bf16 (AArch64dup (bf

[PATCH] D80016: [analyzer] StdLibraryFunctionsChecker: Add support to lookup types

[Gabor Marton via Phabricator via cfe-commits]

martong updated this revision to Diff 266834.
martong marked an inline comment as done.
martong added a comment.

- Rebase to master


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80016

Files:
  clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
  clang/test/Analysis/std-c-library-functions-arg-constraints.c
  clang/test/Analysis/std-c-library-functions-lookup.c
  clang/test/Analysis/std-c-library-functions-lookup.cpp
  clang/test/Analysis/std-c-library-functions.c

Index: clang/test/Analysis/std-c-library-functions.c
===
--- clang/test/Analysis/std-c-library-functions.c
+++ clang/test/Analysis/std-c-library-functions.c
@@ -53,10 +53,10 @@
 // CHECK-NEXT: Loaded summary for: int getc(FILE *)
 // CHECK-NEXT: Loaded summary for: int fgetc(FILE *)
 // CHECK-NEXT: Loaded summary for: int getchar()
+// CHECK-NEXT: Loaded summary for: unsigned int fread(void *restrict, size_t, size_t, FILE *restrict)
+// CHECK-NEXT: Loaded summary for: unsigned int fwrite(const void *restrict, size_t, size_t, FILE *restrict)
 // CHECK-NEXT: Loaded summary for: ssize_t read(int, void *, size_t)
 // CHECK-NEXT: Loaded summary for: ssize_t write(int, const void *, size_t)
-// CHECK-NEXT: Loaded summary for: unsigned int fread(void *restrict, size_t, size_t, FILE *)
-// CHECK-NEXT: Loaded summary for: unsigned int fwrite(const void *restrict, size_t, size_t, FILE *restrict)
 // CHECK-NEXT: Loaded summary for: ssize_t getline(char **, size_t *, FILE *)
 
 void clang_analyzer_eval(int);
@@ -104,7 +104,7 @@
   }
 }
 
-size_t fread(void *restrict, size_t, size_t, FILE *);
+size_t fread(void *restrict, size_t, size_t, FILE *restrict);
 size_t fwrite(const void *restrict, size_t, size_t, FILE *restrict);
 void test_fread_fwrite(FILE *fp, int *buf) {
 
Index: clang/test/Analysis/std-c-library-functions-lookup.cpp
===
--- /dev/null
+++ clang/test/Analysis/std-c-library-functions-lookup.cpp
@@ -0,0 +1,23 @@
+// RUN: %clang_analyze_cc1 %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=apiModeling.StdCLibraryFunctions \
+// RUN:   -analyzer-config apiModeling.StdCLibraryFunctions:DisplayLoadedSummaries=true \
+// RUN:   -analyzer-checker=debug.ExprInspection \
+// RUN:   -analyzer-config eagerly-assume=false \
+// RUN:   -triple i686-unknown-linux 2>&1 | FileCheck %s
+
+//  CHECK: Loaded summary for: size_t fread(void *, size_t, size_t, FILE *)
+//  CHECK-NOT: Loaded summary for: size_t fread(void *, size_t, size_t, MyFile *)
+
+typedef unsigned int size_t;
+typedef struct FILE FILE;
+size_t fread(void *, size_t, size_t, FILE *);
+
+struct MyFile;
+size_t fread(void *, size_t, size_t, MyFile *);
+
+// Must have at least one call expression to initialize the summary map.
+int bar(void);
+void foo() {
+  bar();
+}
Index: clang/test/Analysis/std-c-library-functions-lookup.c
===
--- /dev/null
+++ clang/test/Analysis/std-c-library-functions-lookup.c
@@ -0,0 +1,19 @@
+// RUN: %clang_analyze_cc1 %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=apiModeling.StdCLibraryFunctions \
+// RUN:   -analyzer-config apiModeling.StdCLibraryFunctions:DisplayLoadedSummaries=true \
+// RUN:   -analyzer-checker=debug.ExprInspection \
+// RUN:   -analyzer-config eagerly-assume=false \
+// RUN:   -triple i686-unknown-linux 2>&1 | FileCheck %s
+
+// CHECK: Loaded summary for: unsigned int fread(void *restrict, size_t, size_t, FILE *restrict)
+
+typedef typeof(sizeof(int)) size_t;
+typedef struct FILE FILE;
+size_t fread(void *restrict, size_t, size_t, FILE *restrict);
+
+// Must have at least one call expression to initialize the summary map.
+int bar(void);
+void foo() {
+  bar();
+}
Index: clang/test/Analysis/std-c-library-functions-arg-constraints.c
===
--- clang/test/Analysis/std-c-library-functions-arg-constraints.c
+++ clang/test/Analysis/std-c-library-functions-arg-constraints.c
@@ -64,7 +64,7 @@
 
 typedef struct FILE FILE;
 typedef typeof(sizeof(int)) size_t;
-size_t fread(void *restrict, size_t, size_t, FILE *);
+size_t fread(void *restrict, size_t, size_t, FILE *restrict);
 void test_notnull_concrete(FILE *fp) {
   fread(0, sizeof(int), 10, fp); // \
   // report-warning{{Function argument constraint is not satisfied}} \
Index: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -568,6 +568,26 @@
   return findFunctionSummary(FD, C);
 }
 
+llvm::Optional lookupType(StringRef Name, const ASTContext &ACtx) {
+  IdentifierInfo &II = ACtx.Idents.get(

[PATCH] D80606: [libTooling] Fix Transformer to work with ambient traversal kinds

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel updated this revision to Diff 266831.
ymandel added a comment.

updated per comments and after rebasing


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80606

Files:
  clang/include/clang/Tooling/Transformer/RewriteRule.h
  clang/lib/Tooling/Transformer/RewriteRule.cpp
  clang/unittests/Tooling/TransformerTest.cpp

Index: clang/unittests/Tooling/TransformerTest.cpp
===
--- clang/unittests/Tooling/TransformerTest.cpp
+++ clang/unittests/Tooling/TransformerTest.cpp
@@ -571,6 +571,59 @@
   testRule(Rule, Input, Expected);
 }
 
+// Verifies that a rule with a matcher for an implicit node (like
+// `implicitCastExpr`) does not change the code, when the traversal kind is not
+// explicitly set ti TK_AsIs). In this test, only the rule with the
+// explicit-node matcher will fire.
+TEST_F(TransformerTest, OrderedRuleImplicitIgnored) {
+  std::string Input = R"cc(
+void f1();
+int f2();
+void call_f1() { f1(); }
+float call_f2() { return f2(); }
+  )cc";
+  std::string Expected = R"cc(
+void f1();
+int f2();
+void call_f1() { REPLACE_F1; }
+float call_f2() { return f2(); }
+  )cc";
+
+  RewriteRule ReplaceF1 =
+  makeRule(callExpr(callee(functionDecl(hasName("f1",
+   changeTo(cat("REPLACE_F1")));
+  RewriteRule ReplaceF2 =
+  makeRule(implicitCastExpr(hasSourceExpression(callExpr())),
+   changeTo(cat("REPLACE_F2")));
+  testRule(applyFirst({ReplaceF1, ReplaceF2}), Input, Expected);
+}
+
+// Verifies that explicitly setting the traversal kind fixes the problem in the
+// previous test.
+TEST_F(TransformerTest, OrderedRuleImplicitMatched) {
+  std::string Input = R"cc(
+void f1();
+int f2();
+void call_f1() { f1(); }
+float call_f2() { return f2(); }
+  )cc";
+  std::string Expected = R"cc(
+void f1();
+int f2();
+void call_f1() { REPLACE_F1; }
+float call_f2() { return REPLACE_F2; }
+  )cc";
+
+  RewriteRule ReplaceF1 = makeRule(
+  traverse(clang::TK_AsIs, callExpr(callee(functionDecl(hasName("f1"),
+  changeTo(cat("REPLACE_F1")));
+  RewriteRule ReplaceF2 =
+  makeRule(traverse(clang::TK_AsIs,
+implicitCastExpr(hasSourceExpression(callExpr(,
+   changeTo(cat("REPLACE_F2")));
+  testRule(applyFirst({ReplaceF1, ReplaceF2}), Input, Expected);
+}
+
 //
 // Negative tests (where we expect no transformation to occur).
 //
Index: clang/lib/Tooling/Transformer/RewriteRule.cpp
===
--- clang/lib/Tooling/Transformer/RewriteRule.cpp
+++ clang/lib/Tooling/Transformer/RewriteRule.cpp
@@ -116,10 +116,13 @@
 #endif
 
 // Binds each rule's matcher to a unique (and deterministic) tag based on
-// `TagBase` and the id paired with the case.
+// `TagBase` and the id paired with the case. All of the returned matchers have
+// their traversal kind explicitly set, either based on a pre-set kind or to the
+// provided `DefaultTraversalKind`.
 static std::vector taggedMatchers(
 StringRef TagBase,
-const SmallVectorImpl> &Cases) {
+const SmallVectorImpl> &Cases,
+ast_type_traits::TraversalKind DefaultTraversalKind) {
   std::vector Matchers;
   Matchers.reserve(Cases.size());
   for (const auto &Case : Cases) {
@@ -127,8 +130,10 @@
 // HACK: Many matchers are not bindable, so ensure that tryBind will work.
 DynTypedMatcher BoundMatcher(Case.second.Matcher);
 BoundMatcher.setAllowBind(true);
-auto M = BoundMatcher.tryBind(Tag);
-Matchers.push_back(*std::move(M));
+auto M = *BoundMatcher.tryBind(Tag);
+Matchers.push_back(!M.getTraversalKind()
+   ? M.withTraversalKind(DefaultTraversalKind)
+   : std::move(M));
   }
   return Matchers;
 }
@@ -158,14 +163,21 @@
 Buckets[Cases[I].Matcher.getSupportedKind()].emplace_back(I, Cases[I]);
   }
 
+  // Each anyOf explicitly controls the traversal kind. The anyOf itself is set
+  // to `TK_AsIs` to ensure no nodes are skipped, thereby deferring to the kind
+  // of the branches. Then, each branch is either left as is, if the kind is
+  // already set, or explicitly set to `TK_IgnoreUnlessSpelledInSource`. We
+  // choose this setting, because we think it as the one most friendly to
+  // beginners, who are (largely) the target audience of Transformer.
   std::vector Matchers;
   for (const auto &Bucket : Buckets) {
 DynTypedMatcher M = DynTypedMatcher::constructVariadic(
 DynTypedMatcher::VO_AnyOf, Bucket.first,
-taggedMatchers("Tag", Bucket.second));
+taggedMatchers("Tag", Bucket.second, TK_IgnoreUnlessSpelledInSource));
 M.setAllowBind(true);
 // `tryBind` is guaranteed to succeed, because `AllowBind` was set to true.
-Matchers.push_back(*M.tryBind(RewriteRule::RootID));
+Matchers.push_back

[PATCH] D80606: [libTooling] Fix Transformer to work with ambient traversal kinds

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel updated this revision to Diff 266835.
ymandel marked 2 inline comments as done.
ymandel added a comment.

typo fix


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80606

Files:
  clang/include/clang/Tooling/Transformer/RewriteRule.h
  clang/lib/Tooling/Transformer/RewriteRule.cpp
  clang/unittests/Tooling/TransformerTest.cpp

Index: clang/unittests/Tooling/TransformerTest.cpp
===
--- clang/unittests/Tooling/TransformerTest.cpp
+++ clang/unittests/Tooling/TransformerTest.cpp
@@ -571,6 +571,59 @@
   testRule(Rule, Input, Expected);
 }
 
+// Verifies that a rule with a matcher for an implicit node (like
+// `implicitCastExpr`) does not change the code, when the traversal kind is not
+// explicitly set ti TK_AsIs). In this test, only the rule with the
+// explicit-node matcher will fire.
+TEST_F(TransformerTest, OrderedRuleImplicitIgnored) {
+  std::string Input = R"cc(
+void f1();
+int f2();
+void call_f1() { f1(); }
+float call_f2() { return f2(); }
+  )cc";
+  std::string Expected = R"cc(
+void f1();
+int f2();
+void call_f1() { REPLACE_F1; }
+float call_f2() { return f2(); }
+  )cc";
+
+  RewriteRule ReplaceF1 =
+  makeRule(callExpr(callee(functionDecl(hasName("f1",
+   changeTo(cat("REPLACE_F1")));
+  RewriteRule ReplaceF2 =
+  makeRule(implicitCastExpr(hasSourceExpression(callExpr())),
+   changeTo(cat("REPLACE_F2")));
+  testRule(applyFirst({ReplaceF1, ReplaceF2}), Input, Expected);
+}
+
+// Verifies that explicitly setting the traversal kind fixes the problem in the
+// previous test.
+TEST_F(TransformerTest, OrderedRuleImplicitMatched) {
+  std::string Input = R"cc(
+void f1();
+int f2();
+void call_f1() { f1(); }
+float call_f2() { return f2(); }
+  )cc";
+  std::string Expected = R"cc(
+void f1();
+int f2();
+void call_f1() { REPLACE_F1; }
+float call_f2() { return REPLACE_F2; }
+  )cc";
+
+  RewriteRule ReplaceF1 = makeRule(
+  traverse(clang::TK_AsIs, callExpr(callee(functionDecl(hasName("f1"),
+  changeTo(cat("REPLACE_F1")));
+  RewriteRule ReplaceF2 =
+  makeRule(traverse(clang::TK_AsIs,
+implicitCastExpr(hasSourceExpression(callExpr(,
+   changeTo(cat("REPLACE_F2")));
+  testRule(applyFirst({ReplaceF1, ReplaceF2}), Input, Expected);
+}
+
 //
 // Negative tests (where we expect no transformation to occur).
 //
Index: clang/lib/Tooling/Transformer/RewriteRule.cpp
===
--- clang/lib/Tooling/Transformer/RewriteRule.cpp
+++ clang/lib/Tooling/Transformer/RewriteRule.cpp
@@ -116,10 +116,13 @@
 #endif
 
 // Binds each rule's matcher to a unique (and deterministic) tag based on
-// `TagBase` and the id paired with the case.
+// `TagBase` and the id paired with the case. All of the returned matchers have
+// their traversal kind explicitly set, either based on a pre-set kind or to the
+// provided `DefaultTraversalKind`.
 static std::vector taggedMatchers(
 StringRef TagBase,
-const SmallVectorImpl> &Cases) {
+const SmallVectorImpl> &Cases,
+ast_type_traits::TraversalKind DefaultTraversalKind) {
   std::vector Matchers;
   Matchers.reserve(Cases.size());
   for (const auto &Case : Cases) {
@@ -127,8 +130,10 @@
 // HACK: Many matchers are not bindable, so ensure that tryBind will work.
 DynTypedMatcher BoundMatcher(Case.second.Matcher);
 BoundMatcher.setAllowBind(true);
-auto M = BoundMatcher.tryBind(Tag);
-Matchers.push_back(*std::move(M));
+auto M = *BoundMatcher.tryBind(Tag);
+Matchers.push_back(!M.getTraversalKind()
+   ? M.withTraversalKind(DefaultTraversalKind)
+   : std::move(M));
   }
   return Matchers;
 }
@@ -158,14 +163,21 @@
 Buckets[Cases[I].Matcher.getSupportedKind()].emplace_back(I, Cases[I]);
   }
 
+  // Each anyOf explicitly controls the traversal kind. The anyOf itself is set
+  // to `TK_AsIs` to ensure no nodes are skipped, thereby deferring to the kind
+  // of the branches. Then, each branch is either left as is, if the kind is
+  // already set, or explicitly set to `TK_IgnoreUnlessSpelledInSource`. We
+  // choose this setting, because we think it is the one most friendly to
+  // beginners, who are (largely) the target audience of Transformer.
   std::vector Matchers;
   for (const auto &Bucket : Buckets) {
 DynTypedMatcher M = DynTypedMatcher::constructVariadic(
 DynTypedMatcher::VO_AnyOf, Bucket.first,
-taggedMatchers("Tag", Bucket.second));
+taggedMatchers("Tag", Bucket.second, TK_IgnoreUnlessSpelledInSource));
 M.setAllowBind(true);
 // `tryBind` is guaranteed to succeed, because `AllowBind` was set to true.
-Matchers.push_back(*M.tryBind(RewriteRule::RootID));
+Matcher

[PATCH] D80627: [clang-format] Create a python documentation tool to generate a summary of the clang-format status for the whole of the LLVM project

[Sylvestre Ledru via Phabricator via cfe-commits]

sylvestre.ledru added inline comments.



Comment at: clang/docs/tools/generate_formatted_state.py:48
+if "/test/" in path:
+print root
+continue

print xx is python 2
please use
print(xx)




Comment at: clang/docs/tools/generate_formatted_state.py:71
+
+if (clean==False):
+print os.path.relpath(file_path,LLVM_DIR), ":", "FAIL"

if clean:
and revert the two blocks :)



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80627



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

[PATCH] D80606: [libTooling] Fix Transformer to work with ambient traversal kinds

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

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



Comment at: clang/lib/Tooling/Transformer/RewriteRule.cpp:154
+const RewriteRule &Rule,
+ast_type_traits::TraversalKind DefaultTraversalKind) {
   // Map the cases into buckets of matchers -- one for each "root" AST kind,

gribozavr2 wrote:
> I don't see any callers using the new argument. Do we need this flexibility?
No. Abortive effort to support pulling it from the context. However, turned out 
the context is not available when this is called. Moreover, I've decided it's 
the wrong UX altogether. Matchers are not polymorphic (usually) -- they only 
make sense with respect to a single TK, whether explicit or implicit.  So, my 
goal is that RewriteRules will specify how "open" matchers are interpreted -- 
that is, the matchers TK will be set at rule creation time rather than at rule 
execution time (by pulling from the context).  I don't achieve that goal in 
this revision because I'm only updating `applyFirst`, but I should follow up 
with changes makeRule and the corresponding docs to make this clear.`

That said, I chose `TK_IgnoreUnlessSpelledInSource` as the default setting for 
"open" matchers to be consistent with the consensus that its worth trying this 
out as a default.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80606



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

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

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

mostly LG, sorry for not noticing regularfile check(or maybe forgetting a 
discussion ...) at previous revision




Comment at: clang/lib/Frontend/PrecompiledPreamble.cpp:108
+// (We have some false negatives if PP recovered e.g.  -> "foo")
+if (File == nullptr)
+  return;

um, shouldn't this be

```
if (File != nullptr)
  return;
```

? as we are only interested in not-found files?



Comment at: clang/lib/Frontend/PrecompiledPreamble.cpp:529
+// If a mapped file was previously missing, then it has changed.
+llvm::SmallString<127> MappedPath(R.first);
+if (!VFS->makeAbsolute(MappedPath))

128 seems to be more widely used :P



Comment at: clang/lib/Frontend/PrecompiledPreamble.cpp:595
+if (auto Status = VFS->status(F.getKey())) {
+  if (Status->isRegularFile())
+return false;

what about others? at least symlinks ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77942



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

[PATCH] D80704: Remove WrapperMatcherInterface

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

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

Yes, thank you!  There are (IMO) too many layers of abstractions in this 
framework; this is a good step in the right direction. :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80704



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

[PATCH] D80606: [libTooling] Fix Transformer to work with ambient traversal kinds

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel updated this revision to Diff 266837.
ymandel added a comment.

more typo fixes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80606

Files:
  clang/include/clang/Tooling/Transformer/RewriteRule.h
  clang/lib/Tooling/Transformer/RewriteRule.cpp
  clang/unittests/Tooling/TransformerTest.cpp

Index: clang/unittests/Tooling/TransformerTest.cpp
===
--- clang/unittests/Tooling/TransformerTest.cpp
+++ clang/unittests/Tooling/TransformerTest.cpp
@@ -571,6 +571,59 @@
   testRule(Rule, Input, Expected);
 }
 
+// Verifies that a rule with a top-level matcher for an implicit node (like
+// `implicitCastExpr`) does not change the code, when the AST traversal skips
+// implicit nodes. In this test, only the rule with the explicit-node matcher
+// will fire.
+TEST_F(TransformerTest, OrderedRuleImplicitIgnored) {
+  std::string Input = R"cc(
+void f1();
+int f2();
+void call_f1() { f1(); }
+float call_f2() { return f2(); }
+  )cc";
+  std::string Expected = R"cc(
+void f1();
+int f2();
+void call_f1() { REPLACE_F1; }
+float call_f2() { return f2(); }
+  )cc";
+
+  RewriteRule ReplaceF1 =
+  makeRule(callExpr(callee(functionDecl(hasName("f1",
+   changeTo(cat("REPLACE_F1")));
+  RewriteRule ReplaceF2 =
+  makeRule(implicitCastExpr(hasSourceExpression(callExpr())),
+   changeTo(cat("REPLACE_F2")));
+  testRule(applyFirst({ReplaceF1, ReplaceF2}), Input, Expected);
+}
+
+// Verifies that explicitly setting the traversal kind fixes the problem in the
+// previous test.
+TEST_F(TransformerTest, OrderedRuleImplicitMatched) {
+  std::string Input = R"cc(
+void f1();
+int f2();
+void call_f1() { f1(); }
+float call_f2() { return f2(); }
+  )cc";
+  std::string Expected = R"cc(
+void f1();
+int f2();
+void call_f1() { REPLACE_F1; }
+float call_f2() { return REPLACE_F2; }
+  )cc";
+
+  RewriteRule ReplaceF1 = makeRule(
+  traverse(clang::TK_AsIs, callExpr(callee(functionDecl(hasName("f1"),
+  changeTo(cat("REPLACE_F1")));
+  RewriteRule ReplaceF2 =
+  makeRule(traverse(clang::TK_AsIs,
+implicitCastExpr(hasSourceExpression(callExpr(,
+   changeTo(cat("REPLACE_F2")));
+  testRule(applyFirst({ReplaceF1, ReplaceF2}), Input, Expected);
+}
+
 //
 // Negative tests (where we expect no transformation to occur).
 //
Index: clang/lib/Tooling/Transformer/RewriteRule.cpp
===
--- clang/lib/Tooling/Transformer/RewriteRule.cpp
+++ clang/lib/Tooling/Transformer/RewriteRule.cpp
@@ -116,10 +116,13 @@
 #endif
 
 // Binds each rule's matcher to a unique (and deterministic) tag based on
-// `TagBase` and the id paired with the case.
+// `TagBase` and the id paired with the case. All of the returned matchers have
+// their traversal kind explicitly set, either based on a pre-set kind or to the
+// provided `DefaultTraversalKind`.
 static std::vector taggedMatchers(
 StringRef TagBase,
-const SmallVectorImpl> &Cases) {
+const SmallVectorImpl> &Cases,
+ast_type_traits::TraversalKind DefaultTraversalKind) {
   std::vector Matchers;
   Matchers.reserve(Cases.size());
   for (const auto &Case : Cases) {
@@ -127,8 +130,10 @@
 // HACK: Many matchers are not bindable, so ensure that tryBind will work.
 DynTypedMatcher BoundMatcher(Case.second.Matcher);
 BoundMatcher.setAllowBind(true);
-auto M = BoundMatcher.tryBind(Tag);
-Matchers.push_back(*std::move(M));
+auto M = *BoundMatcher.tryBind(Tag);
+Matchers.push_back(!M.getTraversalKind()
+   ? M.withTraversalKind(DefaultTraversalKind)
+   : std::move(M));
   }
   return Matchers;
 }
@@ -158,14 +163,21 @@
 Buckets[Cases[I].Matcher.getSupportedKind()].emplace_back(I, Cases[I]);
   }
 
+  // Each anyOf explicitly controls the traversal kind. The anyOf itself is set
+  // to `TK_AsIs` to ensure no nodes are skipped, thereby deferring to the kind
+  // of the branches. Then, each branch is either left as is, if the kind is
+  // already set, or explicitly set to `TK_IgnoreUnlessSpelledInSource`. We
+  // choose this setting, because we think it is the one most friendly to
+  // beginners, who are (largely) the target audience of Transformer.
   std::vector Matchers;
   for (const auto &Bucket : Buckets) {
 DynTypedMatcher M = DynTypedMatcher::constructVariadic(
 DynTypedMatcher::VO_AnyOf, Bucket.first,
-taggedMatchers("Tag", Bucket.second));
+taggedMatchers("Tag", Bucket.second, TK_IgnoreUnlessSpelledInSource));
 M.setAllowBind(true);
 // `tryBind` is guaranteed to succeed, because `AllowBind` was set to true.
-Matchers.push_back(*M.tryBind(RewriteRule::RootID));
+Matchers.push_back(
+M.tryBind(Rewrite

cfe-commits@lists.llvm.org

[Alexandros Lamprineas via Phabricator via cfe-commits]

labrinea added inline comments.



Comment at: clang/test/CodeGen/aarch64-bf16-ldst-intrinsics.c:36
+// CHECK32: ret <4 x bfloat> %vld1_lane
+
+bfloat16x8_t test_vld1q_lane_bf16(bfloat16_t const *ptr, bfloat16x8_t src) {

CHECK-NEXT or CHECK-DAG are preferable for sequences.



Comment at: clang/test/CodeGen/aarch64-bf16-ldst-intrinsics.c:180
+// %.fca.0.2.insert = insertvalue %struct.bfloat16x4x3_t %.fca.0.1.insert, <4 
x bfloat> %vld3_lane.fca.2.extract, 0, 2
+
+bfloat16x8x3_t test_vld3q_lane_bf16(bfloat16_t const *ptr, bfloat16x8x3_t src) 
{

where are the check lines?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80716



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

[PATCH] D76793: [Matrix] Implement + and - operators for MatrixType.

[Florian Hahn via Phabricator via cfe-commits]

fhahn updated this revision to Diff 266842.
fhahn marked 4 inline comments as done.
fhahn added a comment.

Use initialization step for all conversions (including for arithemtic types), 
add & call separate addMatrixBinaryArithmeticOverloads


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76793

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/test/CodeGen/matrix-type-operators.c
  clang/test/CodeGenCXX/matrix-type-operators.cpp
  clang/test/Sema/matrix-type-operators.c
  clang/test/SemaCXX/matrix-type-operators.cpp
  llvm/include/llvm/IR/MatrixBuilder.h

Index: llvm/include/llvm/IR/MatrixBuilder.h
===
--- llvm/include/llvm/IR/MatrixBuilder.h
+++ llvm/include/llvm/IR/MatrixBuilder.h
@@ -127,6 +127,16 @@
   /// Add matrixes \p LHS and \p RHS. Support both integer and floating point
   /// matrixes.
   Value *CreateAdd(Value *LHS, Value *RHS) {
+assert(LHS->getType()->isVectorTy() || RHS->getType()->isVectorTy());
+if (LHS->getType()->isVectorTy() && !RHS->getType()->isVectorTy())
+  RHS = B.CreateVectorSplat(
+  cast(LHS->getType())->getNumElements(), RHS,
+  "scalar.splat");
+else if (!LHS->getType()->isVectorTy() && RHS->getType()->isVectorTy())
+  LHS = B.CreateVectorSplat(
+  cast(RHS->getType())->getNumElements(), LHS,
+  "scalar.splat");
+
 return cast(LHS->getType())
->getElementType()
->isFloatingPointTy()
@@ -137,6 +147,16 @@
   /// Subtract matrixes \p LHS and \p RHS. Support both integer and floating
   /// point matrixes.
   Value *CreateSub(Value *LHS, Value *RHS) {
+assert(LHS->getType()->isVectorTy() || RHS->getType()->isVectorTy());
+if (LHS->getType()->isVectorTy() && !RHS->getType()->isVectorTy())
+  RHS = B.CreateVectorSplat(
+  cast(LHS->getType())->getNumElements(), RHS,
+  "scalar.splat");
+else if (!LHS->getType()->isVectorTy() && RHS->getType()->isVectorTy())
+  LHS = B.CreateVectorSplat(
+  cast(RHS->getType())->getNumElements(), LHS,
+  "scalar.splat");
+
 return cast(LHS->getType())
->getElementType()
->isFloatingPointTy()
Index: clang/test/SemaCXX/matrix-type-operators.cpp
===
--- clang/test/SemaCXX/matrix-type-operators.cpp
+++ clang/test/SemaCXX/matrix-type-operators.cpp
@@ -114,3 +114,96 @@
   a[2] = f;
   // expected-error@-1 {{single subscript expressions are not allowed for matrix values}}
 }
+
+template 
+struct MyMatrix {
+  using matrix_t = EltTy __attribute__((matrix_type(Rows, Columns)));
+
+  matrix_t value;
+};
+
+template 
+typename MyMatrix::matrix_t add(MyMatrix &A, MyMatrix &B) {
+  char *v1 = A.value + B.value;
+  // expected-error@-1 {{cannot initialize a variable of type 'char *' with an rvalue of type 'MyMatrix::matrix_t' (aka 'unsigned int __attribute__((matrix_type(2, 2)))')}}
+  // expected-error@-2 {{invalid operands to binary expression ('MyMatrix::matrix_t' (aka 'unsigned int __attribute__((matrix_type(3, 3)))') and 'MyMatrix::matrix_t' (aka 'float __attribute__((matrix_type(2, 2)))'))}}
+  // expected-error@-3 {{invalid operands to binary expression ('MyMatrix::matrix_t' (aka 'unsigned int __attribute__((matrix_type(2, 2)))') and 'MyMatrix::matrix_t' (aka 'unsigned int __attribute__((matrix_type(3, 3)))'))}}
+
+  return A.value + B.value;
+  // expected-error@-1 {{invalid operands to binary expression ('MyMatrix::matrix_t' (aka 'unsigned int __attribute__((matrix_type(3, 3)))') and 'MyMatrix::matrix_t' (aka 'float __attribute__((matrix_type(2, 2)))'))}}
+  // expected-error@-2 {{invalid operands to binary expression ('MyMatrix::matrix_t' (aka 'unsigned int __attribute__((matrix_type(2, 2)))') and 'MyMatrix::matrix_t' (aka 'unsigned int __attribute__((matrix_type(3, 3)))'))}}
+}
+
+void test_add_template(unsigned *Ptr1, float *Ptr2) {
+  MyMatrix Mat1;
+  MyMatrix Mat2;
+  MyMatrix Mat3;
+  Mat1.value = *((decltype(Mat1)::matrix_t *)Ptr1);
+  unsigned v1 = add(Mat1, Mat1);
+  // expected-error@-1 {{cannot initialize a variable of type 'unsigned int' with an rvalue of type 'typename MyMatrix::matrix_t' (aka 'unsigned int __attribute__((matrix_type(2, 2)))')}}
+  // expected-note@-2 {{in instantiation of function template specialization 'add' requested here}}
+
+  Mat1.value = add(Mat1, Mat2);
+  // expected-note@-1 {{in instantiation of function template specialization 'add' requested here}}
+
+  Mat1.value = add(Mat2, Mat3);
+  // expected-note@-1 {{in instantiation of function template specialization 'add' requested here}}
+}
+
+template 
+typename MyMatrix::matrix_t subtract(MyMatrix &A, MyMatrix &B) {
+  char *v1 = A.value - B.value;
+  // expected-error

[PATCH] D80522: [Analyzer] [NFC] Parameter Regions -- Alternative Approach

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

baloghadamsoftware updated this revision to Diff 266851.
baloghadamsoftware marked an inline comment as done.
baloghadamsoftware added a comment.

Updated according to the comments of @balazske.


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

https://reviews.llvm.org/D80522

Files:
  clang/include/clang/StaticAnalyzer/Checkers/SValExplainer.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/Regions.def
  clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
  clang/lib/StaticAnalyzer/Core/CallEvent.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
  clang/lib/StaticAnalyzer/Core/MemRegion.cpp
  clang/lib/StaticAnalyzer/Core/Store.cpp
  clang/test/Analysis/explain-svals.c
  clang/test/Analysis/explain-svals.cpp
  clang/test/Analysis/explain-svals.m
  clang/unittests/StaticAnalyzer/CMakeLists.txt
  clang/unittests/StaticAnalyzer/ParamRegionTest.cpp

Index: clang/unittests/StaticAnalyzer/ParamRegionTest.cpp
===
--- /dev/null
+++ clang/unittests/StaticAnalyzer/ParamRegionTest.cpp
@@ -0,0 +1,109 @@
+//===- unittests/StaticAnalyzer/ParamRegionTest.cpp ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "Reusables.h"
+
+#include "clang/Tooling/Tooling.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace ento {
+namespace {
+
+class ParamRegionTestConsumer : public ExprEngineConsumer {
+  void performTest(const Decl *D) {
+StoreManager &StMgr = Eng.getStoreManager();
+MemRegionManager &MRMgr = StMgr.getRegionManager();
+const StackFrameContext *SFC =
+Eng.getAnalysisDeclContextManager().getStackFrame(D);
+
+if (const auto *FD = dyn_cast(D)) {
+  for (const auto *P : FD->parameters()) {
+const TypedValueRegion *Reg = MRMgr.getVarRegion(P, SFC);
+if (SFC->inTopFrame())
+  assert(isa(Reg));
+else
+  assert(isa(Reg));
+  }
+} else if (const auto *CD = dyn_cast(D)) {
+  for (const auto *P : CD->parameters()) {
+const TypedValueRegion *Reg = MRMgr.getVarRegion(P, SFC);
+if (SFC->inTopFrame())
+  assert(isa(Reg));
+else
+  assert(isa(Reg));
+  }
+} else if (const auto *MD = dyn_cast(D)) {
+  for (const auto *P : MD->parameters()) {
+const TypedValueRegion *Reg = MRMgr.getVarRegion(P, SFC);
+if (SFC->inTopFrame())
+  assert(isa(Reg));
+else
+  assert(isa(Reg));
+  }
+}
+  }
+
+public:
+  ParamRegionTestConsumer(CompilerInstance &C) : ExprEngineConsumer(C) {}
+
+  bool HandleTopLevelDecl(DeclGroupRef DG) override {
+for (const auto *D : DG) {
+  performTest(D);
+}
+return true;
+  }
+};
+
+class ParamRegionTestAction : public ASTFrontendAction {
+public:
+  std::unique_ptr CreateASTConsumer(CompilerInstance &Compiler,
+ StringRef File) override {
+return std::make_unique(Compiler);
+  }
+};
+
+TEST(ParamRegion, ParamRegionTest) {
+  EXPECT_TRUE(
+  tooling::runToolOnCode(std::make_unique(),
+ R"(void foo(int n) {
+ auto lambda = [n](int m) {
+   return n + m;
+ };
+
+ int k = lambda(2);
+   }
+
+   void bar(int l) {
+ foo(l);
+   }
+
+   struct S {
+ int n;
+ S(int nn): n(nn) {}
+   };
+
+   void baz(int p) {
+ S s(p);
+   })"));
+  EXPECT_TRUE(
+  tooling::runToolOnCode(std::make_unique(),
+ R"(@interface O
+   + alloc;
+   - initWithInt:(int)q;
+   @end
+
+   void qix(int r) {
+ O *o = [[O alloc] initWithInt:r];
+   })",
+ "input.m"));
+}
+
+} // namespace
+} // namespace ento
+} // namespace clang
Index: clang/unittests/StaticAnalyzer/CMakeLists.txt
===
--- clang/unittests/StaticAnaly

[PATCH] D80522: [Analyzer] [NFC] Parameter Regions -- Alternative Approach

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

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



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h:917
 assert(isa(sReg) || isa(sReg) ||
isa(sReg) || isa(sReg));
   }

balazske wrote:
> This assert should not be duplicated. It has the place in `VarRegion` or 
> `NonParamVarRegion`, but not both.
Copy-paste...


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

https://reviews.llvm.org/D80522



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

[PATCH] D80522: [Analyzer] [NFC] Parameter Regions -- Alternative Approach

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

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

> I will check tomorrow whether we could remove `DeclRegion` without too much 
> code duplication.

It turns out that `DeclRegion` is used in 7 different checkers plus in 
`BugReporterVisitor`s. One typical use case is to get a name for the region (by 
getting the `Decl` and trying to cast it to `NamedDecl`), but there are also 
others as well. For now I suggest to keep it to avoid code duplication.


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

https://reviews.llvm.org/D80522



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

[PATCH] D80627: [clang-format] Create a python documentation tool to generate a summary of the clang-format status for the whole of the LLVM project

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay planned changes to this revision.
MyDeveloperDay added a comment.

I'm running through flake8... almost every line is changing ;-) ...let me 
update before wasting any more of your time.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80627



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

[PATCH] D80669: [analyzer] LoopWidening: fix crash by avoiding aliased references invalidation

[Abbas Sabra via Phabricator via cfe-commits]

AbbasSabra added a comment.

In D80669#2058824 , @NoQ wrote:

> Fair!


Thanks for the review! May you please take care of merging this one too?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80669



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

cfe-commits@lists.llvm.org

[Ties Stuij via Phabricator via cfe-commits]

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

We need testing for the backend code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80716



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

[PATCH] D44823: [libcxx] Improving std::vector and std::deque perfomance

[Louis Dionne via Phabricator via cfe-commits]

ldionne added inline comments.



Comment at: libcxx/trunk/include/__split_buffer:201
 __alloc_rr& __a = this->__alloc();
+pointer __to_be_end = this->__end_;
 do

danlark wrote:
> hiraditya wrote:
> > danlark wrote:
> > > lichray wrote:
> > > > mclow.lists wrote:
> > > > > I have been asked specifically by the optimizer folks to NOT do 
> > > > > things like this in libc++, but rather to file bugs against the 
> > > > > optimizer.
> > > > > 
> > > > > And I have done so for this exact case:  
> > > > > https://bugs.llvm.org/show_bug.cgi?id=35637
> > > > From the thread I didn't see that the compiler side asked you not to do 
> > > > so.
> > > > 
> > > > And I disagree with the view.  libc++ shouldn't *wait* for compilers, 
> > > > because we don't dictate users' compiler choices.  This change doesn't 
> > > > make libc++ worse to coming compilers, and makes libc++ better on 
> > > > existing compilers, so what benefit we get by not approving this?
> > > So, what is the status? Are we waiting for the compiler code-gen fix?
> > > 
> > > At Yandex we are using patched version like half a year or more.
> > > 
> > > https://github.com/catboost/catboost/blob/master/contrib/libs/cxxsupp/libcxx/include/vector#L995
> > It would be great to get this patch in. Waiting for compiler for this 
> > optimization seems overkill.
> It was separately submitted by the libcxx mainterner in July 2019 --  
> https://reviews.llvm.org/rL367183
Did https://reviews.llvm.org/rL367183 fix your problem? If so, let's abandon 
this patch and you can remove your downstream patch as well.


Repository:
  rCXX libc++

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

https://reviews.llvm.org/D44823



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

[PATCH] D80197: [DebugInfo] Upgrade DISubrange to support Fortran dynamic arrays

[Alok Kumar Sharma via Phabricator via cfe-commits]

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



Comment at: llvm/lib/Bitcode/Writer/BitcodeWriter.cpp:1522
 
 static uint64_t rotateSign(int64_t I) {
   uint64_t U = I;

melver wrote:
> rotateSign() is no longer used in this file. If there are no plans to use it 
> again, please remove it.
> Thanks!
Thanks. It is fixed in a0d847c6cdcbe167213d91313577c57073d5c013 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80197



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

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

[Sam McCall via Phabricator via cfe-commits]

sammccall marked 10 inline comments as done.
sammccall added inline comments.



Comment at: clang-tools-extra/clangd/CompileCommands.cpp:156
+  llvm::SmallString<256> Resolved;
+  if (llvm::sys::fs::real_path(Driver, Resolved))
+return Driver.str();

kadircet wrote:
> what about taking a VFS instead and calling `VFS.getRealPath`?
> 
> It should make testing easier and commandmangler vfs friendly.
That's a more invasive change I don't feel up to at the moment.
CommandMangler doesn't make much sense if you don't make assumptions about the 
FS being real (e.g. looking up things on the PATH, using the output of xcrun...)
I don't think testing alone is a strong reason to VFSify it.



Comment at: clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp:140
+  std::vector Cmd = {(TempDir + "/bin/foo").str(), "foo.cc"};
+  Mangler.adjust(Cmd);
+  // Directory based on resolved symlink, basename preserved.

kadircet wrote:
> irrelevant to the patch:
> 
> any reason for `adjust` to mutate its parameter in-place instead of returning 
> it(I believe callers can do `std::move(Cmd)` if need be) ?
Not a strong reason either way I think.



Comment at: clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp:144
+
+  // Set PATH to point to temp/bin so we can find 'foo' on it.
+  ASSERT_TRUE(::getenv("PATH"));

kadircet wrote:
> can we rather move the `ifdef` here the rest previous part should hopefully 
> work on other platforms as well (at least after VFS)?
I don't think they'll work: create_link doesn't create a symlink on windows, 
PATHEXT is a thing, etc.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75414



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

[PATCH] D80016: [analyzer] StdLibraryFunctionsChecker: Add support to lookup types

[Gabor Marton via Phabricator via cfe-commits]

martong updated this revision to Diff 266858.
martong marked 2 inline comments as done.
martong added a comment.

- Add `if (FileTy)`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80016

Files:
  clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
  clang/test/Analysis/std-c-library-functions-arg-constraints.c
  clang/test/Analysis/std-c-library-functions-lookup.c
  clang/test/Analysis/std-c-library-functions-lookup.cpp
  clang/test/Analysis/std-c-library-functions.c

Index: clang/test/Analysis/std-c-library-functions.c
===
--- clang/test/Analysis/std-c-library-functions.c
+++ clang/test/Analysis/std-c-library-functions.c
@@ -53,10 +53,10 @@
 // CHECK-NEXT: Loaded summary for: int getc(FILE *)
 // CHECK-NEXT: Loaded summary for: int fgetc(FILE *)
 // CHECK-NEXT: Loaded summary for: int getchar()
+// CHECK-NEXT: Loaded summary for: unsigned int fread(void *restrict, size_t, size_t, FILE *restrict)
+// CHECK-NEXT: Loaded summary for: unsigned int fwrite(const void *restrict, size_t, size_t, FILE *restrict)
 // CHECK-NEXT: Loaded summary for: ssize_t read(int, void *, size_t)
 // CHECK-NEXT: Loaded summary for: ssize_t write(int, const void *, size_t)
-// CHECK-NEXT: Loaded summary for: unsigned int fread(void *restrict, size_t, size_t, FILE *)
-// CHECK-NEXT: Loaded summary for: unsigned int fwrite(const void *restrict, size_t, size_t, FILE *restrict)
 // CHECK-NEXT: Loaded summary for: ssize_t getline(char **, size_t *, FILE *)
 
 void clang_analyzer_eval(int);
@@ -104,7 +104,7 @@
   }
 }
 
-size_t fread(void *restrict, size_t, size_t, FILE *);
+size_t fread(void *restrict, size_t, size_t, FILE *restrict);
 size_t fwrite(const void *restrict, size_t, size_t, FILE *restrict);
 void test_fread_fwrite(FILE *fp, int *buf) {
 
Index: clang/test/Analysis/std-c-library-functions-lookup.cpp
===
--- /dev/null
+++ clang/test/Analysis/std-c-library-functions-lookup.cpp
@@ -0,0 +1,23 @@
+// RUN: %clang_analyze_cc1 %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=apiModeling.StdCLibraryFunctions \
+// RUN:   -analyzer-config apiModeling.StdCLibraryFunctions:DisplayLoadedSummaries=true \
+// RUN:   -analyzer-checker=debug.ExprInspection \
+// RUN:   -analyzer-config eagerly-assume=false \
+// RUN:   -triple i686-unknown-linux 2>&1 | FileCheck %s
+
+//  CHECK: Loaded summary for: size_t fread(void *, size_t, size_t, FILE *)
+//  CHECK-NOT: Loaded summary for: size_t fread(void *, size_t, size_t, MyFile *)
+
+typedef unsigned int size_t;
+typedef struct FILE FILE;
+size_t fread(void *, size_t, size_t, FILE *);
+
+struct MyFile;
+size_t fread(void *, size_t, size_t, MyFile *);
+
+// Must have at least one call expression to initialize the summary map.
+int bar(void);
+void foo() {
+  bar();
+}
Index: clang/test/Analysis/std-c-library-functions-lookup.c
===
--- /dev/null
+++ clang/test/Analysis/std-c-library-functions-lookup.c
@@ -0,0 +1,19 @@
+// RUN: %clang_analyze_cc1 %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=apiModeling.StdCLibraryFunctions \
+// RUN:   -analyzer-config apiModeling.StdCLibraryFunctions:DisplayLoadedSummaries=true \
+// RUN:   -analyzer-checker=debug.ExprInspection \
+// RUN:   -analyzer-config eagerly-assume=false \
+// RUN:   -triple i686-unknown-linux 2>&1 | FileCheck %s
+
+// CHECK: Loaded summary for: unsigned int fread(void *restrict, size_t, size_t, FILE *restrict)
+
+typedef typeof(sizeof(int)) size_t;
+typedef struct FILE FILE;
+size_t fread(void *restrict, size_t, size_t, FILE *restrict);
+
+// Must have at least one call expression to initialize the summary map.
+int bar(void);
+void foo() {
+  bar();
+}
Index: clang/test/Analysis/std-c-library-functions-arg-constraints.c
===
--- clang/test/Analysis/std-c-library-functions-arg-constraints.c
+++ clang/test/Analysis/std-c-library-functions-arg-constraints.c
@@ -64,7 +64,7 @@
 
 typedef struct FILE FILE;
 typedef typeof(sizeof(int)) size_t;
-size_t fread(void *restrict, size_t, size_t, FILE *);
+size_t fread(void *restrict, size_t, size_t, FILE *restrict);
 void test_notnull_concrete(FILE *fp) {
   fread(0, sizeof(int), 10, fp); // \
   // report-warning{{Function argument constraint is not satisfied}} \
Index: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -568,6 +568,26 @@
   return findFunctionSummary(FD, C);
 }
 
+llvm::Optional lookupType(StringRef Name, const ASTContext &ACtx) {
+  IdentifierInfo &II = ACtx.Idents.get

[PATCH] D79711: [ARM] Add poly64_t on AArch32.

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

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

Ok, LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79711



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

cfe-commits@lists.llvm.org

[Simon Tatham via Phabricator via cfe-commits]

simon_tatham added inline comments.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:10366
+auto Alignment = CGM.getNaturalPointeeTypeAlignment(
+E->getArg(0)->IgnoreParenCasts()->getType());
 Ops[0] = Builder.CreateBitCast(Ops[0], llvm::PointerType::getUnqual(VTy));

What effect is this change of strategy having on the alignment computation, for 
the already-supported instances of this builtin?

It //looks// to me as if `__builtin_neon_vld1_v` with (say) a `uint8_t *` 
pointer argument will now compute `Alignment=1` (the natural alignment of the 
pointee type), whereas it would previously have computed `Alignment=8` (the 
size of the whole vector being loaded or stored).

Is that intentional? Or accidental? Or have I completely misunderstood what's 
going on?

(Whichever of the three, some discussion in the commit message would be a good 
idea, explaining why this change does or does not make a difference, as 
appropriate.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80716



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

[PATCH] D80016: [analyzer] StdLibraryFunctionsChecker: Add support to lookup types

[Gabor Marton via Phabricator via cfe-commits]

martong added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:747
+  : *FilePtrTy)
+: None;
+

balazske wrote:
> martong wrote:
> > balazske wrote:
> > > The `Optional` can be left out from the right side expressions 
> > > (in the `?` operator part). (A `QualType` should be assignable to 
> > > `Optional`.)
> > No that cannot be left out. Implicit conversion does not play when trying 
> > to determine the common type for the 2nd and 3rd argument of the ternary 
> > op. 
> > https://stackoverflow.com/questions/32251419/c-ternary-operator-conditional-operator-and-its-implicit-type-conversion-r
> > So, removing the explicit type I get a compile error.
> Still I do not like that code. Probably this is better then:
> ```
>   Optional FilePtrTy, FilePtrRestrictTy;
>   if (FileTy) {
> FilePtrTy = ACtx.getPointerType(*FileTy);
> FilePtrRestrictTy = ACtx.getLangOpts().C99
> ? ACtx.getRestrictType(*FilePtrTy) // FILE 
> *restrict
> : *FilePtrTy)
>   }
> ```
Ok, I added the `if`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80016



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

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

[Sam McCall via Phabricator via cfe-commits]

sammccall updated this revision to Diff 266861.
sammccall marked an inline comment as done.
sammccall added a comment.

Handle -no-canonical-prefixes
Make memoize map-like instead of functor-like
Address other comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75414

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

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

[clang] 6c2b7ee - Prevent test from failing in my home directory

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2020-05-28T16:14:49+02:00
New Revision: 6c2b7ee2f7fac7b683e343c2c383b7e67fadf9f8

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

LOG: Prevent test from failing in my home directory

Added: 


Modified: 
clang/test/Headers/nvptx_device_math_macro.cpp

Removed: 




diff  --git a/clang/test/Headers/nvptx_device_math_macro.cpp 
b/clang/test/Headers/nvptx_device_math_macro.cpp
index e21aa2b072b9..02bdc1f35b0b 100644
--- a/clang/test/Headers/nvptx_device_math_macro.cpp
+++ b/clang/test/Headers/nvptx_device_math_macro.cpp
@@ -8,9 +8,9 @@
 #pragma omp declare target
 int use_macro() {
   double a(0);
-// CHECK-NOT:  call
+// CHECK-NOT:  call {{.*}}
 // CHECK:  call double @llvm.fabs.f64(double
-// CHECK-NOT:  call
+// CHECK-NOT:  call {{.*}}
 // CHECK:  ret i32 %conv
   return (std::fpclassify(a) != FP_ZERO);
 }



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

[PATCH] D80222: Replace Clang's createRuntimeFunction with the definitions in OMPKinds.def

[Joseph Huber via Phabricator via cfe-commits]

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



Comment at: openmp/runtime/test/tasking/kmp_taskloop.c:100
 th_counter[i] = 0;
-#pragma omp parallel num_threads(N)
+#pragma omp parallel // num_threads(N)
 {

jhuber6 wrote:
> jdoerfert wrote:
> > jhuber6 wrote:
> > > jdoerfert wrote:
> > > > jhuber6 wrote:
> > > > > jdoerfert wrote:
> > > > > > jhuber6 wrote:
> > > > > > > AndreyChurbanov wrote:
> > > > > > > > jhuber6 wrote:
> > > > > > > > > jhuber6 wrote:
> > > > > > > > > > jdoerfert wrote:
> > > > > > > > > > > jhuber6 wrote:
> > > > > > > > > > > > I am not entirely sure why, but commenting this out 
> > > > > > > > > > > > causes the problem to go away. I tried adding proper 
> > > > > > > > > > > > names to the forward-declared functions but since clang 
> > > > > > > > > > > > already knew I had something called ident_t, I couldn't 
> > > > > > > > > > > > declare a new struct with the same name.
> > > > > > > > > > > This is not good. The difference should only be that the 
> > > > > > > > > > > `kmpc_fork_call` has a different argument, right? Does 
> > > > > > > > > > > the segfault happen at compile or runtime?
> > > > > > > > > > > 
> > > > > > > > > > > You can just use the ident_t clang created, right? Did 
> > > > > > > > > > > you print the function names requested by clang as we 
> > > > > > > > > > > discussed?
> > > > > > > > > > I added an assertion and debug statements. If I try to 
> > > > > > > > > > declare a struct named "Ident_t" I get the following error 
> > > > > > > > > > message in the seg-fault. I think the seg-fault is 
> > > > > > > > > > compile-time.
> > > > > > > > > > 
> > > > > > > > > > Found OpenMP runtime function __kmpc_global_thread_num with 
> > > > > > > > > > type i32 (%struct.ident_t.0*). Expected type is i32 
> > > > > > > > > > (%struct.ident_t*)
> > > > > > > > > > clang: 
> > > > > > > > > > /home/jhuber/Documents/llvm-project/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:124:
> > > > > > > > > >  static llvm::Function* 
> > > > > > > > > > llvm::OpenMPIRBuilder::getOrCreateRuntimeFunction(llvm::Module&,
> > > > > > > > > >  llvm::omp::RuntimeFunction): Assertion `FnTy == 
> > > > > > > > > > Fn->getFunctionType() && "Found OpenMP runtime function has 
> > > > > > > > > > mismatched types"' failed.
> > > > > > > > > I'm not sure if there's a way around this without changing 
> > > > > > > > > the getOrCreateRuntimeFunction method to return a 
> > > > > > > > > FunctionCallee and removing the assertion. Clang doesn't know 
> > > > > > > > > about the ident_t struct when it's compiling the file, but 
> > > > > > > > > when its doing the codegen it sees two structs with the same 
> > > > > > > > > name and creates a new name. So when it gets the types it 
> > > > > > > > > says that ident_t and ident_t.0 don't match. As you said the 
> > > > > > > > > old version got around this by adding a bitcasting 
> > > > > > > > > instruction so it knew how to turn it into an ident_t pointer.
> > > > > > > > Note that this change breaks the test on any system with more 
> > > > > > > > that 4 procs.  Because array th_counter[4] is indexed by thread 
> > > > > > > > number which can easily be greater than 3 if number of threads 
> > > > > > > > is not limited.
> > > > > > > The problem was that the num_threads clause required an implicit 
> > > > > > > call to kmpc_global_thread_num so it could be passed to 
> > > > > > > kmpc_push_num_threads. The types of the implicit function and the 
> > > > > > > forward declaration then wouldn't match up. I added another 
> > > > > > > forward declaration to explicitly call kmpc_push_num_threads. Is 
> > > > > > > this a sufficient solution?
> > > > > > We need this to work with num_threads(8). 
> > > > > > 
> > > > > > > Clang doesn't know about the ident_t struct when it's compiling 
> > > > > > > the file, but when its doing the codegen it sees two structs with 
> > > > > > > the same name and creates a new name.
> > > > > > 
> > > > > > Where are the two structs coming from? We should have one. If clang 
> > > > > > introduces one it needs to use the one from OMPKindes.def instead. 
> > > > > > Is that a fix?
> > > > > The first struct is the one that I'm assuming comes from the OpenMP 
> > > > > CodeGen that places the Ident_t struct in the IR file. if I declare a 
> > > > > struct also named ident_t in the C source file it most likely will 
> > > > > see that there's two structs with the same name and call the second 
> > > > > one "ident_t.0" internally. The other ident_t struct is only known 
> > > > > once clang generates the LLVM IR so I can't just use "ident_t" nor 
> > > > > can I declare a struct with the same name.
> > > > 1) Either Clang needs to use the `llvm::omp::types::Ident` *or* Clang 
> > > > needs to define `llvm::omp::types::Ident` and we do not do it via  
> > > > `__OMP_STRUCT_TYPE(Ident, ident_t, Int32, Int32, Int32, Int32, 
> >

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

[Sam McCall via Phabricator via cfe-commits]

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

address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77942

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

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

[PATCH] D78454: [clangd] Highlight related control flow.

[Russell Gallop via Phabricator via cfe-commits]

russell.gallop added a comment.

Hi Sam,

It looks like this is causing a failure on the Windows PS4 buildbot: 
http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast/builds/32606

Please could you take a look? PS4 target disables RTTI, hence exceptions, by 
default so it is probably related to that.

Thanks
Russ


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78454



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

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

[Sam McCall via Phabricator via cfe-commits]

sammccall marked 5 inline comments as done.
sammccall added inline comments.



Comment at: clang/lib/Frontend/PrecompiledPreamble.cpp:108
+// (We have some false negatives if PP recovered e.g.  -> "foo")
+if (File == nullptr)
+  return;

kadircet wrote:
> um, shouldn't this be
> 
> ```
> if (File != nullptr)
>   return;
> ```
> 
> ? as we are only interested in not-found files?
Yeah, I guess I should have run the tests :-)



Comment at: clang/lib/Frontend/PrecompiledPreamble.cpp:595
+if (auto Status = VFS->status(F.getKey())) {
+  if (Status->isRegularFile())
+return false;

kadircet wrote:
> what about others? at least symlinks ?
RealFileSystem::status() resolves symlinks (calls fs::status() with no 
`resolve` arg, defaults to true). That's the most important case, and other 
VFSes that support symlinks should really do the same for compatibility.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77942



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

[PATCH] D80723: [PowerPC] Convert vec_splats functions to macros

[Colin Samples via Phabricator via cfe-commits]

vddvss created this revision.
vddvss added reviewers: nemanjai, PowerPC.
Herald added subscribers: cfe-commits, shchenz, kbarton.
Herald added a project: clang.

This commit converts the `vec_splats` functions in altivec.h to macros, solving 
an issue where `vec_splats` calls could not assign to variables of static 
storage duration, such as:

  static vector int x = vec_splats(1);

Since `vec_splats` was implemented as a function, code such as in this example 

 would result in a compile-time error in `clang`. This differs from `gcc`, 
which allows this construct.

This updates tests accordingly, and fixes PR44276 and PR44455. Sorry for the 
delay in getting this to you.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D80723

Files:
  clang/lib/Headers/altivec.h
  clang/test/CodeGen/ppc-emmintrin.c
  clang/test/CodeGen/ppc-mmintrin.c
  clang/test/CodeGen/ppc-pmmintrin.c
  clang/test/CodeGen/ppc-smmintrin.c
  clang/test/CodeGen/ppc-tmmintrin.c
  clang/test/CodeGen/ppc-xmmintrin.c
  clang/test/CodeGen/pr44276.c

Index: clang/test/CodeGen/pr44276.c
===
--- /dev/null
+++ clang/test/CodeGen/pr44276.c
@@ -0,0 +1,26 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
+// REQUIRES: powerpc-registered-target
+// RUN: %clang -S -emit-llvm -target powerpc64-unknown-unknown -mcpu=pwr8 %s -o - | FileCheck %s
+
+// Check that this compiles
+
+#include 
+
+// CHECK-LABEL: @test(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:ret void
+//
+void test() {
+  static vector unsigned char a = vec_splats('1');
+  static vector signed char b = vec_splats((signed char)'1');
+  static vector unsigned short c = vec_splats(1U);
+  static vector signed short d = vec_splats(1);
+  static vector unsigned int e = vec_splats(1U);
+  static vector signed int f = vec_splats(1);
+  static vector float g = vec_splats(1.0f);
+  static vector unsigned long long h = vec_splats(1ULL);
+  static vector signed long long i = vec_splats(1LL);
+  static vector double j = vec_splats(1.0);
+  static vector unsigned __int128 k = vec_splats((__int128)1);
+  static vector signed __int128 l = vec_splats((__int128)1);
+}
Index: clang/test/CodeGen/ppc-xmmintrin.c
===
--- clang/test/CodeGen/ppc-xmmintrin.c
+++ clang/test/CodeGen/ppc-xmmintrin.c
@@ -1,4 +1,3 @@
-// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
 // REQUIRES: powerpc-registered-target
 
 // RUN: %clang -S -emit-llvm -target powerpc64-unknown-linux-gnu -mcpu=pwr8 -ffreestanding -DNO_WARN_X86_INTRINSICS %s \
@@ -66,11 +65,13 @@
 // CHECK: store i64 {{[0-9a-zA-Z_%.]+}}, i64* {{[0-9a-zA-Z_%.]+}}, align 8
 // CHECK-NEXT: store i64 {{[0-9a-zA-Z_%.]+}}, i64* {{[0-9a-zA-Z_%.]+}}, align 8
 // CHECK-NEXT: [[REG25:[0-9a-zA-Z_%.]+]] = load i64, i64* {{[0-9a-zA-Z_%.]+}}, align 8
-// CHECK-NEXT: [[REG26:[0-9a-zA-Z_%.]+]] = call <2 x i64> @vec_splats(unsigned long long)(i64 [[REG25]])
+// CHECK-NEXT: [[REG26A:[0-9a-zA-Z_%.]+]] = insertelement <2 x i64> undef, i64 [[REG25]], i32 0
+// CHECK-NEXT: [[REG26:[0-9a-zA-Z_%.]+]]  = shufflevector <2 x i64> [[REG26A]], <2 x i64> undef, <2 x i32> zeroinitializer
 // CHECK-NEXT: [[REG27:[0-9a-zA-Z_%.]+]] = bitcast <2 x i64> [[REG26]] to <8 x i16>
 // CHECK-NEXT: store <8 x i16> [[REG27]], <8 x i16>* {{[0-9a-zA-Z_%.]+}}, align 16
 // CHECK-NEXT: [[REG28:[0-9a-zA-Z_%.]+]] = load i64, i64* {{[0-9a-zA-Z_%.]+}}, align 8
-// CHECK-NEXT: [[REG29:[0-9a-zA-Z_%.]+]] = call <2 x i64> @vec_splats(unsigned long long)(i64 [[REG28]])
+// CHECK-NEXT: [[REG29A:[0-9a-zA-Z_%.]+]] = insertelement <2 x i64> undef, i64 [[REG28]], i32 0
+// CHECK-NEXT: [[REG29:[0-9a-zA-Z_%.]+]]  = shufflevector <2 x i64> [[REG29A]], <2 x i64> undef, <2 x i32> zeroinitializer
 // CHECK-NEXT: [[REG30:[0-9a-zA-Z_%.]+]] = bitcast <2 x i64> [[REG29]] to <8 x i16>
 // CHECK-NEXT: store <8 x i16> [[REG30]], <8 x i16>* {{[0-9a-zA-Z_%.]+}}, align 16
 // CHECK-NEXT: [[REG31:[0-9a-zA-Z_%.]+]] = load <8 x i16>, <8 x i16>* {{[0-9a-zA-Z_%.]+}}, align 16
@@ -86,11 +87,13 @@
 // CHECK: store i64 {{[0-9a-zA-Z_%.]+}}, i64* {{[0-9a-zA-Z_%.]+}}, align 8
 // CHECK-NEXT: store i64 {{[0-9a-zA-Z_%.]+}}, i64* {{[0-9a-zA-Z_%.]+}}, align 8
 // CHECK-NEXT: [[REG37:[0-9a-zA-Z_%.]+]] = load i64, i64* {{[0-9a-zA-Z_%.]+}}, align 8
-// CHECK-NEXT: [[REG38:[0-9a-zA-Z_%.]+]] = call <2 x i64> @vec_splats(unsigned long long)(i64 [[REG37]])
+// CHECK-NEXT: [[REG38A:[0-9a-zA-Z_%.]+]] = insertelement <2 x i64> undef, i64 [[REG37]], i32 0
+// CHECK-NEXT: [[REG38:[0-9a-zA-Z_%.]+]]  = shufflevector <2 x i64> [[REG38A]], <2 x i64> undef, <2 x i32> zeroinitializer
 // CHECK-NEXT: [[REG39:[0-9a-zA-Z_%.]+]] = bitcast <2 x i64> [[REG38]] to <16 x i8>
 // CHECK-NEXT: store <16 x i8> [[REG39]], <16 x i8>* {{[0-9a-zA-Z_%.]+}}, align 16

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

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp:112
 
+Optional ExprEngine::retrieveFromConstructionContext(
+ProgramStateRef State, const LocationContext *LCtx,

balazske wrote:
> baloghadamsoftware wrote:
> > baloghadamsoftware wrote:
> > > baloghadamsoftware wrote:
> > > > baloghadamsoftware wrote:
> > > > > NoQ wrote:
> > > > > > baloghadamsoftware wrote:
> > > > > > > NoQ wrote:
> > > > > > > > Please instead re-use the code that computes the object under 
> > > > > > > > construction. That'll save you ~50 lines of code and will be 
> > > > > > > > more future-proof (eg., standalone temporaries without 
> > > > > > > > destructor technically have a construction context with 0 items 
> > > > > > > > so when we implement them correctly your procedure will stop 
> > > > > > > > working).
> > > > > > > That was so my first thought. However, 
> > > > > > > `handleConstructionContext()` is private and non-static. Now I 
> > > > > > > tried to merge the two methods: if the value is already in the 
> > > > > > > construction context, we return it, if not then we add it. Is 
> > > > > > > this what you suggest? Or did I misunderstand you? At the very 
> > > > > > > beginning I tried to simply use `handleConstructionContext()`, 
> > > > > > > but it asserted because the value was already in the map.
> > > > > > I'd like to preserve the assertion that objects-under-construction 
> > > > > > are never filled twice; it's a very useful sanity check. What you 
> > > > > > need in your checker is a function that computes 
> > > > > > object-under-construction but doesn't put it into the 
> > > > > > objects-under-construction map. So you have to separate the 
> > > > > > computation from filling in the state.
> > > > > OK, so I (fortunately) misundertood you. Thus I should refactor this 
> > > > > function to a calculation and a storing part?
> > > > OK, I see what you are speaking about, but I have no idea how to do it 
> > > > properly. The problem is that the control logic of filling in the state 
> > > > also depends on the kind of the construction context. For some kinds we 
> > > > do not fill at all. Every way I try it becomes more complex and less 
> > > > correct:
> > > > 
> > > > 1) `NewAllocatedObjectKind`: we do not add this to the state, we only 
> > > > retrieve the original.
> > > > 2) `SimpleReturnedValueKind` and `CXX17ElidedCopyReturnedValueKind`: 
> > > > depending on whether we are in top frame we handle this case 
> > > > recursively or we do not fill at all, just return the value. What is 
> > > > the construction context item here? Maybe the `ReturnStmt`?
> > > > 3) `ElidedTemporaryObjectKind`: this is the most problematic: we first 
> > > > handle it recursively for the construction context after elision, then 
> > > > we also fill it for the elided temporary object construction context as 
> > > > well.
> > > > 
> > > > The only thing I can (maybe) do is to retrieve the construction context 
> > > > item. But then the switch is still duplicated for filling, because of 
> > > > the different control logic for different construction context kinds.
> > > > The only thing I can (maybe) do is to retrieve the construction context 
> > > > item.
> > > 
> > > This does not help even for retrieving the value, because its control 
> > > logic also depends on the construction context kind. If I do it, it will 
> > > be code triplication instead of duplication and results in a code that is 
> > > worse to understand than the current one.
> > > 
> > > 
> > > Please instead re-use the code that computes the object under 
> > > construction. That'll save you ~50 lines of code and will be more 
> > > future-proof (eg., standalone temporaries without destructor technically 
> > > have a construction context with 0 items so when we implement them 
> > > correctly your procedure will stop working).
> > 
> > Any solution I can come up with rather adds 100 to 150 lines to the code 
> > instead of saving 50. For retrieving we only have to determine the 
> > construction context item (the key). For storing we also have to calculate 
> > the value. It sounds good to calculate these things in separate functions 
> > and then have a simple retriever and store function but there are lots of 
> > recursions, double strores, non-stores, retrieving in the store function 
> > that make it too complicated.
> > 
> > Also retrieving is more complex than just determining the item and getting 
> > the value from the context: if you look at `SimpleReturnedValueKind` and 
> > `CXX17ElidedCopyReturnedValueKind` you can see that we do not use the 
> > construction context we got in the parameter (the construction context of 
> > the return value) but the construction context of the call instead. For 
> > `ElidedTemporaryObjectKind` we take the construction context before the 
> > elusion.
> > 
> > Future proofness: I agree, this is a problem to solve. In 

[PATCH] D80627: [clang-format] Create a python documentation tool to generate a summary of the clang-format status for the whole of the LLVM project

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

I'm adding a summary table which shows LLVM only 44% of all LLVM cpp/h files 
are clang-formatted. ;-(

F12020299: image.png 


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

https://reviews.llvm.org/D80627



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

[PATCH] D79434: [analyzer] Generalize bitwise AND rules for ranges

[Artem Dergachev via Phabricator via cfe-commits]

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

Looks correct, thanks!!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79434



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

[PATCH] D80117: [analyzer] Introduce reasoning about symbolic remainder operator

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

In D80117#2059567 , @NoQ wrote:

> > I believe that as of now we can submit these modifications as is and 
> > explore performance optimizations later if needed.
>
> I still encourage you to explore the tests we have from our previous attempts 
> to simplify expressions recursively without memoization 
> (`test/Analysis/hangs.c`). I'm asking because these aren't all that 
> artificial: this kind of code was previously reported by a frustrated user as 
> "the analyzer started hanging on my code". Like, please replace a bunch of 
> `+`es with `&`/`|`/`%` and see if this causes your code to perform 
> exponentially over the size of the program. If so, i'd rather have us hurry 
> up and implement memoization.


Ok, looks like my memories on this subject are heavily messed up. The actual 
problem that made us hang was solved by D47155 
. This is a dumb bug that would have been 
avoided if we had memoization but it doesn't require memoization to be avoided 
and it doesn't look like this code risks repeating that mistake.

Then, our experience with memoization in D47402 
 wasn't as good as i expected; it turned out 
that there are other exponential parts of the analysis in such cases that we 
still couldn't avoid. We should probably still do it (given how difficult it is 
now to identify these "other parts" that are exponential, i'd rather not add 
more such parts consciously) but i guess it's not that much of a blocker.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80117



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

[libclc] cf4d4e3 - libclc: Compile with -nostdlib

[Matt Arsenault via cfe-commits]

Author: Matt Arsenault
Date: 2020-05-28T10:41:31-04:00
New Revision: cf4d4e366a2165f0e93948f166d76ae650aecc98

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

LOG: libclc: Compile with -nostdlib

This fixes a build error when compiling for amdgcn-amd-amdhsa, which
defaults to trying to link bitcode libraries.

Added: 


Modified: 
libclc/CMakeLists.txt

Removed: 




diff  --git a/libclc/CMakeLists.txt b/libclc/CMakeLists.txt
index 7b981110f6fd..9472f191fbde 100644
--- a/libclc/CMakeLists.txt
+++ b/libclc/CMakeLists.txt
@@ -262,7 +262,7 @@ foreach( t ${LIBCLC_TARGETS_TO_BUILD} )
target_compile_definitions( builtins.link.${arch_suffix} PRIVATE
"__CLC_INTERNAL" )
target_compile_options( builtins.link.${arch_suffix} PRIVATE  
-target
-   ${t} ${mcpu} -fno-builtin )
+   ${t} ${mcpu} -fno-builtin -nostdlib )
set_target_properties( builtins.link.${arch_suffix} PROPERTIES
LINKER_LANGUAGE CLC )
 



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

Re: [PATCH] D78454: [clangd] Highlight related control flow.

[Sam McCall via cfe-commits]

On Thu, May 28, 2020 at 4:35 PM Russell Gallop via Phabricator <
revi...@reviews.llvm.org> wrote:

> russell.gallop added a comment.
>
> Hi Sam,
>
> It looks like this is causing a failure on the Windows PS4 buildbot:
> http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast/builds/32606
>
> Please could you take a look? PS4 target disables RTTI, hence exceptions,
> by default so it is probably related to that.
>
Thanks!
Hmm, the clangd tests are supposed to be disabled on PS4 for exactly these
sorts of reasons. It looks like that's only working for shell tests, not
gtests though.
Lit config is a tangled mess... is there an #ifdef we can use to disable
just that test on PS4 until I get this sorted out?


>
> Thanks
> Russ
>
>
> Repository:
>   rG LLVM Github Monorepo
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D78454/new/
>
> https://reviews.llvm.org/D78454
>
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D74387: [OpenMP][SYCL] Improve diagnosing of unsupported types usage

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon updated this revision to Diff 266877.
Fznamznon marked 2 inline comments as done.
Fznamznon added a comment.

Applied comments from Johannes.
Fixed failing tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74387

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/CMakeLists.txt
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Sema/SemaSYCL.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/Headers/nvptx_device_math_sin.c
  clang/test/Headers/nvptx_device_math_sin.cpp
  clang/test/OpenMP/nvptx_unsupported_type_codegen.cpp
  clang/test/OpenMP/nvptx_unsupported_type_messages.cpp
  clang/test/SemaSYCL/float128.cpp

Index: clang/test/SemaSYCL/float128.cpp
===
--- /dev/null
+++ clang/test/SemaSYCL/float128.cpp
@@ -0,0 +1,96 @@
+// RUN: %clang_cc1 -triple spir64 -fsycl -fsycl-is-device -verify -fsyntax-only %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsycl -fsycl-is-device -fsyntax-only %s
+
+typedef __float128 BIGTY;
+
+template 
+class Z {
+public:
+  // expected-note@+1 {{'field' defined here}}
+  T field;
+  // expected-note@+1 2{{'field1' defined here}}
+  __float128 field1;
+  using BIGTYPE = __float128;
+  // expected-note@+1 {{'bigfield' defined here}}
+  BIGTYPE bigfield;
+};
+
+void host_ok(void) {
+  __float128 A;
+  int B = sizeof(__float128);
+  Z<__float128> C;
+  C.field1 = A;
+}
+
+void usage() {
+  // expected-note@+1 3{{'A' defined here}}
+  __float128 A;
+  Z<__float128> C;
+  // expected-error@+2 {{'A' requires 128 bit size '__float128' type support, but device 'spir64' does not support it}}
+  // expected-error@+1 {{'field1' requires 128 bit size '__float128' type support, but device 'spir64' does not support it}}
+  C.field1 = A;
+  // expected-error@+1 {{'bigfield' requires 128 bit size 'Z::BIGTYPE' (aka '__float128') type support, but device 'spir64' does not support it}}
+  C.bigfield += 1.0;
+
+  // expected-error@+1 {{'A' requires 128 bit size '__float128' type support, but device 'spir64' does not support it}}
+  auto foo1 = [=]() {
+__float128 AA;
+// expected-note@+2 {{'BB' defined here}}
+// expected-error@+1 {{'A' requires 128 bit size '__float128' type support, but device 'spir64' does not support it}}
+auto BB = A;
+// expected-error@+1 {{'BB' requires 128 bit size '__float128' type support, but device 'spir64' does not support it}}
+BB += 1;
+  };
+
+  // expected-note@+1 {{called by 'usage'}}
+  foo1();
+}
+
+template 
+void foo2(){};
+
+// expected-note@+3 {{'P' defined here}}
+// expected-error@+2 {{'P' requires 128 bit size '__float128' type support, but device 'spir64' does not support it}}
+// expected-note@+1 2{{'foo' defined here}}
+__float128 foo(__float128 P) { return P; }
+
+template 
+__attribute__((sycl_kernel)) void kernel(Func kernelFunc) {
+  // expected-note@+1 5{{called by 'kernel}}
+  kernelFunc();
+}
+
+int main() {
+  // expected-note@+1 {{'CapturedToDevice' defined here}}
+  __float128 CapturedToDevice = 1;
+  host_ok();
+  kernel([=]() {
+decltype(CapturedToDevice) D;
+// expected-error@+1 {{'CapturedToDevice' requires 128 bit size '__float128' type support, but device 'spir64' does not support it}}
+auto C = CapturedToDevice;
+Z<__float128> S;
+// expected-error@+1 {{'field1' requires 128 bit size '__float128' type support, but device 'spir64' does not support it}}
+S.field1 += 1;
+// expected-error@+1 {{'field' requires 128 bit size '__float128' type support, but device 'spir64' does not support it}}
+S.field = 1;
+  });
+
+  kernel([=]() {
+// expected-note@+1 2{{called by 'operator()'}}
+usage();
+// expected-note@+1 {{'' defined here}}
+BIGTY ;
+// expected-note@+3 {{called by 'operator()'}}
+// expected-error@+2 2{{'foo' requires 128 bit size '__float128' type support, but device 'spir64' does not support it}}
+// expected-error@+1 {{'' requires 128 bit size 'BIGTY' (aka '__float128') type support, but device 'spir64' does not support it}}
+auto A = foo();
+  });
+
+  kernel([=]() {
+Z<__float128> S;
+foo2<__float128>();
+auto A = sizeof(CapturedToDevice);
+  });
+
+  return 0;
+}
Index: clang/test/OpenMP/nvptx_unsupported_type_messages.cpp
===
--- clang/test/OpenMP/nvptx_unsupported_type_messages.cpp
+++ clang/test/OpenMP/nvptx_unsupported_type_messages.cpp
@@ -7,18 +7,23 @@
 struct T {
   char a;
 #ifndef _ARCH_PPC
+  // expected-note@+1 {{'f' defined here}}
   __float128 f;
 #else
+  // expected-note@+1 {{'f' defined here}}
   long double f;
 #endif
   char c;
   T() : a(12), f(15) {}
 #ifndef _ARCH_PPC
-// expected-error@+4 {{host requires

[PATCH] D74387: [OpenMP][SYCL] Improve diagnosing of unsupported types usage

[Mariya Podchishchaeva via Phabricator via cfe-commits]

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



Comment at: clang/lib/Sema/Sema.cpp:1727
+
+  QualType Ty = D->getType();
+  auto CheckType = [&](QualType Ty) {

jdoerfert wrote:
> Nit: Move below `CheckType` to avoid shadowing and confusion with the arg 
> there. 
Done, thanks



Comment at: clang/test/OpenMP/nvptx_unsupported_type_codegen.cpp:21
   char c;
   T() : a(12), f(15) {}
   T &operator+(T &b) { f += b.a; return *this;}

jdoerfert wrote:
> Why is this not diagnosed? I mean we cannot assign 15 on the device, can we? 
> Or does it work because it is a constant (and we basically just memcpy 
> something)?
> 
> If it's the latter, do we have a test in the negative version that makes sure 
> `T(int i) : a(i), f(i) {}` is flagged?
Unfortunately, nor this case neither `T(int i) : a(i), f(i) {}` is not 
diagnosed. This happens because `DiagnoseUseOfDecl` call seems missing for 
member initializers, not because there is memcpy. So, for example, such case is 
diagnosed:
```
struct B {
  __float128 a;
};
#pragma omp declare target
void foo() {
  B var = {1}; // error: 'a' requires 128 bit size '__float128' type support, 
but device 'nvptx64-unknown-unknown' does not support it
}
```
`DiagnoseUseOfDecl` function is called in so many cases and I guess it is meant 
to be called on each usage of each declaration, that is why I think the correct 
fix is add call to `DiagnoseUseOfDecl` somewhere near building of member 
initializers . This change even doesn't break my local `check-clang` LIT tests 
run, but I'm not really sure that such change is in scope of this patch, 
because `DiagnoseUseOfDecl` contains a lot of other diagnostics as well.



Comment at: clang/test/OpenMP/nvptx_unsupported_type_codegen.cpp:81
-// CHECK: define weak void 
@__omp_offloading_{{.+}}foo{{.+}}_l75([[BIGTYPE:.+]]*
-// CHECK: store [[BIGTYPE]] 
{{0xL3FFF|0xM3FF0}}, 
[[BIGTYPE]]* %

jdoerfert wrote:
> Just checking, we verify in the other test this would result in an error, 
> right?
Yes, I added such test case in `nvptx_unsupported_type_messages.cpp` .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74387



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

[PATCH] D77658: [analyzer] StdLibraryFunctionsChecker: Add sanity checks for constraints

[Gabor Marton via Phabricator via cfe-commits]

martong updated this revision to Diff 266879.
martong marked an inline comment as done.
martong added a comment.

- Rebase to master


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77658

Files:
  clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp

Index: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -66,7 +66,7 @@
   /// Below is a series of typedefs necessary to define function specs.
   /// We avoid nesting types here because each additional qualifier
   /// would need to be repeated in every function spec.
-  struct Summary;
+  class Summary;
 
   /// Specify how much the analyzer engine should entrust modeling this function
   /// to us. If he doesn't, he performs additional invalidations.
@@ -112,10 +112,23 @@
 virtual ValueConstraintPtr negate() const {
   llvm_unreachable("Not implemented");
 };
+
+/// Do sanity check on the constraint.
+bool validate(const FunctionDecl *FD) const {
+  const bool ValidArg = ArgN == Ret || ArgN < FD->getNumParams();
+  assert(ValidArg && "Arg out of range!");
+  if (!ValidArg)
+return false;
+  // Subclasses may further refine the validation.
+  return sanityCheck(FD);
+}
 ArgNo getArgNo() const { return ArgN; }
 
   protected:
 ArgNo ArgN; // Argument to which we apply the constraint.
+
+/// Do polymorphic sanity check on the constraint.
+virtual bool sanityCheck(const FunctionDecl *FD) const { return true; }
   };
 
   /// Given a range, should the argument stay inside or outside this range?
@@ -165,6 +178,14 @@
   }
   return std::make_shared(Tmp);
 }
+
+bool sanityCheck(const FunctionDecl *FD) const override {
+  const bool ValidArg =
+  getArgType(FD, ArgN)->isIntegralType(FD->getASTContext());
+  assert(ValidArg &&
+ "This constraint should be applied on an integral type");
+  return ValidArg;
+}
   };
 
   class ComparisonConstraint : public ValueConstraint {
@@ -205,12 +226,61 @@
   Tmp.CannotBeNull = !this->CannotBeNull;
   return std::make_shared(Tmp);
 }
+
+bool sanityCheck(const FunctionDecl *FD) const override {
+  const bool ValidArg = getArgType(FD, ArgN)->isPointerType();
+  assert(ValidArg &&
+ "This constraint should be applied only on a pointer type");
+  return ValidArg;
+}
   };
 
   /// The complete list of constraints that defines a single branch.
   typedef std::vector ConstraintSet;
 
   using ArgTypes = std::vector;
+
+  // A placeholder type, we use it whenever we do not care about the concrete
+  // type in a Signature.
+  const QualType Irrelevant{};
+  bool static isIrrelevant(QualType T) { return T.isNull(); }
+
+  // The signature of a function we want to describe with a summary. This is a
+  // concessive signature, meaning there may be irrelevant types in the
+  // signature which we do not check against a function with concrete types.
+  struct Signature {
+const ArgTypes ArgTys;
+const QualType RetTy;
+Signature(ArgTypes ArgTys, QualType RetTy) : ArgTys(ArgTys), RetTy(RetTy) {
+  assertRetTypeSuitableForSignature(RetTy);
+  for (size_t I = 0, E = ArgTys.size(); I != E; ++I) {
+QualType ArgTy = ArgTys[I];
+assertArgTypeSuitableForSignature(ArgTy);
+  }
+}
+bool matches(const FunctionDecl *FD) const;
+
+  private:
+static void assertArgTypeSuitableForSignature(QualType T) {
+  assert((T.isNull() || !T->isVoidType()) &&
+ "We should have no void types in the spec");
+  assert((T.isNull() || T.isCanonical()) &&
+ "We should only have canonical types in the spec");
+}
+static void assertRetTypeSuitableForSignature(QualType T) {
+  assert((T.isNull() || T.isCanonical()) &&
+ "We should only have canonical types in the spec");
+}
+  };
+
+  static QualType getArgType(const FunctionDecl *FD, ArgNo ArgN) {
+assert(FD && "Function must be set");
+QualType T = (ArgN == Ret)
+ ? FD->getReturnType().getCanonicalType()
+ : FD->getParamDecl(ArgN)->getType().getCanonicalType();
+return T;
+  }
+
   using Cases = std::vector;
 
   /// Includes information about
@@ -232,15 +302,19 @@
   ///   * a list of argument constraints, that must be true on every branch.
   /// If these constraints are not satisfied that means a fatal error
   /// usually resulting in undefined behaviour.
-  struct Summary {
-const ArgTypes ArgTys;
-const QualType RetTy;
+  class Summary {
+const Signature Sign;
 const InvalidationKind InvalidationKd;
 Cases CaseConstraints;
 ConstraintSet ArgConstrain

[PATCH] D69764: [clang-format] Add East/West Const fixer capability

[MyDeveloperDay via Phabricator via cfe-commits]

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

In D69764#2058801 , @curdeius wrote:

> One last thought, how about making the config a bit more future-proof and 
> instead of `ConstPlacement` accept what was discussed some time ago:
>
>   QualifierStyle:
>  -   const: Right
>
>
> and restrict it to `const` for the moment?
>  Maybe renaming to `QualifierPlacement` or something more appropriate.


I'm already feeling there needs to be more fine grained control here in the 
config... (even if that is a list of MACRO types to ignore) or 
IgnoreCapitializedIdentifiers, or WarnButDontFix or as we talked about before 
handling more than just the placement of const, I feel you are correct I'll end 
up polluting the toplevel config namespace unless I switch to some form of 
structure in the YAML like we do with BraceWrapping.




Comment at: clang/lib/Format/EastWestConstFixer.cpp:157-158
+  IsCVQualifierOrType(Tok->Next->Next)) {
+// The unsigned/signed case  `const unsigned int` -> `unsigned int
+// const`
+swapFirstThreeTokens(SourceMgr, Fixes, Tok, Tok->Next, Tok->Next->Next,

MyDeveloperDay wrote:
> aaron.ballman wrote:
> > What about something like `const unsigned long long` or the 
> > less-common-but-equally-valid `long const long unsigned`? Or multiple 
> > qualifiers like `unsigned volatile long const long * restrict`
> I would like to cover as many cases as possible, but a small part of me 
> thinks that at least initially if we skip a case or two like this I'd be fine.
> 
> But I'll take a look and see what I think we could add easily in terms of 
> multiple simple types in a row.
@aaron.ballman  if you saw

`long const long unsigned` what would you expect it to become in both east and 
west const cases? 

my assumption is:

 - east : `long long unsigned const`
 - west: `const long long unsigned`

Or something else?

same for

`unsigned volatile long const long * restrict` I assume:

 - east : `unsigned volatile long long const * restrict`
 - west: ` const unsigned volatile long long* restrict`

Its it would help if I understood the typical expectation in these situations?





Comment at: clang/lib/Format/EastWestConstFixer.cpp:195
+FormatToken *Tok) {
+  // We only need to think about streams that begin with const.
+  if (!Tok->is(tok::kw_const)) {

curdeius wrote:
> Why? What about `unsigned const int`?
@curdeius would you help me understand your expectation here?

 - east: `unsigned int const`
 - west: `const unsigned int`

?



Comment at: clang/lib/Format/EastWestConstFixer.cpp:293
+  Tok->Next->Next->Next && Tok->Next->Next->Next->is(tok::kw_const)) {
+swapFourTokens(SourceMgr, Fixes, Tok, Tok->Next, Tok->Next, 
Tok->Next->Next->Next,
+ /*West=*/true);

rsmith wrote:
> There can be more than four type-specifiers / cv-qualifiers in a row. Eg: 
> `unsigned long long int volatile const` -> `const volatile unsigned long long 
> int`.
you have the volatile moving too? if you had the choice would it become:

- `const unsigned long long int volatile`
- `const volatile unsigned long long int`
- `volatile const unsigned long long int`

Any reason why? or is that personal taste? what would be the ideal?




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

https://reviews.llvm.org/D69764



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

[PATCH] D80574: [ExtVector] Support ExtVectorType conditional operator

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.



Comment at: clang/docs/LanguageExtensions.rst:481
 =yes yes   yes yes
-:? [#]_  yes --yes --
+?:   yes --yes --
 sizeof   yes yes   yes yes

I think it was more clear as a footnote though.



Comment at: clang/docs/LanguageExtensions.rst:489
 
+Note that ternary operator(?:) has different behaviors depends on the vector 
type of
+the condition operands. If the condition is a GNU vector (i.e. 
__vector_size__),

has different behaviors depends on -> has different behaviors depending on



Comment at: clang/docs/LanguageExtensions.rst:493
+If it's an extension (OpenCL) vector, it's only available in C and OpenCL C. 
And it 
+selects base on signedness of the condition operands.
+

Maybe we can add a reference to OpenCL C section 6.3.9



Comment at: clang/lib/Sema/SemaExpr.cpp:7484
+  // Only ext vector is allowed
+  if (const auto *VecCondTy = Cond->getType()->getAs()) {
+QualType EleTy = VecCondTy->getElementType();

Why do you need this change? I believe OpenCL makes the same restriction.



Comment at: clang/lib/Sema/SemaExpr.cpp:7975
+// condition and result type.
+QualType CondTy = Cond.get()->getType();
+if (CondTy->isExtVectorType()) {

Do you know where it is done for OpenCL? I think we should try to share the 
same code...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80574



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