[PATCH] D124349: [analyzer] Get direct binding for specific punned case

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

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

Looks good.
Wait for an other accept.




Comment at: clang/test/Analysis/array-struct-region.c:362-366
+int array_struct_bitfield_1() {
+  BITFIELD_CAST ff = {0};
+  BITFIELD_CAST *pff = &ff;
+  return *((int *)pff + 1);
+}

```lang=c
void array_struct_bitfield_1() {
  BITFIELD_CAST ff = {0};
  BITFIELD_CAST *pff = &ff;
  clang_analyzer_eval(*((int *)pff + 1) == 0); // expected-warning{{TRUE}}
  ff.b[0] = 3;
  clang_analyzer_eval(*((int *)pff + 1) == 3); // expected-warning{{TRUE}}
}
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124349

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

[PATCH] D124356: [Driver][Solaris] -r: imply -nostdlib like GCC

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/test/Driver/solaris-ld.c:115
+// RUN:   2>&1 | FileCheck %s --check-prefix=CHECK-RELOCATABLE
+// CHECK-RELOCATABLE: "-r"
+// CHECK-RELOCATABLE-NOT: "-l

It's worth testing that there are some `"-L`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124356

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

[PATCH] D115187: [clangd] Expose CoawaitExpr's operand in the AST

[Nathan Ridge via Phabricator via cfe-commits]

nridge added a comment.

Update on this: I'm working on getting the failing tests 
 to pass. Some of them just 
need expected AST dumps updated to reflect the new node, but some others 
suggest the patch actually causes semantic regressions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115187

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

[PATCH] D124359: [clangd] Add inlay hints for mutable reference parameters

[Tobias Ribizel via Phabricator via cfe-commits]

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

Add a & or && annotation to all parameter inlay hints that refer to a non-const 
reference. That makes it easier to identify them even if semantic highlighting 
is not used (where this is already available)


https://reviews.llvm.org/D124359

Files:
  clang-tools-extra/clangd/InlayHints.cpp


Index: clang-tools-extra/clangd/InlayHints.cpp
===
--- clang-tools-extra/clangd/InlayHints.cpp
+++ clang-tools-extra/clangd/InlayHints.cpp
@@ -10,6 +10,7 @@
 #include "Config.h"
 #include "HeuristicResolver.h"
 #include "ParsedAST.h"
+#include "clang/AST/Decl.h"
 #include "clang/AST/DeclarationName.h"
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/RecursiveASTVisitor.h"
@@ -392,6 +393,7 @@
 // Don't show hints for variadic parameters.
 size_t FixedParamCount = getFixedParamCount(Callee);
 size_t ArgCount = std::min(FixedParamCount, Args.size());
+auto Params = Callee->parameters();
 
 NameVec ParameterNames = chooseParameterNames(Callee, ArgCount);
 
@@ -402,12 +404,18 @@
 
 for (size_t I = 0; I < ArgCount; ++I) {
   StringRef Name = ParameterNames[I];
-  if (!shouldHint(Args[I], Name))
-continue;
+  bool NameHint = shouldNameHint(Args[I], Name);
+  std::string Suffix = ": ";
+  if (!NameHint) {
+Name = "";
+Suffix = "";
+  }
+  Suffix += getRefSuffix(Params[I]);
 
-  addInlayHint(Args[I]->getSourceRange(), HintSide::Left,
-   InlayHintKind::ParameterHint, /*Prefix=*/"", Name,
-   /*Suffix=*/": ");
+  if (!Name.empty() || !Suffix.empty()) {
+addInlayHint(Args[I]->getSourceRange(), HintSide::Left,
+ InlayHintKind::ParameterHint, /*Prefix=*/"", Name, 
Suffix);
+  }
 }
   }
 
@@ -434,12 +442,21 @@
 return WhatItIsSetting.equals_insensitive(ParamNames[0]);
   }
 
-  bool shouldHint(const Expr *Arg, StringRef ParamName) {
+  StringRef getRefSuffix(const ParmVarDecl *Param) {
+// If the parameter is a non-const reference type, print an inlay hint
+auto Type = Param->getType();
+return Type->isReferenceType() &&
+   !Type.getNonReferenceType().isConstQualified()
+   ? (Type->isLValueReferenceType() ? "&" : "&&")
+   : "";
+  }
+
+  bool shouldNameHint(const Expr *Arg, StringRef ParamName) {
 if (ParamName.empty())
   return false;
 
 // If the argument expression is a single name and it matches the
-// parameter name exactly, omit the hint.
+// parameter name exactly, omit the name hint.
 if (ParamName == getSpelledIdentifier(Arg))
   return false;
 


Index: clang-tools-extra/clangd/InlayHints.cpp
===
--- clang-tools-extra/clangd/InlayHints.cpp
+++ clang-tools-extra/clangd/InlayHints.cpp
@@ -10,6 +10,7 @@
 #include "Config.h"
 #include "HeuristicResolver.h"
 #include "ParsedAST.h"
+#include "clang/AST/Decl.h"
 #include "clang/AST/DeclarationName.h"
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/RecursiveASTVisitor.h"
@@ -392,6 +393,7 @@
 // Don't show hints for variadic parameters.
 size_t FixedParamCount = getFixedParamCount(Callee);
 size_t ArgCount = std::min(FixedParamCount, Args.size());
+auto Params = Callee->parameters();
 
 NameVec ParameterNames = chooseParameterNames(Callee, ArgCount);
 
@@ -402,12 +404,18 @@
 
 for (size_t I = 0; I < ArgCount; ++I) {
   StringRef Name = ParameterNames[I];
-  if (!shouldHint(Args[I], Name))
-continue;
+  bool NameHint = shouldNameHint(Args[I], Name);
+  std::string Suffix = ": ";
+  if (!NameHint) {
+Name = "";
+Suffix = "";
+  }
+  Suffix += getRefSuffix(Params[I]);
 
-  addInlayHint(Args[I]->getSourceRange(), HintSide::Left,
-   InlayHintKind::ParameterHint, /*Prefix=*/"", Name,
-   /*Suffix=*/": ");
+  if (!Name.empty() || !Suffix.empty()) {
+addInlayHint(Args[I]->getSourceRange(), HintSide::Left,
+ InlayHintKind::ParameterHint, /*Prefix=*/"", Name, Suffix);
+  }
 }
   }
 
@@ -434,12 +442,21 @@
 return WhatItIsSetting.equals_insensitive(ParamNames[0]);
   }
 
-  bool shouldHint(const Expr *Arg, StringRef ParamName) {
+  StringRef getRefSuffix(const ParmVarDecl *Param) {
+// If the parameter is a non-const reference type, print an inlay hint
+auto Type = Param->getType();
+return Type->isReferenceType() &&
+   !Type.getNonReferenceType().isConstQualified()
+   ? (Type->isLValueReferenceType() ? "&" : "&&")
+   : "";
+  

[PATCH] D124344: [clangd] Output inlay hints with `clangd --check`

[Nathan Ridge via Phabricator via cfe-commits]

nridge added inline comments.



Comment at: clang-tools-extra/clangd/tool/Check.cpp:201
+for (const auto &Hint : Hints) {
+  vlog("  {0} {1}", Hint.position, Hint.label);
+}

upsj wrote:
> nridge wrote:
> > Might be useful for print the hint kind as well?
> right, is the current solution (adding a public toString) okay?
Looks reasonable to me.


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

https://reviews.llvm.org/D124344

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

[PATCH] D115187: [clangd] Expose CoawaitExpr's operand in the AST

[Nathan Ridge via Phabricator via cfe-commits]

nridge added a comment.

Here is a reduced testcase for the semantic regression:

  namespace std {
  template 
  struct coroutine_handle {
static coroutine_handle from_address(void *) noexcept;
  };
  template 
  struct coroutine_traits;
  } // end of namespace std
  
  template  struct coro {};
  template 
  struct std::coroutine_traits, Ps...> {
using promise_type = Promise;
  };
  
  struct awaitable {
bool await_ready() noexcept;
template 
void await_suspend(F) noexcept;
void await_resume() noexcept;
  } a;
  
  struct transform_awaitable {};
  
  struct transform_promise {
coro get_return_object();
awaitable initial_suspend();
awaitable final_suspend() noexcept;
awaitable await_transform(transform_awaitable);
void unhandled_exception();
void return_void();
  };
  
  template 
  coro await_template(U t) {
co_await t;
  }
  
  template coro 
await_template(transform_awaitable);

Without my patch, this compiles fine. With it, I clang gives the following 
errors:

  coroutines.cpp:35:9: error: no viable conversion from 'awaitable' to 
'transform_awaitable'
  coro await_template(U t) {
  ^~
  coroutines.cpp:39:34: note: in instantiation of function template 
specialization 'await_template' 
requested here
  template coro 
await_template(transform_awaitable);
   ^
  coroutines.cpp:23:8: note: candidate constructor (the implicit copy 
constructor) not viable: no known conversion from 'awaitable' to 'const 
transform_awaitable &' for 1st argument
  struct transform_awaitable {};
 ^
  coroutines.cpp:23:8: note: candidate constructor (the implicit move 
constructor) not viable: no known conversion from 'awaitable' to 
'transform_awaitable &&' for 1st argument
  struct transform_awaitable {};
 ^
  coroutines.cpp:29:48: note: passing argument to parameter here
awaitable await_transform(transform_awaitable);
 ^
  coroutines.cpp:35:9: note: call to 'await_transform' implicitly required by 
'co_await' here
  coro await_template(U t) {
  ^~
  1 error generated.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115187

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

[PATCH] D124356: [Driver][Solaris] -r: imply -nostdlib like GCC

[Brad Smith via Phabricator via cfe-commits]

brad updated this revision to Diff 424835.
brad added a comment.

Update test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124356

Files:
  clang/lib/Driver/ToolChains/Solaris.cpp
  clang/test/Driver/solaris-ld.c


Index: clang/test/Driver/solaris-ld.c
===
--- clang/test/Driver/solaris-ld.c
+++ clang/test/Driver/solaris-ld.c
@@ -108,3 +108,13 @@
 // CHECK-SPARC32-SHARED-SAME: "-lc"
 // CHECK-SPARC32-SHARED-NOT: "-lgcc"
 // CHECK-SPARC32-SHARED-NOT: "-lm"
+
+// -r suppresses default -l and crt*.o, values-*.o like -nostdlib.
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o \
+// RUN: --target=sparc-sun-solaris2.11 -r 2>&1 \
+// RUN:   | FileCheck %s --check-prefix=CHECK-RELOCATABLE
+// CHECK-RELOCATABLE: "-L
+// CHECK-RELOCATABLE: "-r"
+// CHECK-RELOCATABLE-NOT: "-l
+// CHECK-RELOCATABLE-NOT: {{.*}}crt{{[^.]+}}.o
+// CHECK-RELOCATABLE-NOT: {{.*}}values-{{[^.]+}}.o
Index: clang/lib/Driver/ToolChains/Solaris.cpp
===
--- clang/lib/Driver/ToolChains/Solaris.cpp
+++ clang/lib/Driver/ToolChains/Solaris.cpp
@@ -84,7 +84,8 @@
 assert(Output.isNothing() && "Invalid output.");
   }
 
-  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles)) {
+  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles,
+   options::OPT_r)) {
 if (!Args.hasArg(options::OPT_shared))
   CmdArgs.push_back(
   Args.MakeArgString(getToolChain().GetFilePath("crt1.o")));
@@ -124,7 +125,8 @@
   bool NeedsSanitizerDeps = addSanitizerRuntimes(getToolChain(), Args, 
CmdArgs);
   AddLinkerInputs(getToolChain(), Inputs, Args, CmdArgs, JA);
 
-  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs)) {
+  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs,
+   options::OPT_r)) {
 if (getToolChain().ShouldLinkCXXStdlib(Args))
   getToolChain().AddCXXStdlibLibArgs(Args, CmdArgs);
 if (Args.hasArg(options::OPT_fstack_protector) ||
@@ -161,11 +163,13 @@
 }
   }
 
-  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles)) {
+  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles,
+   options::OPT_r)) {
 CmdArgs.push_back(
 Args.MakeArgString(getToolChain().GetFilePath("crtend.o")));
+CmdArgs.push_back(
+Args.MakeArgString(getToolChain().GetFilePath("crtn.o")));
   }
-  CmdArgs.push_back(Args.MakeArgString(getToolChain().GetFilePath("crtn.o")));
 
   getToolChain().addProfileRTLibs(Args, CmdArgs);
 


Index: clang/test/Driver/solaris-ld.c
===
--- clang/test/Driver/solaris-ld.c
+++ clang/test/Driver/solaris-ld.c
@@ -108,3 +108,13 @@
 // CHECK-SPARC32-SHARED-SAME: "-lc"
 // CHECK-SPARC32-SHARED-NOT: "-lgcc"
 // CHECK-SPARC32-SHARED-NOT: "-lm"
+
+// -r suppresses default -l and crt*.o, values-*.o like -nostdlib.
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o \
+// RUN: --target=sparc-sun-solaris2.11 -r 2>&1 \
+// RUN:   | FileCheck %s --check-prefix=CHECK-RELOCATABLE
+// CHECK-RELOCATABLE: "-L
+// CHECK-RELOCATABLE: "-r"
+// CHECK-RELOCATABLE-NOT: "-l
+// CHECK-RELOCATABLE-NOT: {{.*}}crt{{[^.]+}}.o
+// CHECK-RELOCATABLE-NOT: {{.*}}values-{{[^.]+}}.o
Index: clang/lib/Driver/ToolChains/Solaris.cpp
===
--- clang/lib/Driver/ToolChains/Solaris.cpp
+++ clang/lib/Driver/ToolChains/Solaris.cpp
@@ -84,7 +84,8 @@
 assert(Output.isNothing() && "Invalid output.");
   }
 
-  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles)) {
+  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles,
+   options::OPT_r)) {
 if (!Args.hasArg(options::OPT_shared))
   CmdArgs.push_back(
   Args.MakeArgString(getToolChain().GetFilePath("crt1.o")));
@@ -124,7 +125,8 @@
   bool NeedsSanitizerDeps = addSanitizerRuntimes(getToolChain(), Args, CmdArgs);
   AddLinkerInputs(getToolChain(), Inputs, Args, CmdArgs, JA);
 
-  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs)) {
+  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs,
+   options::OPT_r)) {
 if (getToolChain().ShouldLinkCXXStdlib(Args))
   getToolChain().AddCXXStdlibLibArgs(Args, CmdArgs);
 if (Args.hasArg(options::OPT_fstack_protector) ||
@@ -161,11 +163,13 @@
 }
   }
 
-  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles)) {
+  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles,
+   options::OPT_r)) {
 CmdArgs.push_back(
 Args.MakeArgString(getToolChain().GetFilePath("crtend.o")));
+CmdArgs.push_back(
+Args.MakeArgString(getToolChain().GetFilePath("crtn.o")));
   }
-  CmdArgs.push_b

[PATCH] D124344: [clangd] Output inlay hints with `clangd --check`

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

can you please upload the patch with full context? see 
https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface




Comment at: clang-tools-extra/clangd/tool/Check.cpp:196
+  // Build Inlay Hints for the entire AST or the specified range
+  bool buildInlayHints(llvm::Optional LineRange) {
+log("Building inlay hints");

this function always returns true, let's make it void



Comment at: clang-tools-extra/clangd/tool/Check.cpp:222
 
-  if (!ShouldCheckLine(Pos))
+  if (LineRange && LineRange->contains(Pos))
 continue;

this should be `LineRange && !LineRange->contains`



Comment at: clang-tools-extra/clangd/tool/Check.cpp:296
   if (!C.buildCommand(TFS) || !C.buildInvocation(TFS, Contents) ||
-  !C.buildAST())
+  !C.buildAST() || !C.buildInlayHints(LineRange))
 return false;

failing inlay hints should not fail the rest, can you put it after 
`C.testLocationFatures` call below instead?



Comment at: clang-tools-extra/clangd/tool/Check.cpp:201
+for (const auto &Hint : Hints) {
+  vlog("  {0} {1}", Hint.position, Hint.label);
+}

nridge wrote:
> upsj wrote:
> > nridge wrote:
> > > Might be useful for print the hint kind as well?
> > right, is the current solution (adding a public toString) okay?
> Looks reasonable to me.
we usually override the `llvm::raw_ostream operator<<` instead, can you do that 
for `InlayHint` and log `Hint` directly here?


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

https://reviews.llvm.org/D124344

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

[PATCH] D124344: [clangd] Output inlay hints with `clangd --check`

[Tobias Ribizel via Phabricator via cfe-commits]

upsj updated this revision to Diff 424836.
upsj marked 2 inline comments as done.
upsj added a comment.

- format


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124344

Files:
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/Protocol.h
  clang-tools-extra/clangd/tool/Check.cpp
  clang-tools-extra/clangd/tool/ClangdMain.cpp

Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -61,8 +61,7 @@
 namespace clangd {
 
 // Implemented in Check.cpp.
-bool check(const llvm::StringRef File,
-   llvm::function_ref ShouldCheckLine,
+bool check(const llvm::StringRef File, llvm::Optional LineRange,
const ThreadsafeFS &TFS, const ClangdLSPServer::Options &Opts,
bool EnableCodeCompletion);
 
@@ -955,8 +954,9 @@
   return 1;
 }
 log("Entering check mode (no LSP server)");
-uint32_t Begin = 0, End = std::numeric_limits::max();
+llvm::Optional CheckLineRange;
 if (!CheckFileLines.empty()) {
+  uint32_t Begin = 0, End = std::numeric_limits::max();
   StringRef RangeStr(CheckFileLines);
   bool ParseError = RangeStr.consumeInteger(0, Begin);
   if (RangeStr.empty()) {
@@ -965,19 +965,18 @@
 ParseError |= !RangeStr.consume_front("-");
 ParseError |= RangeStr.consumeInteger(0, End);
   }
-  if (ParseError || !RangeStr.empty()) {
-elog("Invalid --check-line specified. Use Begin-End format, e.g. 3-17");
+  if (ParseError || !RangeStr.empty() || Begin <= 0 || End < Begin) {
+elog(
+"Invalid --check-lines specified. Use Begin-End format, e.g. 3-17");
 return 1;
   }
+  CheckLineRange = Range{Position{static_cast(Begin - 1), 0},
+ Position{static_cast(End), 0}};
 }
-auto ShouldCheckLine = [&](const Position &Pos) {
-  uint32_t Line = Pos.line + 1; // Position::line is 0-based.
-  return Line >= Begin && Line <= End;
-};
 // For now code completion is enabled any time the range is limited via
 // --check-lines. If it turns out to be to slow, we can introduce a
 // dedicated flag for that instead.
-return check(Path, ShouldCheckLine, TFS, Opts,
+return check(Path, CheckLineRange, TFS, Opts,
  /*EnableCodeCompletion=*/!CheckFileLines.empty())
? 0
: static_cast(ErrorResultCode::CheckFailed);
Index: clang-tools-extra/clangd/tool/Check.cpp
===
--- clang-tools-extra/clangd/tool/Check.cpp
+++ clang-tools-extra/clangd/tool/Check.cpp
@@ -30,8 +30,10 @@
 #include "Config.h"
 #include "GlobalCompilationDatabase.h"
 #include "Hover.h"
+#include "InlayHints.h"
 #include "ParsedAST.h"
 #include "Preamble.h"
+#include "Protocol.h"
 #include "SourceCode.h"
 #include "XRefs.h"
 #include "index/CanonicalIncludes.h"
@@ -190,10 +192,20 @@
 return true;
   }
 
+  // Build Inlay Hints for the entire AST or the specified range
+  bool buildInlayHints(llvm::Optional LineRange) {
+log("Building inlay hints");
+auto Hints = inlayHints(*AST, LineRange);
+
+for (const auto &Hint : Hints) {
+  vlog("  {0} {1} {2}", toString(Hint.kind), Hint.position, Hint.label);
+}
+return true;
+  }
+
   // Run AST-based features at each token in the file.
-  void testLocationFeatures(
-  llvm::function_ref ShouldCheckLine,
-  const bool EnableCodeCompletion) {
+  void testLocationFeatures(llvm::Optional LineRange,
+const bool EnableCodeCompletion) {
 trace::Span Trace("testLocationFeatures");
 log("Testing features at each token (may be slow in large files)");
 auto &SM = AST->getSourceManager();
@@ -207,7 +219,7 @@
   unsigned End = Start + Tok.length();
   Position Pos = offsetToPosition(Inputs.Contents, Start);
 
-  if (!ShouldCheckLine(Pos))
+  if (LineRange && LineRange->contains(Pos))
 continue;
 
   trace::Span Trace("Token");
@@ -254,8 +266,7 @@
 
 } // namespace
 
-bool check(llvm::StringRef File,
-   llvm::function_ref ShouldCheckLine,
+bool check(llvm::StringRef File, llvm::Optional LineRange,
const ThreadsafeFS &TFS, const ClangdLSPServer::Options &Opts,
bool EnableCodeCompletion) {
   llvm::SmallString<0> FakeFile;
@@ -282,9 +293,9 @@
   : /*Don't turn on local configs for an arbitrary temp path.*/ ""));
   Checker C(File, Opts);
   if (!C.buildCommand(TFS) || !C.buildInvocation(TFS, Contents) ||
-  !C.buildAST())
+  !C.buildAST() || !C.buildInlayHints(LineRange))
 return false;
-  C.testLocationFeatures(ShouldCheckLine, EnableCodeCompletion);
+  C.testLocationFeatures(LineRange, EnableCodeCompletion);
 
   log("All checks co

[PATCH] D124356: [Driver][Solaris] -r: imply -nostdlib like GCC

[Fangrui Song via Phabricator via cfe-commits]

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

LGTM.




Comment at: clang/lib/Driver/ToolChains/Solaris.cpp:168
   }
-  CmdArgs.push_back(Args.MakeArgString(getToolChain().GetFilePath("crtn.o")));
 

This appears to fix another bug.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124356

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

[PATCH] D124244: [analyzer] add StoreToImmutable and ModelConstQualifiedReturn checkers

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

steakhal added a comment.

Looks great. I only have nits mostly for the docs.




Comment at: clang/docs/analyzer/checkers.rst:2334-2336
+modified without causing undefined behavior. These functions
+include getenv(), setlocale(), localeconv(), asctime(), and
+strerror(). In such cases, the function call results must be

IMO the list of functions should be mentioned separately. What if the list gets 
longer and longer.
A separate paragraph could list these instead. Also, wrap them with double 
backticks.
```
``getenv()``
```



Comment at: clang/docs/analyzer/checkers.rst:2340
+Checker models return values of these functions as const
+qualified. Their modification is checked in StoreToImmutable
+core checker.

Use crossref links.



Comment at: clang/docs/analyzer/checkers.rst:2343-2351
+.. code-block:: c
+
+  void writing_to_envvar() {
+char *p = getenv("VAR"); // note: getenv return value
+ // should be treated as const
+if (!p)
+  return;

I don't like the fact that this example works only if the 
`alpha.cert.pos.StoreToImmutableChecker` is enabled, which is an //alpha// 
checker.

There are other places where such immutable memory regions arise, such as 
writable strings.
Anyway, at least let's mention that the given alpha checker has anything to do 
with this checker (+ crossref!).



Comment at: clang/docs/analyzer/checkers.rst:2352
+  }
+
 alpha.security.taint

You should also embed an example of how to fix this issue: E.g. advertises 
making a copy of the immutable region to a mutable location and doing the 
mutation there instead. And use `const` as a preventive measure for the future 
for the original pointer.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h:767-769
 assert(isa(sreg) || isa(sreg) ||
-   isa(sreg));
+   isa(sreg) ||
+   isa(sreg));

Please merge these into a single `isa()`.



Comment at: clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt:68-70
   MIGChecker.cpp
+  cert/ModelConstQualifiedReturnChecker.cpp
   MoveChecker.cpp

Put this in the right alphabetical place.



Comment at: clang/lib/StaticAnalyzer/Checkers/StoreToImmutableChecker.cpp:27-29
+  BugType ImmutableMemoryBind{this,
+  "Immutable memory should not be overwritten",
+  categories::MemoryError};

You could expose a pointer to this by creating a `const BugType *getBugType() 
const;` getter method, which could be used by other checkers.



Comment at: clang/lib/StaticAnalyzer/Checkers/StoreToImmutableChecker.cpp:51
+  auto Report = std::make_unique(
+  ImmutableMemoryBind, "modifying immutable memory", ErrorNode);
+  Report->markInteresting(R);

We capitalize the static analyzer warnings and notes.



Comment at: 
clang/lib/StaticAnalyzer/Checkers/cert/ModelConstQualifiedReturnChecker.cpp:1-2
+//== ModelConstQualifiedReturnChecker.cpp --- -*- C++
+//-*--=//
+//

Fix the comment.



Comment at: 
clang/lib/StaticAnalyzer/Checkers/cert/ModelConstQualifiedReturnChecker.cpp:10-11
+//
+// This file defines ModelConstQualifiedReturnChecker
+// Return values of certain functions should be treated as const-qualified
+//

Comments should be capitalized and punctuated as usual.



Comment at: 
clang/lib/StaticAnalyzer/Checkers/cert/ModelConstQualifiedReturnChecker.cpp:30-43
+class ModelConstQualifiedReturnChecker : public Checker {
+private:
+  void evalConstQualifiedReturnCall(const CallEvent &Call,
+CheckerContext &C) const;
+
+  // SEI CERT ENV30-C
+  const CallDescriptionSet ConstQualifiedReturnFunctions = {

I can see a small issue by `evaCall`-ing these functions.
The problem is that we might not model all aspects of these functions, thus the 
modeling can cause harm to the analysis. E.g. by not doing invalidation where 
we actually would need invalidation to kick in, or anything else really.
Consequently, another problem is that no other checker can evaluate these, 
since we evaluate them here.
Thus, fixing such improper modeling could end up in further changes down the 
line.
Unfortunately, I don't have any better ATM, so let's go with this `evalCall` 
approach.



Comment at: 
clang/lib/StaticAnalyzer/Checkers/cert/ModelConstQualifiedReturnChecker.cpp:66
+  [SymReg, FunName](PathSensitiveBugReport &BR, llvm::raw_ostream &Out) {
+if (BR.getBugType().getCheckerName() != "core.StoreToImmutable" ||
+!BR.isInteresting(SymReg))

One should achieve this by comparing the BugType pointers.



Comment 

[PATCH] D124356: [Driver][Solaris] -r: imply -nostdlib like GCC

[Brad Smith via Phabricator via cfe-commits]

brad added inline comments.



Comment at: clang/lib/Driver/ToolChains/Solaris.cpp:168
   }
-  CmdArgs.push_back(Args.MakeArgString(getToolChain().GetFilePath("crtn.o")));
 

MaskRay wrote:
> This appears to fix another bug.
> This appears to fix another bug.

It appears that crtn.o should be handled in the same manner as crti.o further 
up in Solaris.cpp.

https://blogs.oracle.com/solaris/post/new-crt-objects-or-what-are-crt-objects


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124356

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

[PATCH] D124365: review updates

[Tobias Ribizel via Phabricator via cfe-commits]

upsj created this revision.
Herald added subscribers: usaxena95, kadircet, arphaman.
Herald added a project: All.
upsj requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

- Fix inverted check-lines condition
- Output InlayHintKind via ostream operators


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D124365

Files:
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/Protocol.h
  clang-tools-extra/clangd/tool/Check.cpp


Index: clang-tools-extra/clangd/tool/Check.cpp
===
--- clang-tools-extra/clangd/tool/Check.cpp
+++ clang-tools-extra/clangd/tool/Check.cpp
@@ -193,14 +193,13 @@
   }
 
   // Build Inlay Hints for the entire AST or the specified range
-  bool buildInlayHints(llvm::Optional LineRange) {
+  void buildInlayHints(llvm::Optional LineRange) {
 log("Building inlay hints");
 auto Hints = inlayHints(*AST, LineRange);
 
 for (const auto &Hint : Hints) {
-  vlog("  {0} {1} {2}", toString(Hint.kind), Hint.position, Hint.label);
+  vlog("  {0} {1} {2}", Hint.kind, Hint.position, Hint.label);
 }
-return true;
   }
 
   // Run AST-based features at each token in the file.
@@ -219,7 +218,7 @@
   unsigned End = Start + Tok.length();
   Position Pos = offsetToPosition(Inputs.Contents, Start);
 
-  if (LineRange && LineRange->contains(Pos))
+  if (LineRange && !LineRange->contains(Pos))
 continue;
 
   trace::Span Trace("Token");
@@ -293,8 +292,9 @@
   : /*Don't turn on local configs for an arbitrary temp path.*/ ""));
   Checker C(File, Opts);
   if (!C.buildCommand(TFS) || !C.buildInvocation(TFS, Contents) ||
-  !C.buildAST() || !C.buildInlayHints(LineRange))
+  !C.buildAST())
 return false;
+  C.buildInlayHints(LineRange);
   C.testLocationFeatures(LineRange, EnableCodeCompletion);
 
   log("All checks completed, {0} errors", C.ErrCount);
Index: clang-tools-extra/clangd/Protocol.h
===
--- clang-tools-extra/clangd/Protocol.h
+++ clang-tools-extra/clangd/Protocol.h
@@ -1576,10 +1576,10 @@
   /// naturally when placed inline with the code.
   std::string label;
 };
-const char *toString(InlayHintKind);
 llvm::json::Value toJSON(const InlayHint &);
 bool operator==(const InlayHint &, const InlayHint &);
 bool operator<(const InlayHint &, const InlayHint &);
+llvm::raw_ostream &operator<<(llvm::raw_ostream &, InlayHintKind);
 
 struct ReferenceContext {
   /// Include the declaration of the current symbol.
Index: clang-tools-extra/clangd/Protocol.cpp
===
--- clang-tools-extra/clangd/Protocol.cpp
+++ clang-tools-extra/clangd/Protocol.cpp
@@ -1345,6 +1345,10 @@
  std::tie(B.position, B.range, B.kind, B.label);
 }
 
+llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, InlayHintKind Kind) {
+  return OS << toString(Kind);
+}
+
 static const char *toString(OffsetEncoding OE) {
   switch (OE) {
   case OffsetEncoding::UTF8:


Index: clang-tools-extra/clangd/tool/Check.cpp
===
--- clang-tools-extra/clangd/tool/Check.cpp
+++ clang-tools-extra/clangd/tool/Check.cpp
@@ -193,14 +193,13 @@
   }
 
   // Build Inlay Hints for the entire AST or the specified range
-  bool buildInlayHints(llvm::Optional LineRange) {
+  void buildInlayHints(llvm::Optional LineRange) {
 log("Building inlay hints");
 auto Hints = inlayHints(*AST, LineRange);
 
 for (const auto &Hint : Hints) {
-  vlog("  {0} {1} {2}", toString(Hint.kind), Hint.position, Hint.label);
+  vlog("  {0} {1} {2}", Hint.kind, Hint.position, Hint.label);
 }
-return true;
   }
 
   // Run AST-based features at each token in the file.
@@ -219,7 +218,7 @@
   unsigned End = Start + Tok.length();
   Position Pos = offsetToPosition(Inputs.Contents, Start);
 
-  if (LineRange && LineRange->contains(Pos))
+  if (LineRange && !LineRange->contains(Pos))
 continue;
 
   trace::Span Trace("Token");
@@ -293,8 +292,9 @@
   : /*Don't turn on local configs for an arbitrary temp path.*/ ""));
   Checker C(File, Opts);
   if (!C.buildCommand(TFS) || !C.buildInvocation(TFS, Contents) ||
-  !C.buildAST() || !C.buildInlayHints(LineRange))
+  !C.buildAST())
 return false;
+  C.buildInlayHints(LineRange);
   C.testLocationFeatures(LineRange, EnableCodeCompletion);
 
   log("All checks completed, {0} errors", C.ErrCount);
Index: clang-tools-extra/clangd/Protocol.h
===
--- clang-tools-extra/clangd/Protocol.h
+++ clang-tools-extra/clangd/Protocol.h
@@ -1576,10 +1576,10 @@
   /// naturally when placed inline with the code.
   std::string label;
 };
-const char *toString(InlayHintKind);
 llvm::json::Value toJSON(const InlayHint &)

[PATCH] D124344: [clangd] Output inlay hints with `clangd --check`

[Tobias Ribizel via Phabricator via cfe-commits]

upsj updated this revision to Diff 424838.
upsj added a comment.

Review updates

- Fix inverted check-lines condition
- Output InlayHintKind via ostream operators


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124344

Files:
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/Protocol.h
  clang-tools-extra/clangd/tool/Check.cpp


Index: clang-tools-extra/clangd/tool/Check.cpp
===
--- clang-tools-extra/clangd/tool/Check.cpp
+++ clang-tools-extra/clangd/tool/Check.cpp
@@ -193,14 +193,13 @@
   }
 
   // Build Inlay Hints for the entire AST or the specified range
-  bool buildInlayHints(llvm::Optional LineRange) {
+  void buildInlayHints(llvm::Optional LineRange) {
 log("Building inlay hints");
 auto Hints = inlayHints(*AST, LineRange);
 
 for (const auto &Hint : Hints) {
-  vlog("  {0} {1} {2}", toString(Hint.kind), Hint.position, Hint.label);
+  vlog("  {0} {1} {2}", Hint.kind, Hint.position, Hint.label);
 }
-return true;
   }
 
   // Run AST-based features at each token in the file.
@@ -219,7 +218,7 @@
   unsigned End = Start + Tok.length();
   Position Pos = offsetToPosition(Inputs.Contents, Start);
 
-  if (LineRange && LineRange->contains(Pos))
+  if (LineRange && !LineRange->contains(Pos))
 continue;
 
   trace::Span Trace("Token");
@@ -293,8 +292,9 @@
   : /*Don't turn on local configs for an arbitrary temp path.*/ ""));
   Checker C(File, Opts);
   if (!C.buildCommand(TFS) || !C.buildInvocation(TFS, Contents) ||
-  !C.buildAST() || !C.buildInlayHints(LineRange))
+  !C.buildAST())
 return false;
+  C.buildInlayHints(LineRange);
   C.testLocationFeatures(LineRange, EnableCodeCompletion);
 
   log("All checks completed, {0} errors", C.ErrCount);
Index: clang-tools-extra/clangd/Protocol.h
===
--- clang-tools-extra/clangd/Protocol.h
+++ clang-tools-extra/clangd/Protocol.h
@@ -1576,10 +1576,10 @@
   /// naturally when placed inline with the code.
   std::string label;
 };
-const char *toString(InlayHintKind);
 llvm::json::Value toJSON(const InlayHint &);
 bool operator==(const InlayHint &, const InlayHint &);
 bool operator<(const InlayHint &, const InlayHint &);
+llvm::raw_ostream &operator<<(llvm::raw_ostream &, InlayHintKind);
 
 struct ReferenceContext {
   /// Include the declaration of the current symbol.
Index: clang-tools-extra/clangd/Protocol.cpp
===
--- clang-tools-extra/clangd/Protocol.cpp
+++ clang-tools-extra/clangd/Protocol.cpp
@@ -1345,6 +1345,10 @@
  std::tie(B.position, B.range, B.kind, B.label);
 }
 
+llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, InlayHintKind Kind) {
+  return OS << toString(Kind);
+}
+
 static const char *toString(OffsetEncoding OE) {
   switch (OE) {
   case OffsetEncoding::UTF8:


Index: clang-tools-extra/clangd/tool/Check.cpp
===
--- clang-tools-extra/clangd/tool/Check.cpp
+++ clang-tools-extra/clangd/tool/Check.cpp
@@ -193,14 +193,13 @@
   }
 
   // Build Inlay Hints for the entire AST or the specified range
-  bool buildInlayHints(llvm::Optional LineRange) {
+  void buildInlayHints(llvm::Optional LineRange) {
 log("Building inlay hints");
 auto Hints = inlayHints(*AST, LineRange);
 
 for (const auto &Hint : Hints) {
-  vlog("  {0} {1} {2}", toString(Hint.kind), Hint.position, Hint.label);
+  vlog("  {0} {1} {2}", Hint.kind, Hint.position, Hint.label);
 }
-return true;
   }
 
   // Run AST-based features at each token in the file.
@@ -219,7 +218,7 @@
   unsigned End = Start + Tok.length();
   Position Pos = offsetToPosition(Inputs.Contents, Start);
 
-  if (LineRange && LineRange->contains(Pos))
+  if (LineRange && !LineRange->contains(Pos))
 continue;
 
   trace::Span Trace("Token");
@@ -293,8 +292,9 @@
   : /*Don't turn on local configs for an arbitrary temp path.*/ ""));
   Checker C(File, Opts);
   if (!C.buildCommand(TFS) || !C.buildInvocation(TFS, Contents) ||
-  !C.buildAST() || !C.buildInlayHints(LineRange))
+  !C.buildAST())
 return false;
+  C.buildInlayHints(LineRange);
   C.testLocationFeatures(LineRange, EnableCodeCompletion);
 
   log("All checks completed, {0} errors", C.ErrCount);
Index: clang-tools-extra/clangd/Protocol.h
===
--- clang-tools-extra/clangd/Protocol.h
+++ clang-tools-extra/clangd/Protocol.h
@@ -1576,10 +1576,10 @@
   /// naturally when placed inline with the code.
   std::string label;
 };
-const char *toString(InlayHintKind);
 llvm::json::Value toJSON(const InlayHint &);
 bool operator==(const InlayHint &, const InlayHint &);
 bool operator<(const InlayHint

[PATCH] D123352: [analyzer] Add FixItHint to `nullability.NullReturnedFromNonnull` and `nullability.NullableReturnedFromNonnull`

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

steakhal added a subscriber: NoQ.
steakhal added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp:589
+  RetTypeLoc.getType().getCanonicalType().getAsString();
+  if (CanonicalTypeStr == "id" || CanonicalTypeStr == "instancetype") {
+return None;

You should avoid `std::string` comparisons. `llvm::StringRefs` are a better 
alternative.



Comment at: clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp:592-602
+  // If we're inside of an NS_ASSUME, then the sourceRange will end before the
+  // asterisk.
+  const auto CanonicalTypeSize = CanonicalTypeStr.size();
+  const bool IsInsideOfAssume =
+  NullabilityLoc == 
RetTypeLoc.getSourceRange().getBegin().getLocWithOffset(
+CanonicalTypeSize - 1);
+

Uh, this is really ugly. I don't believe this is the preferred way of detecting 
this. @NoQ WDYT?



Comment at: clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp:687-688
+
+  const bool ContextSupportsSyntaxSugar =
+  dyn_cast(D) != nullptr;
+  llvm::Optional Hint =





Comment at: clang/test/Analysis/nullability-fixits.mm:5-10
+// RUN: %clang_cc1 -analyze \
+
+// RUN: -Wno-nonnull \
+// RUN: 
-analyzer-checker=core,nullability.NullableReturnedFromNonnull,nullability.NullReturnedFromNonnull
 \
+// RUN: -fdiagnostics-parseable-fixits %s 2>%t
+// RUN: cat %t | FileCheck %s -check-prefix=CHECK-FIXIT-DISABLED-IMPLICIT

Why don't you use the `%clang_analyze_cc1 ...` form instead or even better the 
`%check_analyzer_fixit` tool-subst pattern?
See the `clang/test/Analysis/dead-stores.c` as an example.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123352

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

[PATCH] D124344: [clangd] Output inlay hints with `clangd --check`

[Tobias Ribizel via Phabricator via cfe-commits]

upsj updated this revision to Diff 424839.
upsj added a comment.

push full patch


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124344

Files:
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/Protocol.h
  clang-tools-extra/clangd/tool/Check.cpp
  clang-tools-extra/clangd/tool/ClangdMain.cpp

Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -61,8 +61,7 @@
 namespace clangd {
 
 // Implemented in Check.cpp.
-bool check(const llvm::StringRef File,
-   llvm::function_ref ShouldCheckLine,
+bool check(const llvm::StringRef File, llvm::Optional LineRange,
const ThreadsafeFS &TFS, const ClangdLSPServer::Options &Opts,
bool EnableCodeCompletion);
 
@@ -955,8 +954,9 @@
   return 1;
 }
 log("Entering check mode (no LSP server)");
-uint32_t Begin = 0, End = std::numeric_limits::max();
+llvm::Optional CheckLineRange;
 if (!CheckFileLines.empty()) {
+  uint32_t Begin = 0, End = std::numeric_limits::max();
   StringRef RangeStr(CheckFileLines);
   bool ParseError = RangeStr.consumeInteger(0, Begin);
   if (RangeStr.empty()) {
@@ -965,19 +965,18 @@
 ParseError |= !RangeStr.consume_front("-");
 ParseError |= RangeStr.consumeInteger(0, End);
   }
-  if (ParseError || !RangeStr.empty()) {
-elog("Invalid --check-line specified. Use Begin-End format, e.g. 3-17");
+  if (ParseError || !RangeStr.empty() || Begin <= 0 || End < Begin) {
+elog(
+"Invalid --check-lines specified. Use Begin-End format, e.g. 3-17");
 return 1;
   }
+  CheckLineRange = Range{Position{static_cast(Begin - 1), 0},
+ Position{static_cast(End), 0}};
 }
-auto ShouldCheckLine = [&](const Position &Pos) {
-  uint32_t Line = Pos.line + 1; // Position::line is 0-based.
-  return Line >= Begin && Line <= End;
-};
 // For now code completion is enabled any time the range is limited via
 // --check-lines. If it turns out to be to slow, we can introduce a
 // dedicated flag for that instead.
-return check(Path, ShouldCheckLine, TFS, Opts,
+return check(Path, CheckLineRange, TFS, Opts,
  /*EnableCodeCompletion=*/!CheckFileLines.empty())
? 0
: static_cast(ErrorResultCode::CheckFailed);
Index: clang-tools-extra/clangd/tool/Check.cpp
===
--- clang-tools-extra/clangd/tool/Check.cpp
+++ clang-tools-extra/clangd/tool/Check.cpp
@@ -30,8 +30,10 @@
 #include "Config.h"
 #include "GlobalCompilationDatabase.h"
 #include "Hover.h"
+#include "InlayHints.h"
 #include "ParsedAST.h"
 #include "Preamble.h"
+#include "Protocol.h"
 #include "SourceCode.h"
 #include "XRefs.h"
 #include "index/CanonicalIncludes.h"
@@ -190,10 +192,19 @@
 return true;
   }
 
+  // Build Inlay Hints for the entire AST or the specified range
+  void buildInlayHints(llvm::Optional LineRange) {
+log("Building inlay hints");
+auto Hints = inlayHints(*AST, LineRange);
+
+for (const auto &Hint : Hints) {
+  vlog("  {0} {1} {2}", Hint.kind, Hint.position, Hint.label);
+}
+  }
+
   // Run AST-based features at each token in the file.
-  void testLocationFeatures(
-  llvm::function_ref ShouldCheckLine,
-  const bool EnableCodeCompletion) {
+  void testLocationFeatures(llvm::Optional LineRange,
+const bool EnableCodeCompletion) {
 trace::Span Trace("testLocationFeatures");
 log("Testing features at each token (may be slow in large files)");
 auto &SM = AST->getSourceManager();
@@ -207,7 +218,7 @@
   unsigned End = Start + Tok.length();
   Position Pos = offsetToPosition(Inputs.Contents, Start);
 
-  if (!ShouldCheckLine(Pos))
+  if (LineRange && !LineRange->contains(Pos))
 continue;
 
   trace::Span Trace("Token");
@@ -254,8 +265,7 @@
 
 } // namespace
 
-bool check(llvm::StringRef File,
-   llvm::function_ref ShouldCheckLine,
+bool check(llvm::StringRef File, llvm::Optional LineRange,
const ThreadsafeFS &TFS, const ClangdLSPServer::Options &Opts,
bool EnableCodeCompletion) {
   llvm::SmallString<0> FakeFile;
@@ -284,7 +294,8 @@
   if (!C.buildCommand(TFS) || !C.buildInvocation(TFS, Contents) ||
   !C.buildAST())
 return false;
-  C.testLocationFeatures(ShouldCheckLine, EnableCodeCompletion);
+  C.buildInlayHints(LineRange);
+  C.testLocationFeatures(LineRange, EnableCodeCompletion);
 
   log("All checks completed, {0} errors", C.ErrCount);
   return C.ErrCount == 0;
Index: clang-tools-extra/clangd/Protocol.h
===
--- clang-too

[PATCH] D124349: [analyzer] Get direct binding for specific punned case

[Vince Bridgers via Phabricator via cfe-commits]

vabridgers added inline comments.



Comment at: clang/test/Analysis/array-struct-region.c:362-366
+int array_struct_bitfield_1() {
+  BITFIELD_CAST ff = {0};
+  BITFIELD_CAST *pff = &ff;
+  return *((int *)pff + 1);
+}

steakhal wrote:
> ```lang=c
> void array_struct_bitfield_1() {
>   BITFIELD_CAST ff = {0};
>   BITFIELD_CAST *pff = &ff;
>   clang_analyzer_eval(*((int *)pff + 1) == 0); // expected-warning{{TRUE}}
>   ff.b[0] = 3;
>   clang_analyzer_eval(*((int *)pff + 1) == 3); // expected-warning{{TRUE}}
> }
> ```
Good suggestion, thanks. I'll add this and wait for another accept.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124349

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

[PATCH] D124349: [analyzer] Get direct binding for specific punned case

[Vince Bridgers via Phabricator via cfe-commits]

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

update LIT per comments from @steakhal


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124349

Files:
  clang/lib/StaticAnalyzer/Core/RegionStore.cpp
  clang/test/Analysis/array-struct-region.c


Index: clang/test/Analysis/array-struct-region.c
===
--- clang/test/Analysis/array-struct-region.c
+++ clang/test/Analysis/array-struct-region.c
@@ -353,3 +353,23 @@
   // FIXME: Should be TRUE.
   clang_analyzer_eval(vals[index].a[0].x == 42); // expected-warning{{UNKNOWN}}
 }
+
+typedef struct {
+  int a : 1;
+  int b[2];
+} BITFIELD_CAST;
+
+void array_struct_bitfield_1() {
+  BITFIELD_CAST ff = {0};
+  BITFIELD_CAST *pff = &ff;
+  clang_analyzer_eval(*((int *)pff + 1) == 0); // expected-warning{{TRUE}}
+  ff.b[0] = 3;
+  clang_analyzer_eval(*((int *)pff + 1) == 3); // expected-warning{{TRUE}}
+}
+
+int array_struct_bitfield_2() {
+  BITFIELD_CAST ff = {0};
+  BITFIELD_CAST *pff = &ff;
+  int a = *((int *)pff + 2); // expected-warning{{Assigned value is garbage or 
undefined [core.uninitialized.Assign]}}
+  return a;
+}
Index: clang/lib/StaticAnalyzer/Core/RegionStore.cpp
===
--- clang/lib/StaticAnalyzer/Core/RegionStore.cpp
+++ clang/lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -2153,8 +2153,13 @@
   return UnknownVal();
 
 // Additionally allow introspection of a block's internal layout.
-if (!hasPartialLazyBinding && !isa(R->getBaseRegion()))
+// Try to get direct binding if all other attempts failed thus far.
+// Else, return UndefinedVal()
+if (!hasPartialLazyBinding && !isa(R->getBaseRegion())) {
+  if (const Optional &V = B.getDefaultBinding(R))
+return *V;
   return UndefinedVal();
+}
   }
 
   // All other values are symbolic.


Index: clang/test/Analysis/array-struct-region.c
===
--- clang/test/Analysis/array-struct-region.c
+++ clang/test/Analysis/array-struct-region.c
@@ -353,3 +353,23 @@
   // FIXME: Should be TRUE.
   clang_analyzer_eval(vals[index].a[0].x == 42); // expected-warning{{UNKNOWN}}
 }
+
+typedef struct {
+  int a : 1;
+  int b[2];
+} BITFIELD_CAST;
+
+void array_struct_bitfield_1() {
+  BITFIELD_CAST ff = {0};
+  BITFIELD_CAST *pff = &ff;
+  clang_analyzer_eval(*((int *)pff + 1) == 0); // expected-warning{{TRUE}}
+  ff.b[0] = 3;
+  clang_analyzer_eval(*((int *)pff + 1) == 3); // expected-warning{{TRUE}}
+}
+
+int array_struct_bitfield_2() {
+  BITFIELD_CAST ff = {0};
+  BITFIELD_CAST *pff = &ff;
+  int a = *((int *)pff + 2); // expected-warning{{Assigned value is garbage or undefined [core.uninitialized.Assign]}}
+  return a;
+}
Index: clang/lib/StaticAnalyzer/Core/RegionStore.cpp
===
--- clang/lib/StaticAnalyzer/Core/RegionStore.cpp
+++ clang/lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -2153,8 +2153,13 @@
   return UnknownVal();
 
 // Additionally allow introspection of a block's internal layout.
-if (!hasPartialLazyBinding && !isa(R->getBaseRegion()))
+// Try to get direct binding if all other attempts failed thus far.
+// Else, return UndefinedVal()
+if (!hasPartialLazyBinding && !isa(R->getBaseRegion())) {
+  if (const Optional &V = B.getDefaultBinding(R))
+return *V;
   return UndefinedVal();
+}
   }
 
   // All other values are symbolic.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D124344: [clangd] Output inlay hints with `clangd --check`

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

(LGTM too, thanks!)




Comment at: clang-tools-extra/clangd/Protocol.cpp:1319
 
-llvm::json::Value toJSON(InlayHintKind K) {
+const char *toString(InlayHintKind K) {
   switch (K) {

nit: static (this doesn't need to be public)
nit: return llvm::StringLiteral (which includes the length)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124344

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

[PATCH] D124344: [clangd] Output inlay hints with `clangd --check`

[Tobias Ribizel via Phabricator via cfe-commits]

upsj updated this revision to Diff 424846.
upsj added a comment.

- use string literal for toString result


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124344

Files:
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/Protocol.h
  clang-tools-extra/clangd/tool/Check.cpp
  clang-tools-extra/clangd/tool/ClangdMain.cpp

Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -61,8 +61,7 @@
 namespace clangd {
 
 // Implemented in Check.cpp.
-bool check(const llvm::StringRef File,
-   llvm::function_ref ShouldCheckLine,
+bool check(const llvm::StringRef File, llvm::Optional LineRange,
const ThreadsafeFS &TFS, const ClangdLSPServer::Options &Opts,
bool EnableCodeCompletion);
 
@@ -955,8 +954,9 @@
   return 1;
 }
 log("Entering check mode (no LSP server)");
-uint32_t Begin = 0, End = std::numeric_limits::max();
+llvm::Optional CheckLineRange;
 if (!CheckFileLines.empty()) {
+  uint32_t Begin = 0, End = std::numeric_limits::max();
   StringRef RangeStr(CheckFileLines);
   bool ParseError = RangeStr.consumeInteger(0, Begin);
   if (RangeStr.empty()) {
@@ -965,19 +965,18 @@
 ParseError |= !RangeStr.consume_front("-");
 ParseError |= RangeStr.consumeInteger(0, End);
   }
-  if (ParseError || !RangeStr.empty()) {
-elog("Invalid --check-line specified. Use Begin-End format, e.g. 3-17");
+  if (ParseError || !RangeStr.empty() || Begin <= 0 || End < Begin) {
+elog(
+"Invalid --check-lines specified. Use Begin-End format, e.g. 3-17");
 return 1;
   }
+  CheckLineRange = Range{Position{static_cast(Begin - 1), 0},
+ Position{static_cast(End), 0}};
 }
-auto ShouldCheckLine = [&](const Position &Pos) {
-  uint32_t Line = Pos.line + 1; // Position::line is 0-based.
-  return Line >= Begin && Line <= End;
-};
 // For now code completion is enabled any time the range is limited via
 // --check-lines. If it turns out to be to slow, we can introduce a
 // dedicated flag for that instead.
-return check(Path, ShouldCheckLine, TFS, Opts,
+return check(Path, CheckLineRange, TFS, Opts,
  /*EnableCodeCompletion=*/!CheckFileLines.empty())
? 0
: static_cast(ErrorResultCode::CheckFailed);
Index: clang-tools-extra/clangd/tool/Check.cpp
===
--- clang-tools-extra/clangd/tool/Check.cpp
+++ clang-tools-extra/clangd/tool/Check.cpp
@@ -30,8 +30,10 @@
 #include "Config.h"
 #include "GlobalCompilationDatabase.h"
 #include "Hover.h"
+#include "InlayHints.h"
 #include "ParsedAST.h"
 #include "Preamble.h"
+#include "Protocol.h"
 #include "SourceCode.h"
 #include "XRefs.h"
 #include "index/CanonicalIncludes.h"
@@ -190,10 +192,19 @@
 return true;
   }
 
+  // Build Inlay Hints for the entire AST or the specified range
+  void buildInlayHints(llvm::Optional LineRange) {
+log("Building inlay hints");
+auto Hints = inlayHints(*AST, LineRange);
+
+for (const auto &Hint : Hints) {
+  vlog("  {0} {1} {2}", Hint.kind, Hint.position, Hint.label);
+}
+  }
+
   // Run AST-based features at each token in the file.
-  void testLocationFeatures(
-  llvm::function_ref ShouldCheckLine,
-  const bool EnableCodeCompletion) {
+  void testLocationFeatures(llvm::Optional LineRange,
+const bool EnableCodeCompletion) {
 trace::Span Trace("testLocationFeatures");
 log("Testing features at each token (may be slow in large files)");
 auto &SM = AST->getSourceManager();
@@ -207,7 +218,7 @@
   unsigned End = Start + Tok.length();
   Position Pos = offsetToPosition(Inputs.Contents, Start);
 
-  if (!ShouldCheckLine(Pos))
+  if (LineRange && !LineRange->contains(Pos))
 continue;
 
   trace::Span Trace("Token");
@@ -254,8 +265,7 @@
 
 } // namespace
 
-bool check(llvm::StringRef File,
-   llvm::function_ref ShouldCheckLine,
+bool check(llvm::StringRef File, llvm::Optional LineRange,
const ThreadsafeFS &TFS, const ClangdLSPServer::Options &Opts,
bool EnableCodeCompletion) {
   llvm::SmallString<0> FakeFile;
@@ -284,7 +294,8 @@
   if (!C.buildCommand(TFS) || !C.buildInvocation(TFS, Contents) ||
   !C.buildAST())
 return false;
-  C.testLocationFeatures(ShouldCheckLine, EnableCodeCompletion);
+  C.buildInlayHints(LineRange);
+  C.testLocationFeatures(LineRange, EnableCodeCompletion);
 
   log("All checks completed, {0} errors", C.ErrCount);
   return C.ErrCount == 0;
Index: clang-tools-extra/clangd/Protocol.h

[PATCH] D121370: [clang-format] SortIncludes should support "@import" lines in Objective-C

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir added a comment.

It appears that this regressed include sorting in the following case, where the 
contents of `test.h` show the expected include order and the `clang-format` 
behavior before this patch:

  % cat test.h
  #include 
  
  #include "util/bar.h"
  #include "util/foo/foo.h"  // foo
  % bin/clang-format --version; bin/clang-format -style=google test.h
  clang-format version 15.0.0 (https://github.com/llvm/llvm-project.git 
d46fa023caa2db5a9f1e21dd038bcb626261d958)
  #include "util/foo/foo.h"  // foo
  #include 
  
  #include "util/bar.h"

@kwk could you please take a look


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121370

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

[PATCH] D124250: [Serialization] write expr dependence bits as a single integer

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.

LGTM. Nice savings




Comment at: clang/lib/Serialization/ASTWriterDecl.cpp:2291
 
+  unsigned ExprDependenceBits = llvm::BitWidth;
   // Abbreviation for EXPR_DECL_REF

NIT:  add `constexpr`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124250

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

[PATCH] D119544: Deferred Concept Instantiation Implementation

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added inline comments.



Comment at: clang/test/SemaTemplate/concepts.cpp:391
+
+  CausesFriendConstraint CFC;
+  FriendFunc(CFC, 1);

erichkeane wrote:
> erichkeane wrote:
> > A bunch of the tests below this all fail.
> See these two tests, which fail by saying that "::local is not a member of 
> class 'X'".
I've spent some time to look into this. I don't find the root cause now. But I 
find that it is related to the wrong lookups. The error is emitted in 
CheckQualifiedMemberReference. And it looks like we lookup for the original S 
instead of the instantiated S. And the compiler thinks the 2 structs are 
different. So here is the error. Do you have any other ideas?


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

https://reviews.llvm.org/D119544

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

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers

[Gabor Marton via Phabricator via cfe-commits]

martong added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:2280-2290
+/// Return a symbol which is the best canidate to save it in the constraint
+/// map. We should correct symbol because in case of truncation cast we can
+/// only reason about truncated bytes but not the whole value. E.g. (char)(int
+/// x), we can store constraints for the first lower byte but we still don't
+/// know the root value. Also in case of promotion or converion we should
+/// store the root value instead of cast symbol, because we can always get
+/// a correct range using `castTo` metho. And we are not intrested in any

ASDenysPetrov wrote:
> martong wrote:
> > I think, we should definitely store the constraints as they appear in the 
> > analyzed code. This way we mix the infer logic into the constraint setting, 
> > which is bad.
> > I mean, we should simply store the constraint directly for the symbol as it 
> > is. And then only in `VisitSymbolCast` should we infer the proper value 
> > from the stored constraint (if we can).
> > 
> > (Of course, if we have related symbols (casts of the original symbol) then 
> > their constraints must be updated.)
> I see what you mean. I thought about this. Here what I've faced with.
> # Let's say you meet `(wchar_t)x > 0`, which you store like a pair 
> {(wchar_t)x, [1,32767]}.
> # Then you meet `(short)x < 0`, where you have to answer whether it's `true` 
> or `false`.
> # So would be your next step? Brute forcing all the constraint pairs to find 
> any `x`-related symbol? Obviously, O(n) complexity for every single integral 
> symbol is inefficient.
> What I propose is to "canonize" arbitrary types to a general form where this 
> form could be a part of key along with `x` and we could get a constraint with 
> a classic map complexity. So that:
> # You meet `(wchar_t)x > 0`, which you convert `wchar_t` to `int16` and store 
> like a pair {(int16)x, [1,32767]}.
> # Then you meet `(short)x < 0`, where you convert `short` to `int16` and get 
> a constraint.
> That's why I've introduced `NominalTypeList`.
> But now I admited your concern about arbitrary size of integers and will 
> redesign my solution.
> So would be your next step? Brute forcing all the constraint pairs to find 
> any x-related symbol? Obviously, O(n) complexity for every single integral 
> symbol is inefficient.

I don't think we need a brute force search among the constraints if we have the 
additional `CastMap` ([[ https://reviews.llvm.org/D103096#3459049 | that I have 
previously proposed ]]).
So, the next step would be: Lookup the root symbol of `(short)x` that is 
`(wchar_t)x` (supposing we have seen a declaration `wchar_t x;` first during 
the symbolic execution, but having a different root might work out as well).
Then from the `CastMap` we can query in O(1) the set of the related cast 
symbols of the root symbol. From this set it takes to query the constraints for 
each of the members from the existing equivalneClass->constraint mapping. 
That's O(1) times the number of the members of the cast set (which is assumed 
to be very few in the usual case).




Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:2346-2350
+/// FIXME: Update bigger casts. We only can reason about ranges of smaller
+/// types, because it would be too complicated to update, say, the entire
+/// `int` range if you only have knowledge that its lowest byte has been
+/// changed. So we don't touch bigger casts and they may be potentially
+/// invalid. For future, for:

ASDenysPetrov wrote:
> martong wrote:
> > Instead of a noop we should be more conservative in this case. We should 
> > invalidate (remove) the constraints of all the symbols that have more bits 
> > than the currently set symbol. However, we might be clever in cases of 
> > special values (e.g `0` or in case of the `true` rangeSet {[MIN, -1], [1, 
> > MAX]}).
> No, it's incorrect. Consider next:
> ```
> int x;
> if(x > 100 || x < 10) 
>   return;
> // x (100'000, 1000'000) 
> if((int8)x != 42) 
>   return;
> // x (100'000, 1000'000) && (int8)x (42, 42) 
> ```
> We can't just remove or invalidate `x (100'000, 1000'000)` because this range 
> will still stay //true//.
> Strictly speaking `x` range should be updated with values 100394, 102442, 
> 108586, ...,, 960554 and any other value within the range which has its 
> lowest byte equals to 42.
> We can't just update the `RangeSet` with such a big amount of values due to 
> performance issues. So we just assume it as less accurate.
Okay, this makes perfect sense, thanks for the example!


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

https://reviews.llvm.org/D103096

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

[PATCH] D115187: [clangd] Expose CoawaitExpr's operand in the AST

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In D115187#3471003 , @nridge wrote:

> Without my patch, this compiles fine. With it, I clang gives the following 
> errors:
>
>   coroutines.cpp:35:9: error: no viable conversion from 'awaitable' to 
> 'transform_awaitable'
>   coro await_template(U t) {
>   ^~
>   coroutines.cpp:39:34: note: in instantiation of function template 
> specialization 'await_template' 
> requested here
>   template coro 
> await_template(transform_awaitable);

Poked at this a bit locally.
From the message we see there's a call to `BuildUnresolvedCoawaitExpr` with an 
`awaitable` rather than `transform_awaitable`, so the `await_transform` call 
fails.

It turns out this is the `initial_suspend` call (CXXMemberCallExpr). We're not 
supposed to  transform initial/final 
suspends, but the code is now doing so.

This seems to be a consequence of switching from BuildResolvedCoawaitExpr 
(which does not apply the transform) to BuildUnresolvedCoawaitExpr (which does) 
during template instantiation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115187

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

[PATCH] D124250: [Serialization] write expr dependence bits as a single integer

[Sam McCall via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG0cd5cd19af0e: [Serialization] write expr dependence bits as 
a single integer (authored by sammccall).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124250

Files:
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp

Index: clang/lib/Serialization/ASTWriterStmt.cpp
===
--- clang/lib/Serialization/ASTWriterStmt.cpp
+++ clang/lib/Serialization/ASTWriterStmt.cpp
@@ -543,11 +543,7 @@
 void ASTStmtWriter::VisitExpr(Expr *E) {
   VisitStmt(E);
   Record.AddTypeRef(E->getType());
-  Record.push_back(E->isTypeDependent());
-  Record.push_back(E->isValueDependent());
-  Record.push_back(E->isInstantiationDependent());
-  Record.push_back(E->containsUnexpandedParameterPack());
-  Record.push_back(E->containsErrors());
+  Record.push_back(E->getDependence());
   Record.push_back(E->getValueKind());
   Record.push_back(E->getObjectKind());
 }
Index: clang/lib/Serialization/ASTWriterDecl.cpp
===
--- clang/lib/Serialization/ASTWriterDecl.cpp
+++ clang/lib/Serialization/ASTWriterDecl.cpp
@@ -2288,17 +2288,14 @@
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6));
   DeclCXXMethodAbbrev = Stream.EmitAbbrev(std::move(Abv));
 
+  unsigned ExprDependenceBits = llvm::BitWidth;
   // Abbreviation for EXPR_DECL_REF
   Abv = std::make_shared();
   Abv->Add(BitCodeAbbrevOp(serialization::EXPR_DECL_REF));
   //Stmt
   // Expr
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // Type
-  Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); //TypeDependent
-  Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); //ValueDependent
-  Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); //InstantiationDependent
-  Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); //UnexpandedParamPack
-  Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); //ContainsErrors
+  Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, ExprDependenceBits));
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 3)); //GetValueKind
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 3)); //GetObjectKind
   //DeclRefExpr
@@ -2318,11 +2315,7 @@
   //Stmt
   // Expr
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // Type
-  Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); //TypeDependent
-  Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); //ValueDependent
-  Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); //InstantiationDependent
-  Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); //UnexpandedParamPack
-  Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); //ContainsErrors
+  Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, ExprDependenceBits));
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 3)); //GetValueKind
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 3)); //GetObjectKind
   //Integer Literal
@@ -2337,11 +2330,7 @@
   //Stmt
   // Expr
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // Type
-  Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); //TypeDependent
-  Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); //ValueDependent
-  Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); //InstantiationDependent
-  Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); //UnexpandedParamPack
-  Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); //ContainsErrors
+  Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, ExprDependenceBits));
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 3)); //GetValueKind
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 3)); //GetObjectKind
   //Character Literal
@@ -2356,11 +2345,7 @@
   // Stmt
   // Expr
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // Type
-  Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); //TypeDependent
-  Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); //ValueDependent
-  Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); //InstantiationDependent
-  Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); //UnexpandedParamPack
-  Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); //ContainsErrors
+  Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, ExprDependenceBits));
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 3)); //GetValueKind
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 3)); //GetObjectKind
   // CastExpr
Index: clang/lib/Serialization/ASTReaderStmt.cpp
===
--- clang/lib/Serialization/ASTReaderStmt.cpp
+++ clang/lib/Serialization/ASTReaderStmt.cpp
@@ -107,8 +107,7 @@
 
 /// The number of record fields required for the Expr class
 /// itself.
-static const unsigned NumExprFields =
-

[clang] 0cd5cd1 - [Serialization] write expr dependence bits as a single integer

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2022-04-25T12:09:40+02:00
New Revision: 0cd5cd19af0eee1d35077de6824b2fe3022d2280

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

LOG: [Serialization] write expr dependence bits as a single integer

When exprs are written unabbreviated:
  - these were encoded as 5 x vbr6 = 30 bits
  - now they fit exactly into a one-chunk vbr = 6 bits

clangd --check=clangd/AST.cpp reports ~1% reduction in PCH size
(42826720->42474460)

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

Added: 


Modified: 
clang/lib/Serialization/ASTReaderStmt.cpp
clang/lib/Serialization/ASTWriterDecl.cpp
clang/lib/Serialization/ASTWriterStmt.cpp

Removed: 




diff  --git a/clang/lib/Serialization/ASTReaderStmt.cpp 
b/clang/lib/Serialization/ASTReaderStmt.cpp
index 281385ad9e7d9..77ea1b95d5be5 100644
--- a/clang/lib/Serialization/ASTReaderStmt.cpp
+++ b/clang/lib/Serialization/ASTReaderStmt.cpp
@@ -107,8 +107,7 @@ namespace clang {
 
 /// The number of record fields required for the Expr class
 /// itself.
-static const unsigned NumExprFields =
-NumStmtFields + llvm::BitWidth + 3;
+static const unsigned NumExprFields = NumStmtFields + 4;
 
 /// Read and initialize a ExplicitTemplateArgumentList structure.
 void ReadTemplateKWAndArgsInfo(ASTTemplateKWAndArgsInfo &Args,
@@ -521,26 +520,7 @@ void ASTStmtReader::VisitCapturedStmt(CapturedStmt *S) {
 void ASTStmtReader::VisitExpr(Expr *E) {
   VisitStmt(E);
   E->setType(Record.readType());
-
-  // FIXME: write and read all DependentFlags with a single call.
-  bool TypeDependent = Record.readInt();
-  bool ValueDependent = Record.readInt();
-  bool InstantiationDependent = Record.readInt();
-  bool ContainsUnexpandedTemplateParameters = Record.readInt();
-  bool ContainsErrors = Record.readInt();
-  auto Deps = ExprDependence::None;
-  if (TypeDependent)
-Deps |= ExprDependence::Type;
-  if (ValueDependent)
-Deps |= ExprDependence::Value;
-  if (InstantiationDependent)
-Deps |= ExprDependence::Instantiation;
-  if (ContainsUnexpandedTemplateParameters)
-Deps |= ExprDependence::UnexpandedPack;
-  if (ContainsErrors)
-Deps |= ExprDependence::Error;
-  E->setDependence(Deps);
-
+  E->setDependence(static_cast(Record.readInt()));
   E->setValueKind(static_cast(Record.readInt()));
   E->setObjectKind(static_cast(Record.readInt()));
   assert(Record.getIdx() == NumExprFields &&

diff  --git a/clang/lib/Serialization/ASTWriterDecl.cpp 
b/clang/lib/Serialization/ASTWriterDecl.cpp
index 4e9e97514de0b..3666d5a6daab8 100644
--- a/clang/lib/Serialization/ASTWriterDecl.cpp
+++ b/clang/lib/Serialization/ASTWriterDecl.cpp
@@ -2288,17 +2288,14 @@ void ASTWriter::WriteDeclAbbrevs() {
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6));
   DeclCXXMethodAbbrev = Stream.EmitAbbrev(std::move(Abv));
 
+  unsigned ExprDependenceBits = llvm::BitWidth;
   // Abbreviation for EXPR_DECL_REF
   Abv = std::make_shared();
   Abv->Add(BitCodeAbbrevOp(serialization::EXPR_DECL_REF));
   //Stmt
   // Expr
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // Type
-  Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); //TypeDependent
-  Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); //ValueDependent
-  Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); 
//InstantiationDependent
-  Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); //UnexpandedParamPack
-  Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); //ContainsErrors
+  Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, ExprDependenceBits));
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 3)); //GetValueKind
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 3)); //GetObjectKind
   //DeclRefExpr
@@ -2318,11 +2315,7 @@ void ASTWriter::WriteDeclAbbrevs() {
   //Stmt
   // Expr
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // Type
-  Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); //TypeDependent
-  Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); //ValueDependent
-  Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); 
//InstantiationDependent
-  Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); //UnexpandedParamPack
-  Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); //ContainsErrors
+  Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, ExprDependenceBits));
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 3)); //GetValueKind
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 3)); //GetObjectKind
   //Integer Literal
@@ -2337,11 +2330,7 @@ void ASTWriter::WriteDeclAbbrevs() {
   //Stmt
   // Expr
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // Type
-  Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); //TypeDependent
-  Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); //ValueDe

[PATCH] D124186: [RISCV] Fix incorrect policy implement for unmasked vslidedown and vslideup.

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

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

LGTM. Thanks @khchen !


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124186

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

[PATCH] D124373: [clang] add parameter pack/pack expansion matchers

[Tobias Ribizel via Phabricator via cfe-commits]

upsj created this revision.
upsj added a reviewer: klimek.
Herald added a project: All.
upsj requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

There is no (obvious to me) way to match pack expansions and parameter packs, 
so I added two matchers for them.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D124373

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


Index: clang/lib/ASTMatchers/Dynamic/Registry.cpp
===
--- clang/lib/ASTMatchers/Dynamic/Registry.cpp
+++ clang/lib/ASTMatchers/Dynamic/Registry.cpp
@@ -446,6 +446,7 @@
   REGISTER_MATCHER(isNoThrow);
   REGISTER_MATCHER(isNoneKind);
   REGISTER_MATCHER(isOverride);
+  REGISTER_MATCHER(isParameterPack);
   REGISTER_MATCHER(isPrivate);
   REGISTER_MATCHER(isProtected);
   REGISTER_MATCHER(isPublic);
@@ -513,6 +514,7 @@
   REGISTER_MATCHER(onImplicitObjectArgument);
   REGISTER_MATCHER(opaqueValueExpr);
   REGISTER_MATCHER(optionally);
+  REGISTER_MATCHER(packExpansionExpr);
   REGISTER_MATCHER(parameterCountIs);
   REGISTER_MATCHER(parenExpr);
   REGISTER_MATCHER(parenListExpr);
Index: clang/lib/ASTMatchers/ASTMatchersInternal.cpp
===
--- clang/lib/ASTMatchers/ASTMatchersInternal.cpp
+++ clang/lib/ASTMatchers/ASTMatchersInternal.cpp
@@ -981,6 +981,7 @@
 predefinedExpr;
 const internal::VariadicDynCastAllOfMatcher
 designatedInitExpr;
+const internal::VariadicDynCastAllOfMatcher 
packExpansionExpr;
 const internal::VariadicOperatorMatcherFunc<
 2, std::numeric_limits::max()>
 eachOf = {internal::DynTypedMatcher::VO_EachOf};
Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -731,6 +731,27 @@
   InnerMatcher.matches(*Initializer, Finder, Builder));
 }
 
+/// Determines whether a declaration is a parameter pack.
+///
+/// Given
+/// \code
+/// template  class C,
+///   template  class... Ds>
+/// void foo(A param, Bs... params);
+/// \endcode
+/// templateTypeParmDecl(isParameterPack())
+///   matches 'typename... Bs', but not 'typename A'.
+/// nonTypeTemplateParmDecl(isParameterPack())
+///   matches 'int... js', but not 'int i'.
+/// templateTemplateParmDecl(isParameterPack())
+///   matches 'template  class... Ds', but not
+///   'template  class C'
+/// varDecl(isParameterPack())
+///   matches 'Bs... params', but not 'A param'.
+AST_MATCHER(Decl, isParameterPack) { return Node.isParameterPack(); }
+
 /// Determines whether the function is "main", which is the entry point
 /// into an executable program.
 AST_MATCHER(FunctionDecl, isMain) {
@@ -2699,6 +2720,21 @@
   return Node.size() == N;
 }
 
+/// Matches pack expansion expressions.
+///
+/// Given
+/// \code
+/// void bar(int, int);
+/// template 
+/// void foo(T arg, Args args...) {
+/// bar(arg + 1, (args + 1)...);
+/// }
+/// \endcode
+/// packExpansionExpr()
+///   matches '(args + 1)...', but not 'arg + 1'.
+extern const internal::VariadicDynCastAllOfMatcher
+packExpansionExpr;
+
 /// Matches \c QualTypes in the clang AST.
 extern const internal::VariadicAllOfMatcher qualType;
 


Index: clang/lib/ASTMatchers/Dynamic/Registry.cpp
===
--- clang/lib/ASTMatchers/Dynamic/Registry.cpp
+++ clang/lib/ASTMatchers/Dynamic/Registry.cpp
@@ -446,6 +446,7 @@
   REGISTER_MATCHER(isNoThrow);
   REGISTER_MATCHER(isNoneKind);
   REGISTER_MATCHER(isOverride);
+  REGISTER_MATCHER(isParameterPack);
   REGISTER_MATCHER(isPrivate);
   REGISTER_MATCHER(isProtected);
   REGISTER_MATCHER(isPublic);
@@ -513,6 +514,7 @@
   REGISTER_MATCHER(onImplicitObjectArgument);
   REGISTER_MATCHER(opaqueValueExpr);
   REGISTER_MATCHER(optionally);
+  REGISTER_MATCHER(packExpansionExpr);
   REGISTER_MATCHER(parameterCountIs);
   REGISTER_MATCHER(parenExpr);
   REGISTER_MATCHER(parenListExpr);
Index: clang/lib/ASTMatchers/ASTMatchersInternal.cpp
===
--- clang/lib/ASTMatchers/ASTMatchersInternal.cpp
+++ clang/lib/ASTMatchers/ASTMatchersInternal.cpp
@@ -981,6 +981,7 @@
 predefinedExpr;
 const internal::VariadicDynCastAllOfMatcher
 designatedInitExpr;
+const internal::VariadicDynCastAllOfMatcher packExpansionExpr;
 const internal::VariadicOperatorMatcherFunc<
 2, std::numeric_limits::max()>
 eachOf = {internal::DynTypedMatcher::VO_EachOf};
Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -731,6 +731,27 @@
  

[PATCH] D124131: Thread safety analysis: Store capability kind in CapabilityExpr

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM with the extra safety measure added. I'm happy to review again if you'd 
like, but don't require it.




Comment at: clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h:277
+  /// The kind of capability as specified by @ref CapabilityAttr::getName.
+  StringRef CapKind;
+

aaronpuchert wrote:
> aaron.ballman wrote:
> > Hr, I think this may actually be safe, but it does give me pause to 
> > store a `StringRef` as anything other than a local (or param/return type): 
> > https://llvm.org/docs/ProgrammersManual.html#the-stringref-class
> > 
> > Can you double-check my thinking? This is fine because it's pulling the 
> > `StringRef` from the `CapabilityAttr`, and that stores its name internally 
> > and isn't released until the AST is destroyed.
> Exactly, aside from two places where I'm storing a string literal ("mutex" 
> and "wildcard"), the common case is taking the `StringRef` (as returned by 
> `CapabilityAttr::getName`) from the attribute. So that would be 
> lifetime-coupled to the AST and should be safe for our analysis.
> 
> Of course `StringRef` is implicitly constructible from other types, and we 
> wouldn't want that. Especially `std::string` would be an issue. Perhaps we 
> should prevent implicit conversions, and thus force the caller to pass a 
> `StringRef` glvalue, by overloading with a deleted template?
> ```
> template
> CapabilityExpr(const til::SExpr *, T, bool) = delete;
> ```
> Or maybe you've got a better idea.
> 
> As for a justification: we're building lots of `CapabilityExpr`s, essentially 
> every time we see a capability expression in the code (in attributes or as 
> capability type method call arguments). Given that the kind is only used for 
> actual warnings I wouldn't want us to spend of lot of time or memory on 
> storing it.
> 
> We could also store the original `Expr*` and extract on demand, but that 
> seemed to me antithetical to translating into the TIL. Of course it would be 
> slightly faster, but the current code (before this change) also extracts 
> capability names eagerly without waiting for the need to arise.
> Of course StringRef is implicitly constructible from other types, and we 
> wouldn't want that. Especially std::string would be an issue. Perhaps we 
> should prevent implicit conversions, and thus force the caller to pass a 
> StringRef glvalue, by overloading with a deleted template?

I think that's likely a good safety measure, good suggestion!

> As for a justification: 

Oh, I already thought it was well-motivated, I just wanted to make sure I 
wasn't wrong about the lack of lifetime issues. :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124131

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

[clang] 97a32d3 - [flang][driver] Add support for generating executables

[Andrzej Warzynski via cfe-commits]

Author: Andrzej Warzynski
Date: 2022-04-25T12:00:23Z
New Revision: 97a32d3e43fec35ea424e77c2940ee7e801a

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

LOG: [flang][driver] Add support for generating executables

This patch adds 2 missing items required for `flang-new` to be able to
generate executables:

1. The Fortran_main runtime library, which implements the main entry
   point into Fortran's `PROGRAM` in Flang,

2. Extra linker flags to include Fortran runtime libraries (e.g.
   Fortran_main).

Fortran_main is the bridge between object files generated by Flang and
the C runtime that takes care of program set-up at system-level. For
every Fortran `PROGRAM`, Flang generates the `_QQmain` function.
Fortran_main implements the C `main` function that simply calls
`_QQmain`.

Additionally, "/../lib" directory is added to the list of
search directories for libraries. This is where the required runtime
libraries are currently located. Note that this the case for the build
directory. We haven't considered installation directories/targets yet.

With this change, you can generate an executable that will print `hello,
world!` as follows:

```bash
$ cat hello.f95
PROGRAM HELLO
  write(*, *) "hello, world!"
END PROGRAM HELLO
$ flang-new -flang-experimental-exec hello.f95
./a.out
hello, world!
```

NOTE 1: Fortran_main has to be a static library at all times. It invokes
`_QQmain`, which is the main entry point generated by Flang for the
given input file (you can check this with `flang-new -S hello.f95 -o - |
grep "Qmain"`). This means that Fortran_main has an unresolved
dependency at build time. The linker will allow this for a static
library. However, if Fortran_main was a shared object, then the linker
will produce an error: `undefined symbol: `_QQmain`.

NOTE 2: When Fortran runtime libraries are generated as shared libraries
(excluding Fortran_main, which is always static), you will need to
tell the dynamic linker (by e.g. tweaking LD_LIBRARY_PATH) where to look
for them when invoking the executables. For example:
```bash
LD_LIBRARY_PATH=$LD_LIBRARY_PATH:/lib/ ./a.out
```

NOTE 3: This feature is considered experimental and currently guarded
with a flag: `-flang-experimental-exec`.

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

[1] https://github.com/flang-compiler/f18-llvm-project

CREDITS: Fortran_main was originally written by Eric Schweitz, Jean
Perier, Peter Klausler and Steve Scalpone in the fir-dev` branch in [1].

Co-authored-by: Eric Schweitz 
Co-authored-by: Peter Klausler 
Co-authored-by: Jean Perier 
Co-authored-by: Steve Scalpone , Flags<[NoXarchOption, 
CoreOption]>,
 def fno_sycl : Flag<["-"], "fno-sycl">, Flags<[NoXarchOption, CoreOption]>,
   Group, HelpText<"Disables SYCL kernels compilation for device">;
 
+//===--===//
+// FLangOption + NoXarchOption
+//===--===//
+
+def flang_experimental_exec : Flag<["-"], "flang-experimental-exec">,
+  Flags<[FlangOption, FlangOnlyOption, NoXarchOption, HelpHidden]>,
+  HelpText<"Enable support for generating executables (experimental)">;
+
 
//===--===//
 // FLangOption + CoreOption + NoXarchOption
 
//===--===//

diff  --git a/clang/lib/Driver/ToolChains/Gnu.cpp 
b/clang/lib/Driver/ToolChains/Gnu.cpp
index 5816b91994102..e0fcabc3e250a 100644
--- a/clang/lib/Driver/ToolChains/Gnu.cpp
+++ b/clang/lib/Driver/ToolChains/Gnu.cpp
@@ -382,6 +382,28 @@ void tools::gnutools::StaticLibTool::ConstructJob(
  Exec, CmdArgs, Inputs, Output));
 }
 
+static void addFortranRuntimeLibraryPath(const ToolChain &TC,
+ const ArgList &Args,
+ ArgStringList &CmdArgs) {
+  // Default to the /../lib directory. This works fine on the
+  // platforms that we have tested so far. We will probably have to re-fine
+  // this in the future. In particular:
+  //* on some platforms, we may need to use lib64 instead of lib
+  //* this logic should also work on other similar platforms too, so we
+  //should move it to one of Gnu's parent tool{chain} classes
+  SmallString<256> DefaultLibPath =
+  llvm::sys::path::parent_path(TC.getDriver().Dir);
+  llvm::sys::path::append(DefaultLibPath, "lib");
+  CmdArgs.push_back(Args.MakeArgString("-L" + DefaultLibPath));
+}
+
+static void addFortranLinkerFlags(ArgStringList &CmdArgs) {
+  CmdArgs.push_back("-lFortran_main");
+  CmdArgs.push_back("-lFortranRuntime");
+  CmdArgs.push_back("-lFortranDecimal");
+  CmdArgs.push_back("-lm");
+}
+
 void tools::gnutools::Linker::ConstructJob(Compilation &

[PATCH] D122008: [flang][driver] Add support for generating executables

[Andrzej Warzynski via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG97a32d3e43fe: [flang][driver] Add support for generating 
executables (authored by awarzynski).

Changed prior to commit:
  https://reviews.llvm.org/D122008?vs=422841&id=424874#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122008

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Gnu.cpp
  flang/include/flang/Runtime/stop.h
  flang/runtime/CMakeLists.txt
  flang/runtime/FortranMain/CMakeLists.txt
  flang/runtime/FortranMain/Fortran_main.c
  flang/test/CMakeLists.txt
  flang/test/Driver/driver-help-hidden.f90
  flang/test/Driver/linker-flags.f90
  flang/tools/flang-driver/CMakeLists.txt

Index: flang/tools/flang-driver/CMakeLists.txt
===
--- flang/tools/flang-driver/CMakeLists.txt
+++ flang/tools/flang-driver/CMakeLists.txt
@@ -13,6 +13,14 @@
 add_flang_tool(flang-new
   driver.cpp
   fc1_main.cpp
+
+  DEPENDS
+  # These libraries are used in the linker invocation generated by the driver
+  # (i.e. when constructing the linker job). Without them the driver would be
+  # unable to generate executables.
+  FortranRuntime
+  FortranDecimal
+  Fortran_main
 )
 
 target_link_libraries(flang-new
Index: flang/test/Driver/linker-flags.f90
===
--- /dev/null
+++ flang/test/Driver/linker-flags.f90
@@ -0,0 +1,31 @@
+! Verify that the Fortran runtime libraries are present in the linker
+! invocation. These libraries are added on top of other standard runtime
+! libraries that the Clang driver will include.
+
+! NOTE: The additional linker flags tested here are currently specified in
+! clang/lib/Driver/Toolchains/Gnu.cpp. This makes the current implementation GNU
+! (Linux) specific. The following line will make sure that this test is skipped
+! on Windows. Ideally we should find a more robust way of testing this.
+! REQUIRES: shell
+! UNSUPPORTED: darwin, macos, system-windows
+
+!
+! RUN COMMAND
+!
+! Use `--ld-path` so that the linker location (used in the LABEL below) is deterministic.
+! RUN: %flang -### -flang-experimental-exec --ld-path=/usr/bin/ld %S/Inputs/hello.f90 2>&1 | FileCheck %s
+
+!
+! EXPECTED OUTPUT
+!
+! Compiler invocation to generate the object file
+! CHECK-LABEL: {{.*}} "-emit-obj"
+! CHECK-SAME:  "-o" "[[object_file:.*]]" {{.*}}Inputs/hello.f90
+
+! Linker invocation to generate the executable
+! CHECK-LABEL:  "/usr/bin/ld"
+! CHECK-SAME: "[[object_file]]"
+! CHECK-SAME: -lFortran_main
+! CHECK-SAME: -lFortranRuntime
+! CHECK-SAME: -lFortranDecimal
+! CHECK-SAME: -lm
Index: flang/test/Driver/driver-help-hidden.f90
===
--- flang/test/Driver/driver-help-hidden.f90
+++ flang/test/Driver/driver-help-hidden.f90
@@ -38,6 +38,8 @@
 ! CHECK-NEXT: -finput-charset= Specify the default character set for source files
 ! CHECK-NEXT: -fintrinsic-modules-path 
 ! CHECK-NEXT:Specify where to find the compiled intrinsic modules
+! CHECK-NEXT: -flang-experimental-exec
+! CHECK-NEXT:Enable support for generating executables (experimental)
 ! CHECK-NEXT: -flarge-sizes  Use INTEGER(KIND=8) for the result type in size-related intrinsics
 ! CHECK-NEXT: -flogical-abbreviations Enable logical abbreviations
 ! CHECK-NEXT: -fno-automatic Implies the SAVE attribute for non-automatic local objects in subprograms unless RECURSIVE
Index: flang/test/CMakeLists.txt
===
--- flang/test/CMakeLists.txt
+++ flang/test/CMakeLists.txt
@@ -58,6 +58,9 @@
   llvm-dis
   llvm-objdump
   split-file
+  FortranRuntime
+  Fortran_main
+  FortranDecimal
 )
 
 if (FLANG_INCLUDE_TESTS)
Index: flang/runtime/FortranMain/Fortran_main.c
===
--- /dev/null
+++ flang/runtime/FortranMain/Fortran_main.c
@@ -0,0 +1,21 @@
+//===-- runtime/FortranMain/Fortran_main.c ===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "flang/Runtime/main.h"
+#include "flang/Runtime/stop.h"
+
+/* main entry into PROGRAM */
+void _QQmain();
+
+/* C main stub */
+int main(int argc, const char *argv[], const char *envp[]) {
+  RTNAME(ProgramStart)(argc, argv, envp);
+  _QQmain();
+  RTNAME(ProgramEndStatement)();
+  return 0;
+}
Index: flang/runtime/FortranMain/CMakeLists.txt
===

[PATCH] D124348: [1/2][RISCV]Add Intrinsics for B extension in Clang

[Chang Hu via Phabricator via cfe-commits]

joker881 added a comment.

In D124348#3470612 , @craig.topper 
wrote:

> The "B extension" terminology no longer exists.

Thank you for your comments. I will consider them carefully. And I want to know 
that do you have any documents about intrinsic of bitmanip extension, like 
RISC-V Vector Extension Intrinsic Document.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124348

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

[clang] 00f0c80 - [Frontend] shrink in-memory PCH buffers to fit

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2022-04-25T14:31:14+02:00
New Revision: 00f0c805ff7c0b5780d651d273899abe977fdca7

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

LOG: [Frontend] shrink in-memory PCH buffers to fit

After building a PCH, the vector capacity is on average ~1/3 unused.
If we're going to keep it in memory for a while, reallocate to the right size.
Take care to do this once clang is destroyed so that we can reuse its
memory rather than requesting more.

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

Added: 


Modified: 
clang/lib/Frontend/PrecompiledPreamble.cpp

Removed: 




diff  --git a/clang/lib/Frontend/PrecompiledPreamble.cpp 
b/clang/lib/Frontend/PrecompiledPreamble.cpp
index e8128e3828edd..341ea6121637d 100644
--- a/clang/lib/Frontend/PrecompiledPreamble.cpp
+++ b/clang/lib/Frontend/PrecompiledPreamble.cpp
@@ -374,6 +374,15 @@ class PrecompiledPreamble::PCHStorage {
 return StringRef(Memory->Data.data(), Memory->Data.size());
   }
 
+  // Shrink in-memory buffers to fit.
+  // This incurs a copy, but preambles tend to be long-lived.
+  // Only safe to call once nothing can alias the buffer.
+  void shrink() {
+if (!Memory)
+  return;
+Memory->Data = decltype(Memory->Data)(Memory->Data);
+  }
+
 private:
   PCHStorage() = default;
   PCHStorage(const PCHStorage &) = delete;
@@ -520,7 +529,7 @@ llvm::ErrorOr 
PrecompiledPreamble::Build(
 
   if (!Act->hasEmittedPreamblePCH())
 return BuildPreambleError::CouldntEmitPCH;
-  Act.reset(); // Frees the PCH buffer frees, unless Storage keeps it in 
memory.
+  Act.reset(); // Frees the PCH buffer, unless Storage keeps it in memory.
 
   // Keep track of all of the files that the source manager knows about,
   // so we can verify whether they have changed or not.
@@ -545,6 +554,11 @@ llvm::ErrorOr 
PrecompiledPreamble::Build(
 }
   }
 
+  // Shrinking the storage requires extra temporary memory.
+  // Destroying clang first reduces peak memory usage.
+  CICleanup.unregister();
+  Clang.reset();
+  Storage->shrink();
   return PrecompiledPreamble(
   std::move(Storage), std::move(PreambleBytes), PreambleEndsAtStartOfLine,
   std::move(FilesInPreamble), std::move(MissingFiles));



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

[PATCH] D124242: [Frontend] shrink in-memory PCH buffers to fit

[Sam McCall via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG00f0c805ff7c: [Frontend] shrink in-memory PCH buffers to fit 
(authored by sammccall).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124242

Files:
  clang/lib/Frontend/PrecompiledPreamble.cpp


Index: clang/lib/Frontend/PrecompiledPreamble.cpp
===
--- clang/lib/Frontend/PrecompiledPreamble.cpp
+++ clang/lib/Frontend/PrecompiledPreamble.cpp
@@ -374,6 +374,15 @@
 return StringRef(Memory->Data.data(), Memory->Data.size());
   }
 
+  // Shrink in-memory buffers to fit.
+  // This incurs a copy, but preambles tend to be long-lived.
+  // Only safe to call once nothing can alias the buffer.
+  void shrink() {
+if (!Memory)
+  return;
+Memory->Data = decltype(Memory->Data)(Memory->Data);
+  }
+
 private:
   PCHStorage() = default;
   PCHStorage(const PCHStorage &) = delete;
@@ -520,7 +529,7 @@
 
   if (!Act->hasEmittedPreamblePCH())
 return BuildPreambleError::CouldntEmitPCH;
-  Act.reset(); // Frees the PCH buffer frees, unless Storage keeps it in 
memory.
+  Act.reset(); // Frees the PCH buffer, unless Storage keeps it in memory.
 
   // Keep track of all of the files that the source manager knows about,
   // so we can verify whether they have changed or not.
@@ -545,6 +554,11 @@
 }
   }
 
+  // Shrinking the storage requires extra temporary memory.
+  // Destroying clang first reduces peak memory usage.
+  CICleanup.unregister();
+  Clang.reset();
+  Storage->shrink();
   return PrecompiledPreamble(
   std::move(Storage), std::move(PreambleBytes), PreambleEndsAtStartOfLine,
   std::move(FilesInPreamble), std::move(MissingFiles));


Index: clang/lib/Frontend/PrecompiledPreamble.cpp
===
--- clang/lib/Frontend/PrecompiledPreamble.cpp
+++ clang/lib/Frontend/PrecompiledPreamble.cpp
@@ -374,6 +374,15 @@
 return StringRef(Memory->Data.data(), Memory->Data.size());
   }
 
+  // Shrink in-memory buffers to fit.
+  // This incurs a copy, but preambles tend to be long-lived.
+  // Only safe to call once nothing can alias the buffer.
+  void shrink() {
+if (!Memory)
+  return;
+Memory->Data = decltype(Memory->Data)(Memory->Data);
+  }
+
 private:
   PCHStorage() = default;
   PCHStorage(const PCHStorage &) = delete;
@@ -520,7 +529,7 @@
 
   if (!Act->hasEmittedPreamblePCH())
 return BuildPreambleError::CouldntEmitPCH;
-  Act.reset(); // Frees the PCH buffer frees, unless Storage keeps it in memory.
+  Act.reset(); // Frees the PCH buffer, unless Storage keeps it in memory.
 
   // Keep track of all of the files that the source manager knows about,
   // so we can verify whether they have changed or not.
@@ -545,6 +554,11 @@
 }
   }
 
+  // Shrinking the storage requires extra temporary memory.
+  // Destroying clang first reduces peak memory usage.
+  CICleanup.unregister();
+  Clang.reset();
+  Storage->shrink();
   return PrecompiledPreamble(
   std::move(Storage), std::move(PreambleBytes), PreambleEndsAtStartOfLine,
   std::move(FilesInPreamble), std::move(MissingFiles));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D124382: [Clang] Recognize target address space in superset calculation

[Jakub Chlanda via Phabricator via cfe-commits]

jchlanda created this revision.
Herald added subscribers: kerbowa, Anastasia, jvesely.
Herald added a project: All.
jchlanda requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Use target's address space map to handle cases when both language and
target address spaces are provided. In such case, attempt language to
target translation and only then perform the calculation.
The main motivation is to be able to use language address spaces as
inputs for builtins, which are defined in terms of target address space
(as discussed here: https://reviews.llvm.org/D112718) and hence the
definition of builtins with generic address space pointers that would
allow any other address space pointers inputs (bar constant).

This patch attempts to find a happy medium between not recognising target
address spaces at all (current state) and allowing all uses of it, based on
the assumption that users must know better. What it does not to is to
provide a bidirectional translation mechanism, which I'm not sure could ever
be done, with the current address space implementation (use of 0, the value
of default, etc).

Based on OpenCL rules, this patch follows the conversion guidelines for
`generic` and `constant` address space pointers as described here:
https://www.khronos.org/registry/OpenCL/specs/2.2/html/OpenCL_API.html#_memory_model


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D124382

Files:
  clang/include/clang/AST/Type.h
  clang/lib/Sema/SemaCast.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/address_space_type_casts_amdgpu.cl
  clang/test/Sema/address_space_type_casts_default.cl
  clang/test/SemaOpenCL/atomic-ops.cl
  clang/test/SemaOpenCL/numbered-address-space.cl
  clang/test/SemaOpenCL/predefined-expr.cl
  clang/test/SemaOpenCL/vector-conv.cl

Index: clang/test/SemaOpenCL/vector-conv.cl
===
--- clang/test/SemaOpenCL/vector-conv.cl
+++ clang/test/SemaOpenCL/vector-conv.cl
@@ -16,7 +16,8 @@
   e = (constant int4)i;
   e = (private int4)i;
 
-  private int4 *private_ptr = (const private int4 *)const_global_ptr; // expected-error{{casting 'const __global int4 *' to type 'const __private int4 *' changes address space of pointer}}
-  global int4 *global_ptr = const_global_ptr; // expected-warning {{initializing '__global int4 *__private' with an expression of type 'const __global int4 *__private' discards qualifiers}}
+private
+  int4 *private_ptr = (const private int4 *)const_global_ptr; // expected-error{{casting 'const __global int4 *' to type 'const __private int4 *' changes address space of pointer}}
+  global int4 *global_ptr = const_global_ptr;
   global_ptr = (global int4 *)const_global_ptr;
 }
Index: clang/test/SemaOpenCL/predefined-expr.cl
===
--- clang/test/SemaOpenCL/predefined-expr.cl
+++ clang/test/SemaOpenCL/predefined-expr.cl
@@ -2,7 +2,7 @@
 // RUN: %clang_cc1 %s -verify -cl-std=CL2.0
 
 void f() {
-  char *f1 = __func__;  //expected-error-re{{initializing '{{__generic|__private}} char *__private' with an expression of type 'const __constant char *' changes address space of pointer}}
-  constant char *f2 = __func__; //expected-warning{{initializing '__constant char *__private' with an expression of type 'const __constant char[2]' discards qualifiers}}
+  char *f1 = __func__; // expected-error-re{{initializing '{{__generic|__private}} char *__private' with an expression of type 'const __constant char *' changes address space of pointer}}
+  constant char *f2 = __func__;
   constant const char *f3 = __func__;
 }
Index: clang/test/SemaOpenCL/numbered-address-space.cl
===
--- clang/test/SemaOpenCL/numbered-address-space.cl
+++ clang/test/SemaOpenCL/numbered-address-space.cl
@@ -2,11 +2,16 @@
 // RUN: %clang_cc1 -cl-std=CL2.0 -triple amdgcn-amd-amdhsa -verify -pedantic -fsyntax-only %s
 
 void test_numeric_as_to_generic_implicit_cast(__attribute__((address_space(3))) int *as3_ptr, float src) {
-  generic int* generic_ptr = as3_ptr; // FIXME: This should error
+  generic int *generic_ptr = as3_ptr;
+}
+
+// AS 4 is constant on AMDGPU, casting it to generic is illegal.
+void test_numeric_as_const_to_generic_implicit_cast(__attribute__((address_space(4))) int *as4_ptr, float src) {
+  generic int *generic_ptr = as4_ptr; // expected-error{{initializing '__generic int *__private' with an expression of type '__attribute__((address_space(4))) int *__private' changes address space of pointer}}
 }
 
 void test_numeric_as_to_generic_explicit_cast(__attribute__((address_space(3))) int *as3_ptr, float src) {
-  generic int* generic_ptr = (generic int*) as3_ptr; // Should maybe be valid?
+  generic int *generic_ptr = (generic int *)as3_ptr;
 }
 
 void test_generic_to_numeric_as_implicit_cast(void) {
@@ -20,12 +25,12 @@
 }
 
 void test_g

[PATCH] D124382: [Clang] Recognize target address space in superset calculation

[Jakub Chlanda via Phabricator via cfe-commits]

jchlanda added a comment.

@tra @Naghasan @t4c1 you might find it interesting, a follow up from the 
discussion here: https://reviews.llvm.org/D112718


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124382

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

[PATCH] D124262: compile commands header to source heuristic lower-cases filenames before inferring file types

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Change looks good, thank you!

Can you add a test demonstrating it, and update the diff here?
Take a look at the `InterpolateTest` cases in 
`clang/unittests/Tooling/CompilationDatabaseTest.cpp`.

(If you can, please create diffs with context using `-U9` - this lets us 
see more context around the changed lines in phabricator)




Comment at: clang/lib/Tooling/InterpolatingCompilationDatabase.cpp:332
   Paths.emplace_back(Path, I);
-  Types.push_back(foldType(guessType(Path)));
+  Types.push_back(foldType(guessType(StringRef(OriginalPaths[I];
   Stems.emplace_back(sys::path::stem(Path), I);

nit: StringRef() isn't needed here, conversion will happen implicitly.

(The one on 329 is needed because the overload set of StringSaver::save is 
ambiguous)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124262

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

[PATCH] D123352: [analyzer] Add FixItHint to `nullability.NullReturnedFromNonnull` and `nullability.NullableReturnedFromNonnull`

[Moshe via Phabricator via cfe-commits]

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

Thanks for all the thoughtful and fantastic feedback!




Comment at: clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp:592-602
+  // If we're inside of an NS_ASSUME, then the sourceRange will end before the
+  // asterisk.
+  const auto CanonicalTypeSize = CanonicalTypeStr.size();
+  const bool IsInsideOfAssume =
+  NullabilityLoc == 
RetTypeLoc.getSourceRange().getBegin().getLocWithOffset(
+CanonicalTypeSize - 1);
+

steakhal wrote:
> Uh, this is really ugly. I don't believe this is the preferred way of 
> detecting this. @NoQ WDYT?
> Uh, this is really ugly.

It is really ugly. And a more correct implementation would probably handle some 
of the edge cases highlighted in the diff summary. It’s on me - being a n00b at 
llvm stuff. 

By “_this_” do you mean `IsInsideOfAssume` or `UseSyntaxSugar` in general?

> I don't believe this is the preferred way of detecting this. @NoQ WDYT?

I tried `isMacroExpansion` for assume nonnull, passing the end loc of the 
return type without success. 

A colleague suggested 
[SourceManager::isMacroBodyExpansion](https://clang.llvm.org/doxygen/classclang_1_1SourceManager.html#a04eadd011628b7c4cd3304712c8c221c).
 I’ll look at it again later today. 

Thank you so much for your patience and direction with reviews as I work on 
this. I really appreciate you making time for me!



Comment at: clang/test/Analysis/nullability-fixits.mm:5-10
+// RUN: %clang_cc1 -analyze \
+
+// RUN: -Wno-nonnull \
+// RUN: 
-analyzer-checker=core,nullability.NullableReturnedFromNonnull,nullability.NullReturnedFromNonnull
 \
+// RUN: -fdiagnostics-parseable-fixits %s 2>%t
+// RUN: cat %t | FileCheck %s -check-prefix=CHECK-FIXIT-DISABLED-IMPLICIT

steakhal wrote:
> Why don't you use the `%clang_analyze_cc1 ...` form instead or even better 
> the `%check_analyzer_fixit` tool-subst pattern?
> See the `clang/test/Analysis/dead-stores.c` as an example.
That’s a good call out. I looked at that exact test and was having trouble 
testing. Instead, I implemented the RUN commands from the most basic invocation 
I could find. 

I’m happy to update the tests that verify that fixits are emitted. For the one 
that checks that fixits are applied, is there a better way? The `-verify` flag 
doesn’t do that, from what I saw in the docs. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123352

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

[PATCH] D124221: Add new builtin __builtin_reflect_struct.

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D124221#3470550 , @aaron.ballman 
wrote:

> In D124221#3469251 , @rsmith wrote:
>
>> In D124221#3468706 , 
>> @aaron.ballman wrote:
>>
>>> Thank you for looking into this! I've also thought that the 
>>> `__builtin_dump_struct` implementation hasn't been quite as robust as it 
>>> could be. However, the primary use case for that builtin has been for 
>>> kernel folks to debug when they have no access to an actual debugger -- it 
>>> being a super limited interface is actually a feature rather than a bug, in 
>>> some ways.
>>
>> I'm more concerned about the design of `__builtin_dump_struct` than the 
>> implementation -- we can always fix the implementation. Right now the design 
>> is hostile to any use other than the very simplest one where the function 
>> you give it is essentially `printf`. You can't even dump to a log file or to 
>> a string without having to fight the design of the builtin. And if you want 
>> anything other than the exact recursion and formatting choices that we've 
>> arbitrarily made, you're out of luck. And of course, if you're in C++, 
>> you'll want to be able to print out `std::string` and `std::optional` 
>> and similar types too, which are not supported by the current design at all. 
>> Perhaps this is precisely what a specific set of kernel folks want for a 
>> specific use case, but if the only purpose is to address that one use case, 
>> then I don't really see how we can justify keeping this builtin in-tree. So 
>> I think we should either expand the scope beyond the specific kernel folks 
>> or we should remove it.
>
> Yes, I agree that the current builtin is limited and not particularly well 
> designed for general use. I would be fine removing support for it. I'd be 
> happier augmenting it so it works for more use cases. However, I don't think 
> what you've got here so far "expands the scope" so much as "moves the 
> goalposts" -- if the replacement can't be used by the kernel folks, then 
> we're losing the primary use case we have for that builtin. However, it 
> sounds like you may have some ideas on that so we can make it more usable in 
> C.

In general I've not been a fan of `__builtin_dump_struct` (I wasn't paying 
attention when it got merged), but I was assured in the meantime that it was 
JUST for printf-debugging in places without a debugger.  I found its 
inflexibility, changing format, and inconsistent functionality to be a 
'feature' at that point, as it would discourage non-printf-debugging use cases. 
BUT I guess you're seeing Hyrum's law in action :D  It would be a shame to LOSE 
the builtin, since it is apparently REALLY useful in the debugging case, but I 
definitely don't think I would have +1'ed it if I were to see/understand it as 
a new addition.

>>> I think what you're designing here is straight-up reflection, which is a 
>>> different use case. If we're going to get something that's basically only 
>>> usable in C++, I'm not certain there's much value in adding builtins for 
>>> reflection until WG21 decides what reflection looks like, and then we can 
>>> design around that. (Personally, I think designing usable reflection 
>>> interfaces for C would be considerably harder but would provide 
>>> considerably more benefit to users given the lack of reflection 
>>> capabilities. There's almost no chance WG14 and WG21 would come up with the 
>>> same interfaces for reflection, so I think we've got some opportunity for 
>>> exploration here.)
>>
>> What I set out to build is something that lets you implement struct dumping 
>> without all the shortcomings I see in `__builtin_dump_struct`. I think it's 
>> inevitable that that ends up being substantially a struct reflection system, 
>> and indeed `__builtin_dump_struct` is a reflection system, too, just a 
>> really awkward one -- people have already started parsing its format strings 
>> to extract struct field names, for example. (The usage I've seen actually 
>> *is* struct dumping, but that usage can't use `__builtin_dump_struct` 
>> directly because of its limitations, so `__builtin_dump_struct` is just used 
>> to extract the field names, and the field values and types are extracted via 
>> structured bindings instead.)
>
> Agreed that the current builtin is effectively (really bad) reflection as 
> well, and I'm sad to hear people are trying to parse its output to get 
> structure field names (we have a JSON AST dump for exactly that kind of 
> situation, so I'm not super sympathetic to people using builtin dump struct 
> in that way unless they also need the runtime value information of the 
> fields).

I agree completely, over the weekend I also came to the conclusion that any 
attempt to do ANYTHING in this space is likely going to be used as a poor-man's 
reflection.  At that point, I consider the awkwardness to be a bit of a 

[PATCH] D122920: [Clang][CodeGen]Fix __builtin_dump_struct missing record type field name

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

@yihanaa : I'd suggest seeing the conversation that @rsmith @aaron.ballman and 
I are having about this builtin here: https://reviews.llvm.org/D124221

In general it looks like the three of us think that this builtin needs an 
overhaul in implementation/functionality in order to be further useful. While 
we very much appreciate your attempts to try to improve it, we believe there 
needs to be larger-scale changes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122920

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

[PATCH] D123182: [Concepts] Fix overload resolution bug with constrained candidates

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a subscriber: nkreeger.
erichkeane added a comment.

@nkreeger : Can you please explain your revert, both in the revert commit 
message (next time), as well as the patch so that the author/rest of us have 
SOME hint as to why it was reverted?  Frequent reverts make it painful as a 
downstream, and confusing as a reviewer/implementer as to the state of things.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123182

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

[PATCH] D124384: [clang] Fix a constant evaluator crash on a NULL-type expr.

[Haojian Wu via Phabricator via cfe-commits]

hokein created this revision.
hokein added a reviewer: sammccall.
Herald added a project: All.
hokein requested review of this revision.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D124384

Files:
  clang/lib/AST/ExprConstant.cpp
  clang/test/SemaCXX/recovery-expr-type.cpp


Index: clang/test/SemaCXX/recovery-expr-type.cpp
===
--- clang/test/SemaCXX/recovery-expr-type.cpp
+++ clang/test/SemaCXX/recovery-expr-type.cpp
@@ -151,3 +151,13 @@
 // Enumerators can be evaluated (they evaluate as zero, but we don't care).
 static_assert(Circular_A == 0 && Circular_A != 0, ""); // expected-error 
{{static_assert failed}}
 }
+
+namespace test14 {
+extern "C" void *memset(void *, int b, unsigned long) {
+  int * const c(undef()); // expected-error {{undeclared identifier}}
+  // Verify we do not crash on evaluating *c whose initializer is a NULL-type 
ParenListExpr!
+  memset(c, 0, *c); // crash1
+
+  b = __builtin_object_size(c, 0); // crash2
+}
+}
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -8631,7 +8631,7 @@
 return false;
 
   const Expr *Init = VD->getAnyInitializer();
-  if (!Init)
+  if (!Init || Init->getType().isNull())
 return false;
 
   const Expr *E = Init->IgnoreParens();


Index: clang/test/SemaCXX/recovery-expr-type.cpp
===
--- clang/test/SemaCXX/recovery-expr-type.cpp
+++ clang/test/SemaCXX/recovery-expr-type.cpp
@@ -151,3 +151,13 @@
 // Enumerators can be evaluated (they evaluate as zero, but we don't care).
 static_assert(Circular_A == 0 && Circular_A != 0, ""); // expected-error {{static_assert failed}}
 }
+
+namespace test14 {
+extern "C" void *memset(void *, int b, unsigned long) {
+  int * const c(undef()); // expected-error {{undeclared identifier}}
+  // Verify we do not crash on evaluating *c whose initializer is a NULL-type ParenListExpr!
+  memset(c, 0, *c); // crash1
+
+  b = __builtin_object_size(c, 0); // crash2
+}
+}
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -8631,7 +8631,7 @@
 return false;
 
   const Expr *Init = VD->getAnyInitializer();
-  if (!Init)
+  if (!Init || Init->getType().isNull())
 return false;
 
   const Expr *E = Init->IgnoreParens();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123182: [Concepts] Fix overload resolution bug with constrained candidates

[Nick Kreeger via Phabricator via cfe-commits]

nkreeger added a comment.

In D123182#3471661 , @erichkeane 
wrote:

> @nkreeger : Can you please explain your revert, both in the revert commit 
> message (next time), as well as the patch so that the author/rest of us have 
> SOME hint as to why it was reverted?  Frequent reverts make it painful as a 
> downstream, and confusing as a reviewer/implementer as to the state of things.

Apologies - I was reverting my commit and accidently had the wrong git-revert 
commit in my tree locally. I thought I had it cleaned up, but was wrong when I 
pushed. I ensured the tree was back in place with another revert. Please let me 
know if this is not the case. Sorry again :-(


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123182

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

[PATCH] D122663: Mark identifier prefixes as substitutable

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D122663#3470687 , @hvdijk wrote:

> In D122663#3457330 , @erichkeane 
> wrote:
>
>> LGTM!  I would like @rjmccall to take a pass if he ends up having time in 
>> the next day or two (perhaps tack on an extra day or two because of Easter), 
>> else I'll be willing to approve later in the week.
>
> ping, I did get feedback from @rsmith (much appreciated) and applied his 
> suggestions, but not from @rjmccall, would you be okay to approve it then?

Ping me EOW if @rsmith doesn't respond in the meantime.  It is also not clear 
to me whether you were able to capture/fix the issue he had with the 
clang-abi-compat.cpp test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122663

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

[PATCH] D122663: Mark identifier prefixes as substitutable

[Harald van Dijk via Phabricator via cfe-commits]

hvdijk added a comment.

In D122663#3471741 , @erichkeane 
wrote:

> Ping me EOW if @rsmith doesn't respond in the meantime.  It is also not clear 
> to me whether you were able to capture/fix the issue he had with the 
> clang-abi-compat.cpp test.

Will do, sure. For the record, for that test I applied @rsmith's suggestion 
verbatim so it *should* address his concern, but I will wait for him to confirm.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122663

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

[PATCH] D119544: Deferred Concept Instantiation Implementation

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: clang/test/SemaTemplate/concepts.cpp:391
+
+  CausesFriendConstraint CFC;
+  FriendFunc(CFC, 1);

ChuanqiXu wrote:
> erichkeane wrote:
> > erichkeane wrote:
> > > A bunch of the tests below this all fail.
> > See these two tests, which fail by saying that "::local is not a member of 
> > class 'X'".
> I've spent some time to look into this. I don't find the root cause now. But 
> I find that it is related to the wrong lookups. The error is emitted in 
> CheckQualifiedMemberReference. And it looks like we lookup for the original S 
> instead of the instantiated S. And the compiler thinks the 2 structs are 
> different. So here is the error. Do you have any other ideas?
I unfortunately had little time to work on this over the weekend, but right 
before I left on Friday realized that I think the idea of doing 
`RebuildExprInCurrentInstantiation` was the incorrect one.  It ATTEMPTS to 
rebuild things, but doesn't have enough information to do so (which is the case 
you're seeing I think?).  If I change that back to what I had before in every 
case, all except `CheckMemberVar::foo` fails (note `foo2` does not!).  

THAT I have tracked down to `TemplateInstantiator::TransformDecl` failing to 
actually transform the variable from `S` to `S` as you noticed.

It gets me to 3 cases where that function is called, 2 of which work.  
1- `CheckMemberVar::foo`: Fails
2- `CheckMemberVar::foo2`: Works
3- Similar example with the instantiation in the BODY of the func: Works

I am going to spend more time trying to look into that hopefully early this 
week.


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

https://reviews.llvm.org/D119544

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

[PATCH] D122663: Mark identifier prefixes as substitutable

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D122663#3471748 , @hvdijk wrote:

> In D122663#3471741 , @erichkeane 
> wrote:
>
>> Ping me EOW if @rsmith doesn't respond in the meantime.  It is also not 
>> clear to me whether you were able to capture/fix the issue he had with the 
>> clang-abi-compat.cpp test.
>
> Will do, sure. For the record, for that test I applied @rsmith's suggestion 
> verbatim so it *should* address his concern, but I will wait for him to 
> confirm.

I DID see that and am REASONABLY sure you do, but sometimes folks will ask for 
test coverage because they suspect the resulting behavior will demonstrate some 
sort of problem with the current patch.  I DID run this through the demangler 
and found that the PRE15 case looks odd?

`void test10(X::Y::a, X::Y::b, float*, X::Y)`
vs
`void test10(X::Y::a, X::Y::b, float*, float*)`

Or is that exactly the bug being caught here?  That is, is the `float*` 
substitution not being properly registered/emitted and being confused with 
`X::Y`?

PS:, and as an unrelated-lament... A while back I wrote a test regarding 
mangling where I ALSO had it run us through llvm-cxxfilt so that we had an 
output of every mangled string that showed the demangled version.  I wish we'd 
done that from the beginning to make it much less confusing what is going on in 
these.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122663

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

[PATCH] D123182: [Concepts] Fix overload resolution bug with constrained candidates

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D123182#3471687 , @nkreeger wrote:

> In D123182#3471661 , @erichkeane 
> wrote:
>
>> @nkreeger : Can you please explain your revert, both in the revert commit 
>> message (next time), as well as the patch so that the author/rest of us have 
>> SOME hint as to why it was reverted?  Frequent reverts make it painful as a 
>> downstream, and confusing as a reviewer/implementer as to the state of 
>> things.
>
> Apologies - I was reverting my commit and accidently had the wrong git-revert 
> commit in my tree locally. I thought I had it cleaned up, but was wrong when 
> I pushed. I ensured the tree was back in place with another revert. Please 
> let me know if this is not the case. Sorry again :-(

I see that now, thanks for the explanation! IN the future, even when reverting 
your OWN commit, it is appreciated if you explain the WHY to the revert so that 
it has some level of context.  Particularly when you show some sort of example 
of the failure (like a link to the broken bot, etc), as it helps the downstream 
test-failure-analysis a ton.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123182

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

[PATCH] D123182: [Concepts] Fix overload resolution bug with constrained candidates

[Nick Kreeger via Phabricator via cfe-commits]

nkreeger added a comment.

In D123182#3471789 , @erichkeane 
wrote:

> In D123182#3471687 , @nkreeger 
> wrote:
>
>> In D123182#3471661 , @erichkeane 
>> wrote:
>>
>>> @nkreeger : Can you please explain your revert, both in the revert commit 
>>> message (next time), as well as the patch so that the author/rest of us 
>>> have SOME hint as to why it was reverted?  Frequent reverts make it painful 
>>> as a downstream, and confusing as a reviewer/implementer as to the state of 
>>> things.
>>
>> Apologies - I was reverting my commit and accidently had the wrong 
>> git-revert commit in my tree locally. I thought I had it cleaned up, but was 
>> wrong when I pushed. I ensured the tree was back in place with another 
>> revert. Please let me know if this is not the case. Sorry again :-(
>
> I see that now, thanks for the explanation! IN the future, even when 
> reverting your OWN commit, it is appreciated if you explain the WHY to the 
> revert so that it has some level of context.  Particularly when you show some 
> sort of example of the failure (like a link to the broken bot, etc), as it 
> helps the downstream test-failure-analysis a ton.

Roger noted - apologies again I felt horrible!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123182

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

[PATCH] D122663: Mark identifier prefixes as substitutable

[Nathan Sidwell via Phabricator via cfe-commits]

urnathan added a comment.

[was out last week]  It seems the demangler already gets this right, probably 
because it just looks like a regular nested name


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122663

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

[PATCH] D122663: Mark identifier prefixes as substitutable

[Harald van Dijk via Phabricator via cfe-commits]

hvdijk added a comment.

In D122663#3471763 , @erichkeane 
wrote:

> I DID see that and am REASONABLY sure you do, but sometimes folks will ask 
> for test coverage because they suspect the resulting behavior will 
> demonstrate some sort of problem with the current patch.  I DID run this 
> through the demangler and found that the PRE15 case looks odd?
>
> `void test10(X::Y::a, X::Y::b, float*, X::Y)`
> vs
> `void test10(X::Y::a, X::Y::b, float*, float*)`
>
> Or is that exactly the bug being caught here?  That is, is the `float*` 
> substitution not being properly registered/emitted and being confused with 
> `X::Y`?

Yes, that is exactly it. That is what @rsmith was getting at with his comment 
about regarding this as two separate bugs (he can correct me if I am misstating 
things), one that we do not add the substitution and as such use wrong numbers 
for subsequent substitutions, the other that we do not use the substitution. 
The other is what I had covered in my initial test, his addition covers that 
first bug where we wrongly used S4 when we should be using S5.

> PS:, and as an unrelated-lament... A while back I wrote a test regarding 
> mangling where I ALSO had it run us through llvm-cxxfilt so that we had an 
> output of every mangled string that showed the demangled version.  I wish 
> we'd done that from the beginning to make it much less confusing what is 
> going on in these.

llvm-cxxfilt has no option to be compatible with earlier incorrect mangling 
though, so it would not help with this particular test. But it could help with 
other tests, agreed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122663

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

[PATCH] D124128: Thread safety analysis: Store CapabilityExprs in ScopedLockableFactEntry (NFC)

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert added inline comments.



Comment at: clang/lib/Analysis/ThreadSafety.cpp:911-919
+UnderlyingMutexes.push_back(UnderlyingCapability{M, UCK_Acquired});
   }
 
   void addExclusiveUnlock(const CapabilityExpr &M) {
-UnderlyingMutexes.emplace_back(M.sexpr(), UCK_ReleasedExclusive);
+UnderlyingMutexes.push_back(UnderlyingCapability{M, 
UCK_ReleasedExclusive});
   }
 

aaron.ballman wrote:
> I think we can continue to use `emplace_back()` here, can't we?
For `emplace_back` we need a constructor and can't just do aggregate 
initialization. But I could omit the explicit type, like 
`UnderlyingMutexes.push_back({M, UCK_*})`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124128

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

[PATCH] D124389: [clang][NFC] Inclusive language: remove use of Whitelist in clang/lib/Analysis/

[Quinn Pham via Phabricator via cfe-commits]

quinnp created this revision.
Herald added a project: All.
quinnp requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

[NFC] As part of using inclusive language within the llvm project, this patch
rewords a comment to replace Whitelist with Allowlist in
`RetainSummaryManager.cpp`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D124389

Files:
  clang/lib/Analysis/RetainSummaryManager.cpp


Index: clang/lib/Analysis/RetainSummaryManager.cpp
===
--- clang/lib/Analysis/RetainSummaryManager.cpp
+++ clang/lib/Analysis/RetainSummaryManager.cpp
@@ -398,7 +398,7 @@
   } else if (FName.startswith("NSLog")) {
 return getDoNothingSummary();
   } else if (FName.startswith("NS") && FName.contains("Insert")) {
-// Whitelist NSXXInsertXX, for example NSMapInsertIfAbsent, since they can
+// Allowlist NSXXInsertXX, for example NSMapInsertIfAbsent, since they can
 // be deallocated by NSMapRemove. (radar://11152419)
 ScratchArgs = AF.add(ScratchArgs, 1, ArgEffect(StopTracking));
 ScratchArgs = AF.add(ScratchArgs, 2, ArgEffect(StopTracking));


Index: clang/lib/Analysis/RetainSummaryManager.cpp
===
--- clang/lib/Analysis/RetainSummaryManager.cpp
+++ clang/lib/Analysis/RetainSummaryManager.cpp
@@ -398,7 +398,7 @@
   } else if (FName.startswith("NSLog")) {
 return getDoNothingSummary();
   } else if (FName.startswith("NS") && FName.contains("Insert")) {
-// Whitelist NSXXInsertXX, for example NSMapInsertIfAbsent, since they can
+// Allowlist NSXXInsertXX, for example NSMapInsertIfAbsent, since they can
 // be deallocated by NSMapRemove. (radar://11152419)
 ScratchArgs = AF.add(ScratchArgs, 1, ArgEffect(StopTracking));
 ScratchArgs = AF.add(ScratchArgs, 2, ArgEffect(StopTracking));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D124262: compile commands header to source heuristic lower-cases filenames before inferring file types

[Ishaan Gandhi via Phabricator via cfe-commits]

ishaangandhi updated this revision to Diff 424904.
ishaangandhi added a comment.

- Added a test case
- Removed redundant "StringRef" constructor


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124262

Files:
  clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
  clang/unittests/Tooling/CompilationDatabaseTest.cpp


Index: clang/unittests/Tooling/CompilationDatabaseTest.cpp
===
--- clang/unittests/Tooling/CompilationDatabaseTest.cpp
+++ clang/unittests/Tooling/CompilationDatabaseTest.cpp
@@ -845,6 +845,14 @@
   EXPECT_EQ(getProxy("foo/bar/baz/shout.C"), "FOO/BAR/BAZ/SHOUT.cc");
 }
 
+TEST_F(InterpolateTest, LanguagePreference) {
+  add("foo/bar/baz/exact.C");
+  add("foo/bar/baz/exact.cpp");
+  add("other/random/path.cpp");
+  // Proxies for ".H" files are ".C" files, and not ".cpp files"
+  EXPECT_EQ(getProxy("foo/bar/baz/exact.H"), "foo/bar/baz/exact.C");
+}
+
 TEST_F(InterpolateTest, Aliasing) {
   add("foo.cpp", "-faligned-new");
 
Index: clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
===
--- clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
+++ clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
@@ -329,7 +329,7 @@
   StringRef Path = Strings.save(StringRef(OriginalPaths[I]).lower());
 
   Paths.emplace_back(Path, I);
-  Types.push_back(foldType(guessType(StringRef(OriginalPaths[I];
+  Types.push_back(foldType(guessType(OriginalPaths[I])));
   Stems.emplace_back(sys::path::stem(Path), I);
   auto Dir = ++sys::path::rbegin(Path), DirEnd = sys::path::rend(Path);
   for (int J = 0; J < DirectorySegmentsIndexed && Dir != DirEnd; ++J, 
++Dir)


Index: clang/unittests/Tooling/CompilationDatabaseTest.cpp
===
--- clang/unittests/Tooling/CompilationDatabaseTest.cpp
+++ clang/unittests/Tooling/CompilationDatabaseTest.cpp
@@ -845,6 +845,14 @@
   EXPECT_EQ(getProxy("foo/bar/baz/shout.C"), "FOO/BAR/BAZ/SHOUT.cc");
 }
 
+TEST_F(InterpolateTest, LanguagePreference) {
+  add("foo/bar/baz/exact.C");
+  add("foo/bar/baz/exact.cpp");
+  add("other/random/path.cpp");
+  // Proxies for ".H" files are ".C" files, and not ".cpp files"
+  EXPECT_EQ(getProxy("foo/bar/baz/exact.H"), "foo/bar/baz/exact.C");
+}
+
 TEST_F(InterpolateTest, Aliasing) {
   add("foo.cpp", "-faligned-new");
 
Index: clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
===
--- clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
+++ clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
@@ -329,7 +329,7 @@
   StringRef Path = Strings.save(StringRef(OriginalPaths[I]).lower());
 
   Paths.emplace_back(Path, I);
-  Types.push_back(foldType(guessType(StringRef(OriginalPaths[I];
+  Types.push_back(foldType(guessType(OriginalPaths[I])));
   Stems.emplace_back(sys::path::stem(Path), I);
   auto Dir = ++sys::path::rbegin(Path), DirEnd = sys::path::rend(Path);
   for (int J = 0; J < DirectorySegmentsIndexed && Dir != DirEnd; ++J, ++Dir)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D124262: compile commands header to source heuristic lower-cases filenames before inferring file types

[Ishaan Gandhi via Phabricator via cfe-commits]

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

Test case added, re-diffed with `-U`, and removed redundant constructor. 
Thanks for the quick feedback, @sammccall!

(I didn't wait for the tests to run locally, I am hoping to use your CI systems 
to do that. It seems like a simple enough of a test that I hope I can get it 
right on the first try.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124262

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

[PATCH] D124262: compile commands header to source heuristic lower-cases filenames before inferring file types

[Ishaan Gandhi via Phabricator via cfe-commits]

ishaangandhi updated this revision to Diff 424906.

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

https://reviews.llvm.org/D124262

Files:
  clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
  clang/unittests/Tooling/CompilationDatabaseTest.cpp


Index: clang/unittests/Tooling/CompilationDatabaseTest.cpp
===
--- clang/unittests/Tooling/CompilationDatabaseTest.cpp
+++ clang/unittests/Tooling/CompilationDatabaseTest.cpp
@@ -845,6 +845,14 @@
   EXPECT_EQ(getProxy("foo/bar/baz/shout.C"), "FOO/BAR/BAZ/SHOUT.cc");
 }
 
+TEST_F(InterpolateTest, LanguagePreference) {
+  add("foo/bar/baz/exact.C");
+  add("foo/bar/baz/exact.cpp");
+  add("other/random/path.cpp");
+  // Proxies for ".H" files are ".C" files, and not ".cpp files"
+  EXPECT_EQ(getProxy("foo/bar/baz/exact.H"), "foo/bar/baz/exact.C");
+}
+
 TEST_F(InterpolateTest, Aliasing) {
   add("foo.cpp", "-faligned-new");
 
Index: clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
===
--- clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
+++ clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
@@ -329,7 +329,7 @@
   StringRef Path = Strings.save(StringRef(OriginalPaths[I]).lower());
 
   Paths.emplace_back(Path, I);
-  Types.push_back(foldType(guessType(Path)));
+  Types.push_back(foldType(guessType(OriginalPaths[I])));
   Stems.emplace_back(sys::path::stem(Path), I);
   auto Dir = ++sys::path::rbegin(Path), DirEnd = sys::path::rend(Path);
   for (int J = 0; J < DirectorySegmentsIndexed && Dir != DirEnd; ++J, 
++Dir)


Index: clang/unittests/Tooling/CompilationDatabaseTest.cpp
===
--- clang/unittests/Tooling/CompilationDatabaseTest.cpp
+++ clang/unittests/Tooling/CompilationDatabaseTest.cpp
@@ -845,6 +845,14 @@
   EXPECT_EQ(getProxy("foo/bar/baz/shout.C"), "FOO/BAR/BAZ/SHOUT.cc");
 }
 
+TEST_F(InterpolateTest, LanguagePreference) {
+  add("foo/bar/baz/exact.C");
+  add("foo/bar/baz/exact.cpp");
+  add("other/random/path.cpp");
+  // Proxies for ".H" files are ".C" files, and not ".cpp files"
+  EXPECT_EQ(getProxy("foo/bar/baz/exact.H"), "foo/bar/baz/exact.C");
+}
+
 TEST_F(InterpolateTest, Aliasing) {
   add("foo.cpp", "-faligned-new");
 
Index: clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
===
--- clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
+++ clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
@@ -329,7 +329,7 @@
   StringRef Path = Strings.save(StringRef(OriginalPaths[I]).lower());
 
   Paths.emplace_back(Path, I);
-  Types.push_back(foldType(guessType(Path)));
+  Types.push_back(foldType(guessType(OriginalPaths[I])));
   Stems.emplace_back(sys::path::stem(Path), I);
   auto Dir = ++sys::path::rbegin(Path), DirEnd = sys::path::rend(Path);
   for (int J = 0; J < DirectorySegmentsIndexed && Dir != DirEnd; ++J, ++Dir)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D122920: [Clang][CodeGen]Fix __builtin_dump_struct missing record type field name

[Wang Yihan via Phabricator via cfe-commits]

yihanaa added a comment.

In D122920#3471654 , @erichkeane 
wrote:

> @yihanaa : I'd suggest seeing the conversation that @rsmith @aaron.ballman 
> and I are having about this builtin here: https://reviews.llvm.org/D124221
>
> In general it looks like the three of us think that this builtin needs an 
> overhaul in implementation/functionality in order to be further useful. While 
> we very much appreciate your attempts to try to improve it, we believe there 
> needs to be larger-scale changes.

Thanks for your reply, I would like to get involved and help if needed


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122920

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

[PATCH] D122663: Mark identifier prefixes as substitutable

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

> llvm-cxxfilt has no option to be compatible with earlier incorrect mangling 
> though, so it would not help with this particular test. But it could help 
> with other tests, agreed.

This actually wouldn't be necessary in this case: cxxfilt is already 'right', 
this is just for humans to be able to see "we changed this from mangling as 
 to ".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122663

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

[PATCH] D124104: [clang][dataflow] Fix `Environment::join`'s handling of flow condition merging

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

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

address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124104

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

Index: clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
@@ -10,6 +10,7 @@
 #include "TestingSupport.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/ExprCXX.h"
+#include "clang/AST/Type.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/Analysis/CFG.h"
@@ -295,6 +296,164 @@
   // FIXME: Called functions at point `p` should contain only "foo".
 }
 
+// Models an analysis that uses flow conditions.
+class SpecialBoolAnalysis
+: public DataflowAnalysis {
+public:
+  explicit SpecialBoolAnalysis(ASTContext &Context)
+  : DataflowAnalysis(Context) {}
+
+  static NoopLattice initialElement() { return {}; }
+
+  void transfer(const Stmt *S, NoopLattice &, Environment &Env) {
+auto SpecialBoolRecordDecl = recordDecl(hasName("SpecialBool"));
+auto HasSpecialBoolType = hasType(SpecialBoolRecordDecl);
+
+if (const auto *E = selectFirst(
+"call", match(cxxConstructExpr(HasSpecialBoolType).bind("call"), *S,
+  getASTContext( {
+  auto &ConstructorVal = *cast(Env.createValue(E->getType()));
+  ConstructorVal.setProperty("is_set", Env.getBoolLiteralValue(false));
+  Env.setValue(*Env.getStorageLocation(*E, SkipPast::None), ConstructorVal);
+} else if (const auto *E = selectFirst(
+   "call", match(cxxMemberCallExpr(callee(cxxMethodDecl(ofClass(
+   SpecialBoolRecordDecl
+ .bind("call"),
+ *S, getASTContext( {
+  auto *Object = E->getImplicitObjectArgument();
+  assert(Object != nullptr);
+
+  auto *ObjectLoc =
+  Env.getStorageLocation(*Object, SkipPast::ReferenceThenPointer);
+  assert(ObjectLoc != nullptr);
+
+  auto &ConstructorVal =
+  *cast(Env.createValue(Object->getType()));
+  ConstructorVal.setProperty("is_set", Env.getBoolLiteralValue(true));
+  Env.setValue(*ObjectLoc, ConstructorVal);
+}
+  }
+
+  bool compareEquivalent(QualType Type, const Value &Val1,
+ const Environment &Env1, const Value &Val2,
+ const Environment &Env2) final {
+const auto *Decl = Type->getAsCXXRecordDecl();
+if (Decl == nullptr || Decl->getIdentifier() == nullptr ||
+Decl->getName() != "SpecialBool")
+  return false;
+
+auto *IsSet1 = cast_or_null(
+cast(&Val1)->getProperty("is_set"));
+if (IsSet1 == nullptr)
+  return true;
+
+auto *IsSet2 = cast_or_null(
+cast(&Val2)->getProperty("is_set"));
+if (IsSet2 == nullptr)
+  return false;
+
+return IsSet1 == IsSet2 || Env1.flowConditionImplies(*IsSet1) ==
+   Env2.flowConditionImplies(*IsSet2);
+  }
+
+  // Always returns `true` to accept the `MergedVal`.
+  bool merge(QualType Type, const Value &Val1, const Environment &Env1,
+ const Value &Val2, const Environment &Env2, Value &MergedVal,
+ Environment &MergedEnv) final {
+const auto *Decl = Type->getAsCXXRecordDecl();
+if (Decl == nullptr || Decl->getIdentifier() == nullptr ||
+Decl->getName() != "SpecialBool")
+  return true;
+
+auto *IsSet1 = cast_or_null(
+cast(&Val1)->getProperty("is_set"));
+if (IsSet1 == nullptr)
+  return true;
+
+auto *IsSet2 = cast_or_null(
+cast(&Val2)->getProperty("is_set"));
+if (IsSet2 == nullptr)
+  return true;
+
+auto &IsSet = MergedEnv.makeAtomicBoolValue();
+cast(&MergedVal)->setProperty("is_set", IsSet);
+if (Env1.flowConditionImplies(*IsSet1) &&
+Env2.flowConditionImplies(*IsSet2))
+  MergedEnv.addToFlowCondition(IsSet);
+
+return true;
+  }
+};
+
+class JoinFlowConditionsTest : public Test {
+protected:
+  template 
+  void runDataflow(llvm::StringRef Code, Matcher Match) {
+ASSERT_THAT_ERROR(
+test::checkDataflow(
+Code, "target",
+[](ASTContext &Context, Environment &Env) {
+  return SpecialBoolAnalysis(Context);
+},
+[&Match](
+llvm::ArrayRef<
+std::pair>>
+Results,
+ASTContext &ASTCtx) { Match(Results, ASTCtx); },
+{"-fsyntax-only", "-std=c++17"}),
+

[PATCH] D124104: [clang][dataflow] Fix `Environment::join`'s handling of flow condition merging

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

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



Comment at: 
clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp:356
+
+return V1 == V2 ||
+   Env1.flowConditionImplies(*V1) == Env2.flowConditionImplies(*V2);

sgatev wrote:
> That's guaranteed to be false in `compareEquivalent`.
I thought just `Val1 == Val2` was guaranteed false? Also, I see that the 
variable naming could be confusing, so I've renamed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124104

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

[PATCH] D124262: compile commands header to source heuristic lower-cases filenames before inferring file types

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In D124262#3471841 , @ishaangandhi 
wrote:

> Test case added, re-diffed with `-U`, and removed redundant constructor. 
> Thanks for the quick feedback, @sammccall!
>
> (I didn't wait for the tests to run locally, I am hoping to use your CI 
> systems to do that. It seems like a simple enough of a test that I hope I can 
> get it right on the first try.)

Unfortunately it looks like you've created a patch that phabricator can render, 
but the precommit CI can't apply (it's using `git apply`).
The usual way to upload is with `arc`, but the output of `git diff -U` 
should also work.




Comment at: clang/unittests/Tooling/CompilationDatabaseTest.cpp:852
+  add("other/random/path.cpp");
+  // Proxies for ".H" files are ".C" files, and not ".cpp files"
+  EXPECT_EQ(getProxy("foo/bar/baz/exact.H"), "foo/bar/baz/exact.C");

Is this really what you intend to test?

I think our goal here is to treat `.H` and `.C` as C++ rather than C, not to 
link them together in some way. If the current file is `foo.H`, then `foo.cpp` 
and `foo.C` should be equally compelling candidates, but both beat `foo.c`.


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

https://reviews.llvm.org/D124262

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

[PATCH] D124250: [Serialization] write expr dependence bits as a single integer

[Jake Egan via Phabricator via cfe-commits]

Jake-Egan added a comment.

Hi, this broke a test on AIX 
https://lab.llvm.org/buildbot/#/builders/214/builds/903/steps/6/logs/FAIL__Clang__pch-with-module_m


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124250

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

[PATCH] D124262: compile commands header to source heuristic lower-cases filenames before inferring file types

[Ishaan Gandhi via Phabricator via cfe-commits]

ishaangandhi added inline comments.



Comment at: clang/unittests/Tooling/CompilationDatabaseTest.cpp:852
+  add("other/random/path.cpp");
+  // Proxies for ".H" files are ".C" files, and not ".cpp files"
+  EXPECT_EQ(getProxy("foo/bar/baz/exact.H"), "foo/bar/baz/exact.C");

sammccall wrote:
> Is this really what you intend to test?
> 
> I think our goal here is to treat `.H` and `.C` as C++ rather than C, not to 
> link them together in some way. If the current file is `foo.H`, then 
> `foo.cpp` and `foo.C` should be equally compelling candidates, but both beat 
> `foo.c`.
Bleh brain fart. You're right. "exact.C" should win over "exact.c", not 
necessarily "exact.cpp".


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

https://reviews.llvm.org/D124262

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

[PATCH] D107668: [OpenMP]Fix PR50336: Remove temporary files in the offload bundler tool

[Victor Lomuller via Phabricator via cfe-commits]

Naghasan added a comment.

Thanks for your answer, make sense.

If we are moving away I won't bother looking fixing my issue upstream as it is 
very niche (I don't think you can trigger this with the upstream clang driver).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107668

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

[PATCH] D124384: [clang] Fix a constant evaluator crash on a NULL-type expr.

[Sam McCall via Phabricator via cfe-commits]

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

This is a reasonable immediate fix, but creating null-typed expressions that 
live *outside* the recoveryexpr seems scary, and likely to violate expected 
invariants elsewhere too...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124384

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

[PATCH] D124262: compile commands header to source heuristic lower-cases filenames before inferring file types

[Ishaan Gandhi via Phabricator via cfe-commits]

ishaangandhi updated this revision to Diff 424914.
ishaangandhi added a comment.

Made test case reflect that proxies for ".H" files are ".C" files, and not ".c 
files".


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

https://reviews.llvm.org/D124262

Files:
  clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
  clang/unittests/Tooling/CompilationDatabaseTest.cpp


Index: clang/unittests/Tooling/CompilationDatabaseTest.cpp
===
--- clang/unittests/Tooling/CompilationDatabaseTest.cpp
+++ clang/unittests/Tooling/CompilationDatabaseTest.cpp
@@ -845,6 +845,14 @@
   EXPECT_EQ(getProxy("foo/bar/baz/shout.C"), "FOO/BAR/BAZ/SHOUT.cc");
 }
 
+TEST_F(InterpolateTest, LanguagePreference) {
+  add("foo/bar/baz/exact.C");
+  add("foo/bar/baz/exact.c");
+  add("other/random/path.cpp");
+  // Proxies for ".H" files are ".C" files, and not ".c files".
+  EXPECT_EQ(getProxy("foo/bar/baz/exact.H"), "foo/bar/baz/exact.C");
+}
+
 TEST_F(InterpolateTest, Aliasing) {
   add("foo.cpp", "-faligned-new");
 
Index: clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
===
--- clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
+++ clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
@@ -329,7 +329,7 @@
   StringRef Path = Strings.save(StringRef(OriginalPaths[I]).lower());
 
   Paths.emplace_back(Path, I);
-  Types.push_back(foldType(guessType(Path)));
+  Types.push_back(foldType(guessType(OriginalPaths[I])));
   Stems.emplace_back(sys::path::stem(Path), I);
   auto Dir = ++sys::path::rbegin(Path), DirEnd = sys::path::rend(Path);
   for (int J = 0; J < DirectorySegmentsIndexed && Dir != DirEnd; ++J, 
++Dir)


Index: clang/unittests/Tooling/CompilationDatabaseTest.cpp
===
--- clang/unittests/Tooling/CompilationDatabaseTest.cpp
+++ clang/unittests/Tooling/CompilationDatabaseTest.cpp
@@ -845,6 +845,14 @@
   EXPECT_EQ(getProxy("foo/bar/baz/shout.C"), "FOO/BAR/BAZ/SHOUT.cc");
 }
 
+TEST_F(InterpolateTest, LanguagePreference) {
+  add("foo/bar/baz/exact.C");
+  add("foo/bar/baz/exact.c");
+  add("other/random/path.cpp");
+  // Proxies for ".H" files are ".C" files, and not ".c files".
+  EXPECT_EQ(getProxy("foo/bar/baz/exact.H"), "foo/bar/baz/exact.C");
+}
+
 TEST_F(InterpolateTest, Aliasing) {
   add("foo.cpp", "-faligned-new");
 
Index: clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
===
--- clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
+++ clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
@@ -329,7 +329,7 @@
   StringRef Path = Strings.save(StringRef(OriginalPaths[I]).lower());
 
   Paths.emplace_back(Path, I);
-  Types.push_back(foldType(guessType(Path)));
+  Types.push_back(foldType(guessType(OriginalPaths[I])));
   Stems.emplace_back(sys::path::stem(Path), I);
   auto Dir = ++sys::path::rbegin(Path), DirEnd = sys::path::rend(Path);
   for (int J = 0; J < DirectorySegmentsIndexed && Dir != DirEnd; ++J, ++Dir)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D124104: [clang][dataflow] Fix `Environment::join`'s handling of flow condition merging

[Stanislav Gatev via Phabricator via cfe-commits]

sgatev accepted this revision.
sgatev added inline comments.



Comment at: 
clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp:356
+
+return V1 == V2 ||
+   Env1.flowConditionImplies(*V1) == Env2.flowConditionImplies(*V2);

ymandel wrote:
> sgatev wrote:
> > That's guaranteed to be false in `compareEquivalent`.
> I thought just `Val1 == Val2` was guaranteed false? Also, I see that the 
> variable naming could be confusing, so I've renamed.
Right. I think I misread it. Having `IsSet1 == IsSet2` here makes sense to me. 
Though, if the test only needs the second part, I suggest removing this to 
simplify the setup.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124104

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

[PATCH] D124104: [clang][dataflow] Fix `Environment::join`'s handling of flow condition merging

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel updated this revision to Diff 424916.
ymandel marked an inline comment as done.
ymandel added a comment.

address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124104

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

Index: clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
@@ -10,6 +10,7 @@
 #include "TestingSupport.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/ExprCXX.h"
+#include "clang/AST/Type.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/Analysis/CFG.h"
@@ -295,6 +296,164 @@
   // FIXME: Called functions at point `p` should contain only "foo".
 }
 
+// Models an analysis that uses flow conditions.
+class SpecialBoolAnalysis
+: public DataflowAnalysis {
+public:
+  explicit SpecialBoolAnalysis(ASTContext &Context)
+  : DataflowAnalysis(Context) {}
+
+  static NoopLattice initialElement() { return {}; }
+
+  void transfer(const Stmt *S, NoopLattice &, Environment &Env) {
+auto SpecialBoolRecordDecl = recordDecl(hasName("SpecialBool"));
+auto HasSpecialBoolType = hasType(SpecialBoolRecordDecl);
+
+if (const auto *E = selectFirst(
+"call", match(cxxConstructExpr(HasSpecialBoolType).bind("call"), *S,
+  getASTContext( {
+  auto &ConstructorVal = *cast(Env.createValue(E->getType()));
+  ConstructorVal.setProperty("is_set", Env.getBoolLiteralValue(false));
+  Env.setValue(*Env.getStorageLocation(*E, SkipPast::None), ConstructorVal);
+} else if (const auto *E = selectFirst(
+   "call", match(cxxMemberCallExpr(callee(cxxMethodDecl(ofClass(
+   SpecialBoolRecordDecl
+ .bind("call"),
+ *S, getASTContext( {
+  auto *Object = E->getImplicitObjectArgument();
+  assert(Object != nullptr);
+
+  auto *ObjectLoc =
+  Env.getStorageLocation(*Object, SkipPast::ReferenceThenPointer);
+  assert(ObjectLoc != nullptr);
+
+  auto &ConstructorVal =
+  *cast(Env.createValue(Object->getType()));
+  ConstructorVal.setProperty("is_set", Env.getBoolLiteralValue(true));
+  Env.setValue(*ObjectLoc, ConstructorVal);
+}
+  }
+
+  bool compareEquivalent(QualType Type, const Value &Val1,
+ const Environment &Env1, const Value &Val2,
+ const Environment &Env2) final {
+const auto *Decl = Type->getAsCXXRecordDecl();
+if (Decl == nullptr || Decl->getIdentifier() == nullptr ||
+Decl->getName() != "SpecialBool")
+  return false;
+
+auto *IsSet1 = cast_or_null(
+cast(&Val1)->getProperty("is_set"));
+if (IsSet1 == nullptr)
+  return true;
+
+auto *IsSet2 = cast_or_null(
+cast(&Val2)->getProperty("is_set"));
+if (IsSet2 == nullptr)
+  return false;
+
+return Env1.flowConditionImplies(*IsSet1) ==
+   Env2.flowConditionImplies(*IsSet2);
+  }
+
+  // Always returns `true` to accept the `MergedVal`.
+  bool merge(QualType Type, const Value &Val1, const Environment &Env1,
+ const Value &Val2, const Environment &Env2, Value &MergedVal,
+ Environment &MergedEnv) final {
+const auto *Decl = Type->getAsCXXRecordDecl();
+if (Decl == nullptr || Decl->getIdentifier() == nullptr ||
+Decl->getName() != "SpecialBool")
+  return true;
+
+auto *IsSet1 = cast_or_null(
+cast(&Val1)->getProperty("is_set"));
+if (IsSet1 == nullptr)
+  return true;
+
+auto *IsSet2 = cast_or_null(
+cast(&Val2)->getProperty("is_set"));
+if (IsSet2 == nullptr)
+  return true;
+
+auto &IsSet = MergedEnv.makeAtomicBoolValue();
+cast(&MergedVal)->setProperty("is_set", IsSet);
+if (Env1.flowConditionImplies(*IsSet1) &&
+Env2.flowConditionImplies(*IsSet2))
+  MergedEnv.addToFlowCondition(IsSet);
+
+return true;
+  }
+};
+
+class JoinFlowConditionsTest : public Test {
+protected:
+  template 
+  void runDataflow(llvm::StringRef Code, Matcher Match) {
+ASSERT_THAT_ERROR(
+test::checkDataflow(
+Code, "target",
+[](ASTContext &Context, Environment &Env) {
+  return SpecialBoolAnalysis(Context);
+},
+[&Match](
+llvm::ArrayRef<
+std::pair>>
+Results,
+ASTContext &ASTCtx) { Match(Results, ASTCtx); },
+{"-fsyntax-only", "-std=c++17"}),
+ll

[PATCH] D124389: [clang][NFC] Inclusive language: remove use of Whitelist in clang/lib/Analysis/

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM, thanks for the cleanup!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124389

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

[PATCH] D124104: [clang][dataflow] Fix `Environment::join`'s handling of flow condition merging

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

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



Comment at: 
clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp:356
+
+return V1 == V2 ||
+   Env1.flowConditionImplies(*V1) == Env2.flowConditionImplies(*V2);

sgatev wrote:
> ymandel wrote:
> > sgatev wrote:
> > > That's guaranteed to be false in `compareEquivalent`.
> > I thought just `Val1 == Val2` was guaranteed false? Also, I see that the 
> > variable naming could be confusing, so I've renamed.
> Right. I think I misread it. Having `IsSet1 == IsSet2` here makes sense to 
> me. Though, if the test only needs the second part, I suggest removing this 
> to simplify the setup.
agreed. done


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124104

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

[clang] 37b4782 - [clang][dataflow] Fix `Environment::join`'s handling of flow-condition merging.

[Yitzhak Mandelbaum via cfe-commits]

Author: Yitzhak Mandelbaum
Date: 2022-04-25T15:05:50Z
New Revision: 37b4782e3e53cba265d26843f222134ed21e1974

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

LOG: [clang][dataflow] Fix `Environment::join`'s handling of flow-condition 
merging.

The current implementation mutates the environment as it performs the
join. However, that interferes with the call to the model's `merge` operation,
which can modify `MergedEnv`. Since any modifications are assumed to apply to
the post-join environment, providing the same environment for both is
incorrect. This mismatch is a particular concern for joining the flow
conditions, where modifications in the old environment may not be propagated to
the new one.

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

Added: 


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

Removed: 




diff  --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp 
b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
index 04bf41605d9d3..91e07bdb88ce0 100644
--- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -66,12 +66,14 @@ static bool equivalentValues(QualType Type, Value *Val1,
   return Model.compareEquivalent(Type, *Val1, Env1, *Val2, Env2);
 }
 
-/// Attempts to merge distinct values `Val1` and `Val1` in `Env1` and `Env2`,
+/// Attempts to merge distinct values `Val1` and `Val2` in `Env1` and `Env2`,
 /// respectively, of the same type `Type`. Merging generally produces a single
 /// value that (soundly) approximates the two inputs, although the actual
 /// meaning depends on `Model`.
-static Value *mergeDistinctValues(QualType Type, Value *Val1, Environment 
&Env1,
-  Value *Val2, const Environment &Env2,
+static Value *mergeDistinctValues(QualType Type, Value *Val1,
+  const Environment &Env1, Value *Val2,
+  const Environment &Env2,
+  Environment &MergedEnv,
   Environment::ValueModel &Model) {
   // Join distinct boolean values preserving information about the constraints
   // in the respective path conditions. Note: this construction can, in
@@ -94,19 +96,19 @@ static Value *mergeDistinctValues(QualType Type, Value 
*Val1, Environment &Env1,
   // have mutually exclusive conditions.
   if (auto *Expr1 = dyn_cast(Val1)) {
 for (BoolValue *Constraint : Env1.getFlowConditionConstraints()) {
-  Expr1 = &Env1.makeAnd(*Expr1, *Constraint);
+  Expr1 = &MergedEnv.makeAnd(*Expr1, *Constraint);
 }
 auto *Expr2 = cast(Val2);
 for (BoolValue *Constraint : Env2.getFlowConditionConstraints()) {
-  Expr2 = &Env1.makeAnd(*Expr2, *Constraint);
+  Expr2 = &MergedEnv.makeAnd(*Expr2, *Constraint);
 }
-return &Env1.makeOr(*Expr1, *Expr2);
+return &MergedEnv.makeOr(*Expr1, *Expr2);
   }
 
   // FIXME: Consider destroying `MergedValue` immediately if 
`ValueModel::merge`
   // returns false to avoid storing unneeded values in `DACtx`.
-  if (Value *MergedVal = Env1.createValue(Type))
-if (Model.merge(Type, *Val1, Env1, *Val2, Env2, *MergedVal, Env1))
+  if (Value *MergedVal = MergedEnv.createValue(Type))
+if (Model.merge(Type, *Val1, Env1, *Val2, Env2, *MergedVal, MergedEnv))
   return MergedVal;
 
   return nullptr;
@@ -315,28 +317,26 @@ LatticeJoinEffect Environment::join(const Environment 
&Other,
 
   auto Effect = LatticeJoinEffect::Unchanged;
 
-  const unsigned DeclToLocSizeBefore = DeclToLoc.size();
-  DeclToLoc = intersectDenseMaps(DeclToLoc, Other.DeclToLoc);
-  if (DeclToLocSizeBefore != DeclToLoc.size())
+  Environment JoinedEnv(*DACtx);
+
+  JoinedEnv.DeclToLoc = intersectDenseMaps(DeclToLoc, Other.DeclToLoc);
+  if (DeclToLoc.size() != JoinedEnv.DeclToLoc.size())
 Effect = LatticeJoinEffect::Changed;
 
-  const unsigned ExprToLocSizeBefore = ExprToLoc.size();
-  ExprToLoc = intersectDenseMaps(ExprToLoc, Other.ExprToLoc);
-  if (ExprToLocSizeBefore != ExprToLoc.size())
+  JoinedEnv.ExprToLoc = intersectDenseMaps(ExprToLoc, Other.ExprToLoc);
+  if (ExprToLoc.size() != JoinedEnv.ExprToLoc.size())
 Effect = LatticeJoinEffect::Changed;
 
-  const unsigned MemberLocToStructSizeBefore = MemberLocToStruct.size();
-  MemberLocToStruct =
+  JoinedEnv.MemberLocToStruct =
   intersectDenseMaps(MemberLocToStruct, Other.MemberLocToStruct);
-  if (MemberLocToStructSizeBefore != MemberLocToStruct.size())
+  if (MemberLocToStruct.size() != JoinedEnv.MemberLocToStruct.size())
 Effect = LatticeJoinEffect::Changed;
 
-  // Move `LocToVal` so that `Environme

[PATCH] D124104: [clang][dataflow] Fix `Environment::join`'s handling of flow condition merging

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
ymandel marked an inline comment as done.
Closed by commit rG37b4782e3e53: [clang][dataflow] Fix 
`Environment::join`'s handling of flow-condition merging. (authored by 
ymandel).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124104

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

Index: clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
@@ -10,6 +10,7 @@
 #include "TestingSupport.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/ExprCXX.h"
+#include "clang/AST/Type.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/Analysis/CFG.h"
@@ -295,6 +296,164 @@
   // FIXME: Called functions at point `p` should contain only "foo".
 }
 
+// Models an analysis that uses flow conditions.
+class SpecialBoolAnalysis
+: public DataflowAnalysis {
+public:
+  explicit SpecialBoolAnalysis(ASTContext &Context)
+  : DataflowAnalysis(Context) {}
+
+  static NoopLattice initialElement() { return {}; }
+
+  void transfer(const Stmt *S, NoopLattice &, Environment &Env) {
+auto SpecialBoolRecordDecl = recordDecl(hasName("SpecialBool"));
+auto HasSpecialBoolType = hasType(SpecialBoolRecordDecl);
+
+if (const auto *E = selectFirst(
+"call", match(cxxConstructExpr(HasSpecialBoolType).bind("call"), *S,
+  getASTContext( {
+  auto &ConstructorVal = *cast(Env.createValue(E->getType()));
+  ConstructorVal.setProperty("is_set", Env.getBoolLiteralValue(false));
+  Env.setValue(*Env.getStorageLocation(*E, SkipPast::None), ConstructorVal);
+} else if (const auto *E = selectFirst(
+   "call", match(cxxMemberCallExpr(callee(cxxMethodDecl(ofClass(
+   SpecialBoolRecordDecl
+ .bind("call"),
+ *S, getASTContext( {
+  auto *Object = E->getImplicitObjectArgument();
+  assert(Object != nullptr);
+
+  auto *ObjectLoc =
+  Env.getStorageLocation(*Object, SkipPast::ReferenceThenPointer);
+  assert(ObjectLoc != nullptr);
+
+  auto &ConstructorVal =
+  *cast(Env.createValue(Object->getType()));
+  ConstructorVal.setProperty("is_set", Env.getBoolLiteralValue(true));
+  Env.setValue(*ObjectLoc, ConstructorVal);
+}
+  }
+
+  bool compareEquivalent(QualType Type, const Value &Val1,
+ const Environment &Env1, const Value &Val2,
+ const Environment &Env2) final {
+const auto *Decl = Type->getAsCXXRecordDecl();
+if (Decl == nullptr || Decl->getIdentifier() == nullptr ||
+Decl->getName() != "SpecialBool")
+  return false;
+
+auto *IsSet1 = cast_or_null(
+cast(&Val1)->getProperty("is_set"));
+if (IsSet1 == nullptr)
+  return true;
+
+auto *IsSet2 = cast_or_null(
+cast(&Val2)->getProperty("is_set"));
+if (IsSet2 == nullptr)
+  return false;
+
+return Env1.flowConditionImplies(*IsSet1) ==
+   Env2.flowConditionImplies(*IsSet2);
+  }
+
+  // Always returns `true` to accept the `MergedVal`.
+  bool merge(QualType Type, const Value &Val1, const Environment &Env1,
+ const Value &Val2, const Environment &Env2, Value &MergedVal,
+ Environment &MergedEnv) final {
+const auto *Decl = Type->getAsCXXRecordDecl();
+if (Decl == nullptr || Decl->getIdentifier() == nullptr ||
+Decl->getName() != "SpecialBool")
+  return true;
+
+auto *IsSet1 = cast_or_null(
+cast(&Val1)->getProperty("is_set"));
+if (IsSet1 == nullptr)
+  return true;
+
+auto *IsSet2 = cast_or_null(
+cast(&Val2)->getProperty("is_set"));
+if (IsSet2 == nullptr)
+  return true;
+
+auto &IsSet = MergedEnv.makeAtomicBoolValue();
+cast(&MergedVal)->setProperty("is_set", IsSet);
+if (Env1.flowConditionImplies(*IsSet1) &&
+Env2.flowConditionImplies(*IsSet2))
+  MergedEnv.addToFlowCondition(IsSet);
+
+return true;
+  }
+};
+
+class JoinFlowConditionsTest : public Test {
+protected:
+  template 
+  void runDataflow(llvm::StringRef Code, Matcher Match) {
+ASSERT_THAT_ERROR(
+test::checkDataflow(
+Code, "target",
+[](ASTContext &Context, Environment &Env) {
+  return SpecialBoolAnalysis(Context);
+},
+[&Match](
+llvm::ArrayRef<
+

[PATCH] D122663: Mark identifier prefixes as substitutable

[Harald van Dijk via Phabricator via cfe-commits]

hvdijk added a comment.

In D122663#3471866 , @erichkeane 
wrote:

> This actually wouldn't be necessary in this case: cxxfilt is already 'right', 
> this is just for humans to be able to see "we changed this from mangling as 
>  to ".

That's a good point. We do have tests in clang that use llvm-cxxfilt already 
(the PPC intrinsic tests) so there should be no problem with it as a 
dependency; I think once this is in I might submit that as a follow-up change 
for review unless someone else beats me to it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122663

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

[PATCH] D122663: Mark identifier prefixes as substitutable

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D122663#3471960 , @hvdijk wrote:

> In D122663#3471866 , @erichkeane 
> wrote:
>
>> This actually wouldn't be necessary in this case: cxxfilt is already 
>> 'right', this is just for humans to be able to see "we changed this from 
>> mangling as  to ".
>
> That's a good point. We do have tests in clang that use llvm-cxxfilt already 
> (the PPC intrinsic tests) so there should be no problem with it as a 
> dependency; I think once this is in I might submit that as a follow-up change 
> for review unless someone else beats me to it.

Any amount of test cleanup in that direction would be greatly appreciated!  
Even as NFC.

The test I did it for was CodeGenCXX/mangle-nttp-anon-union.cpp as an example 
of what I'm talking about.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122663

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

Re: [PATCH] D124250: [Serialization] write expr dependence bits as a single integer

[Sam McCall via cfe-commits]

Hi Jake,

I suspect something about the test/runner is nonhermetic, I'm going to try
to fix it in the test.

This commit changes the serialized PCH format.
It looks like this test is not properly hermetic, and is trying to load the
PCH from a previous run of the test. It clears the module dir, but not the
PCH itself.
This would normally trigger a version mismatch error, but the test disables
it with -fdisable-module-hash for some reason.

Only mystery is why the c-index-test line doesn't fail if it doesn't manage
to overwrite the PCH file.

On Mon, Apr 25, 2022 at 4:59 PM Jake Egan via Phabricator <
revi...@reviews.llvm.org> wrote:

> Jake-Egan added a comment.
>
> Hi, this broke a test on AIX
> https://lab.llvm.org/buildbot/#/builders/214/builds/903/steps/6/logs/FAIL__Clang__pch-with-module_m
>
>
> Repository:
>   rG LLVM Github Monorepo
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D124250/new/
>
> https://reviews.llvm.org/D124250
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 1c65c73 - Clear temporary file in test, buildbot appears to be reusing an old one.

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2022-04-25T17:26:39+02:00
New Revision: 1c65c734c93f8c4d39947e596d7fe89289ce283d

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

LOG: Clear temporary file in test, buildbot appears to be reusing an old one.

https://lab.llvm.org/buildbot/#/builders/214/builds/903/steps/6/logs/FAIL__Clang__pch-with-module_m

Added: 


Modified: 
clang/test/Index/pch-with-module.m

Removed: 




diff  --git a/clang/test/Index/pch-with-module.m 
b/clang/test/Index/pch-with-module.m
index 77262d5eb6d1f..9acd9694830bd 100644
--- a/clang/test/Index/pch-with-module.m
+++ b/clang/test/Index/pch-with-module.m
@@ -1,5 +1,6 @@
 // REQUIRES: x86-registered-target
 // RUN: rm -rf %t.cache
+// RUN: rm -rf %t.h.pch
 // RUN: c-index-test -write-pch %t.h.pch %s -target x86_64-apple-macosx10.7 
-fobjc-arc -fmodules-cache-path=%t.cache -fmodules -F %S/../Modules/Inputs 
-Xclang -fdisable-module-hash
 // RUN: %clang -fsyntax-only %s -target x86_64-apple-macosx10.7 -include %t.h 
-fobjc-arc -fmodules-cache-path=%t.cache -fmodules -F %S/../Modules/Inputs \
 // RUN:  -Xclang -fdisable-module-hash -Xclang 
-detailed-preprocessing-record -Xclang -verify



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

[PATCH] D124250: [Serialization] write expr dependence bits as a single integer

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

1c65c734c93f8c4d39947e596d7fe89289ce283d 
 will 
clear the PCH file. If this
doesn't fix the problem I'll revert.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124250

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

[PATCH] D124395: [clang][dataflow] Optimize flow condition representation

[Stanislav Gatev via Phabricator via cfe-commits]

sgatev created this revision.
sgatev added reviewers: ymandel, xazax.hun, gribozavr2.
Herald added subscribers: tschuett, steakhal, rnkovacs.
Herald added a project: All.
sgatev requested review of this revision.
Herald added a project: clang.

Represent flow conditions as Token <=> Constraints biconditional clauses
in order to make joining of distinct values more efficient. For example,
joining distinct values while preserving flow condition information can
be implemented by adding the tokens of the flow conditions to the formula:
`makeOr(makeAnd(FC1, Val1), makeAnd(FC2, Val2))`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D124395

Files:
  clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
  clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
  clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
  clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp

Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
===
--- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -76,33 +76,14 @@
   Environment &MergedEnv,
   Environment::ValueModel &Model) {
   // Join distinct boolean values preserving information about the constraints
-  // in the respective path conditions. Note: this construction can, in
-  // principle, result in exponential growth in the size of boolean values.
-  // Potential optimizations may be worth considering. For example, represent
-  // the flow condition of each environment using a bool atom and store, in
-  // `DataflowAnalysisContext`, a mapping of bi-conditionals between flow
-  // condition atoms and flow condition constraints. Something like:
-  // \code
-  //   FC1 <=> C1 ^ C2
-  //   FC2 <=> C2 ^ C3 ^ C4
-  //   FC3 <=> (FC1 v FC2) ^ C5
-  // \code
-  // Then, we can track dependencies between flow conditions (e.g. above `FC3`
-  // depends on `FC1` and `FC2`) and modify `flowConditionImplies` to construct
-  // a formula that includes the bi-conditionals for all flow condition atoms in
-  // the transitive set, before invoking the solver.
+  // in the respective path conditions.
   //
   // FIXME: Does not work for backedges, since the two (or more) paths will not
   // have mutually exclusive conditions.
   if (auto *Expr1 = dyn_cast(Val1)) {
-for (BoolValue *Constraint : Env1.getFlowConditionConstraints()) {
-  Expr1 = &MergedEnv.makeAnd(*Expr1, *Constraint);
-}
 auto *Expr2 = cast(Val2);
-for (BoolValue *Constraint : Env2.getFlowConditionConstraints()) {
-  Expr2 = &MergedEnv.makeAnd(*Expr2, *Constraint);
-}
-return &MergedEnv.makeOr(*Expr1, *Expr2);
+return &Env1.makeOr(Env1.makeAnd(Env1.getFlowConditionToken(), *Expr1),
+Env1.makeAnd(Env2.getFlowConditionToken(), *Expr2));
   }
 
   // FIXME: Consider destroying `MergedValue` immediately if `ValueModel::merge`
@@ -156,63 +137,6 @@
   }
 }
 
-/// Returns constraints that represent the disjunction of `Constraints1` and
-/// `Constraints2`.
-///
-/// Requirements:
-///
-///  The elements of `Constraints1` and `Constraints2` must not be null.
-llvm::DenseSet
-joinConstraints(DataflowAnalysisContext *Context,
-const llvm::DenseSet &Constraints1,
-const llvm::DenseSet &Constraints2) {
-  // `(X ^ Y) v (X ^ Z)` is logically equivalent to `X ^ (Y v Z)`. Therefore, to
-  // avoid unnecessarily expanding the resulting set of constraints, we will add
-  // all common constraints of `Constraints1` and `Constraints2` directly and
-  // add a disjunction of the constraints that are not common.
-
-  llvm::DenseSet JoinedConstraints;
-
-  if (Constraints1.empty() || Constraints2.empty()) {
-// Disjunction of empty set and non-empty set is represented as empty set.
-return JoinedConstraints;
-  }
-
-  BoolValue *Val1 = nullptr;
-  for (BoolValue *Constraint : Constraints1) {
-if (Constraints2.contains(Constraint)) {
-  // Add common constraints directly to `JoinedConstraints`.
-  JoinedConstraints.insert(Constraint);
-} else if (Val1 == nullptr) {
-  Val1 = Constraint;
-} else {
-  Val1 = &Context->getOrCreateConjunctionValue(*Val1, *Constraint);
-}
-  }
-
-  BoolValue *Val2 = nullptr;
-  for (BoolValue *Constraint : Constraints2) {
-// Common constraints are added to `JoinedConstraints` above.
-if (Constraints1.contains(Constraint)) {
-  continue;
-}
-if (Val2 == nullptr) {
-  Val2 = Constraint;
-} else {
-  Val2 = &Context->getOrCreateConjunctionValue(*Val2, *Constraint);
-}
-  }
-
-  // An empty set of constraints (represented as a null value) is interpreted as
-  // `true` and `true v X` is logically equivalent to `true` so we need to add a
-  // constraint only if both `Val1` and `Val2` are not null.
-  if

[PATCH] D121299: [NVPTX] Disable DWARF .file directory for PTX

[Andrew Savonichev via Phabricator via cfe-commits]

asavonic added a comment.

I'm going to merge the patch in a couple of days, unless anyone has a strong 
opinion against it.
Alternative options:

- revert this default back to false (not great for other targets)
- add a check for `Triple == NVPTX` directly to llc (tools other than llc will 
have to replicate the same logic)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121299

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

[PATCH] D124396: [HIP] Fix diag msg about sanitizer

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl created this revision.
yaxunl added a reviewer: tra.
Herald added subscribers: kerbowa, jvesely.
Herald added a project: All.
yaxunl requested review of this revision.

Fix the misleading diag msg as the option is ignored for a particular offload 
arch only.


https://reviews.llvm.org/D124396

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/test/Driver/hip-sanitize-options.hip
  clang/test/Driver/openmp-amdgpu-sanitize-options.c


Index: clang/test/Driver/openmp-amdgpu-sanitize-options.c
===
--- clang/test/Driver/openmp-amdgpu-sanitize-options.c
+++ clang/test/Driver/openmp-amdgpu-sanitize-options.c
@@ -32,8 +32,8 @@
 // RUN:   %clang -### -fopenmp --offload-arch=gfx908:xnack- -fsanitize=address 
-fno-gpu-sanitize  %s 2>&1 \
 // RUN:   | FileCheck -check-prefixes=NOGPUSAN,XNACKNEG %s
 
-// XNACK-DAG: warning: ignoring '-fsanitize=address' option as it is not 
currently supported for offload arch 'gfx908:xnack-'. Use it with an offload 
arch containing 'xnack+' instead
-// XNACKNONE-DAG: warning: ignoring '-fsanitize=address' option as it is not 
currently supported for offload arch 'gfx908'. Use it with an offload arch 
containing 'xnack+' instead
+// XNACK-DAG: warning: ignoring '-fsanitize=address' option for offload arch 
'gfx908:xnack-' as it is not currently supported for that offload arch. Use it 
with an offload arch containing 'xnack+' instead
+// XNACKNONE-DAG: warning: ignoring '-fsanitize=address' option for offload 
arch 'gfx908' as it is not currently supported for that offload arch. Use it 
with an offload arch containing 'xnack+' instead
 
 // GPUSAN: clang{{.*}}"-cc1" "-triple" 
"x86_64-unknown-linux-gnu"{{.*}}"-fopenmp"{{.*}}"-fsanitize=address"{{.*}}"-fopenmp-targets=amdgcn-amd-amdhsa"{{.*}}"-x"
 "c"{{.*}}
 // GPUSAN: clang{{.*}}"-cc1" "-triple" 
"x86_64-unknown-linux-gnu"{{.*}}"-fopenmp"{{.*}}"-fsanitize=address"{{.*}}"-fopenmp-targets=amdgcn-amd-amdhsa"{{.*}}"-x"
 "ir"{{.*}}
Index: clang/test/Driver/hip-sanitize-options.hip
===
--- clang/test/Driver/hip-sanitize-options.hip
+++ clang/test/Driver/hip-sanitize-options.hip
@@ -65,8 +65,8 @@
 // FAIL: error: AMDGPU address sanitizer runtime library (asanrtl) not found. 
Please install ROCm device library which supports address sanitizer
 
 // XNACK-DAG: warning: ignoring '-fsanitize=leak' option as it is not 
currently supported for target 'amdgcn-amd-amdhsa'
-// XNACK-DAG: warning: ignoring '-fsanitize=address' option as it is not 
currently supported for offload arch 'gfx900:xnack-'. Use it with an offload 
arch containing 'xnack+' instead
-// XNACK-DAG: warning: ignoring '-fsanitize=address' option as it is not 
currently supported for offload arch 'gfx906'. Use it with an offload arch 
containing 'xnack+' instead
+// XNACK-DAG: warning: ignoring '-fsanitize=address' option for offload arch 
'gfx900:xnack-' as it is not currently supported for that offload arch. Use it 
with an offload arch containing 'xnack+' instead
+// XNACK-DAG: warning: ignoring '-fsanitize=address' option for offload arch 
'gfx906' as it is not currently supported for that offload arch. Use it with an 
offload arch containing 'xnack+' instead
 // XNACK-DAG: {{"[^"]*clang[^"]*".* "-mlink-bitcode-file" ".*asanrtl.bc".* 
"-target-cpu" "gfx900".* "-target-feature" "\+xnack".* "-fsanitize=address"}}
 // XNACK-DAG: {{"[^"]*clang[^"]*".* "-target-cpu" "gfx900".* "-target-feature" 
"-xnack"}}
 // XNACK-DAG: {{"[^"]*clang[^"]*".* "-target-cpu" "gfx906"}}
@@ -85,8 +85,8 @@
 // NOGPU-DAG: {{"[^"]*clang[^"]*".* "-target-cpu" "gfx906"}}
 // NOGPU-DAG: {{"[^"]*clang[^"]*".* "-triple" "x86_64-unknown-linux-gnu".* 
"-fsanitize=address,leak"}}
 // NOGPUNEG-NOT: warning: ignoring '-fsanitize=leak' option as it is not 
currently supported for target 'amdgcn-amd-amdhsa'
-// NOGPUNEG-NOT: warning: ignoring '-fsanitize=address' option as it is not 
currently supported for offload arch 'gfx900:xnack-'. Use it with an offload 
arch containing 'xnack+' instead
-// NOGPUNEG-NOT: warning: ignoring '-fsanitize=address' option as it is not 
currently supported for offload arch 'gfx906'. Use it with an offload arch 
containing 'xnack+' instead
+// NOGPUNEG-NOT: warning: ignoring '-fsanitize=address' option for offload 
arch 'gfx900:xnack-' as it is not currently supported for that offload arch. 
Use it with an offload arch containing 'xnack+' instead
+// NOGPUNEG-NOT: warning: ignoring '-fsanitize=address' option for offload 
arch 'gfx906' as it is not currently supported for that offload arch. Use it 
with an offload arch containing 'xnack+' instead
 // NOGPUNEG-NOT: {{"[^"]*clang[^"]*".* "-mlink-bitcode-file" ".*asanrtl.bc".* 
"-target-cpu" "gfx900".* "-target-feature" "\+xnack".* "-fsanitize=address"}}
 // NOGPUNEG-NOT: {{"[^"]*clang[^"]*".* "-target-cpu" "gfx900".* 
"-target-feature" "\+xnack".* "-fsanitize=address,leak"}}
 // NOGPUNEG-NO

[PATCH] D124221: Add new builtin __builtin_reflect_struct.

[Wang Yihan via Phabricator via cfe-commits]

yihanaa added a comment.

Thank you for inviting me to a discussion on this topic. I have something very 
confusing as follows:

- If the developer already knows all fields info in some struct, wyh doesn't he 
write a simple function to do the printing(like: print_struct_foo(const struct 
foo *); )? It might be easier to write a printing  function.
- If the struct has many levels of nesting, and the developers just want to 
output the details of the struct for debugging purposes, then I think they 
might just need something like the 'p' command in LLDB.
- If we want to support these printing functions, and make this builtin 
flexible and general, I think reflection is a good idea. But we may want to 
make this builtin support C/C++ or other languages supported by Clang at the 
same time. I have a not-so-sophisticated (perhaps silly-sounding) idea: this 
builtin might be one or more C-style functions in order to be usable in many 
different languages. This family of functions supports getting the number of 
struct fields, as well as their details (and possibly getting the value of the 
field by getting the subscript). like:

  int __builtin_reflect_struct_member_count(const void *struct_addr, unsigned 
*count);
  int __builtin_reflect_struct_member_detail(const void *struct_addr, unsigned 
index, char *name, unsigned *bitfield, int *is_unnamed);
  int __builtin_reflect_struct_member_ptr(const void *struct_addr, unsigned 
index, void **ptr);

Maybe it also can support nested struct?

em, my idea looks really stupid, don't laugh at me, hahahaha~~~


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124221

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

[PATCH] D124262: compile commands header to source heuristic lower-cases filenames before inferring file types

[Sam McCall via Phabricator via cfe-commits]

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

Thanks!
I can land this for you if you don't have commit access - can you provide the 
name/email to use for the commit?


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

https://reviews.llvm.org/D124262

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

[PATCH] D124395: [clang][dataflow] Optimize flow condition representation

[Stanislav Gatev via Phabricator via cfe-commits]

sgatev updated this revision to Diff 424930.
sgatev added a comment.

Mark methods as const.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124395

Files:
  clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
  clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
  clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
  clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp

Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
===
--- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -76,33 +76,14 @@
   Environment &MergedEnv,
   Environment::ValueModel &Model) {
   // Join distinct boolean values preserving information about the constraints
-  // in the respective path conditions. Note: this construction can, in
-  // principle, result in exponential growth in the size of boolean values.
-  // Potential optimizations may be worth considering. For example, represent
-  // the flow condition of each environment using a bool atom and store, in
-  // `DataflowAnalysisContext`, a mapping of bi-conditionals between flow
-  // condition atoms and flow condition constraints. Something like:
-  // \code
-  //   FC1 <=> C1 ^ C2
-  //   FC2 <=> C2 ^ C3 ^ C4
-  //   FC3 <=> (FC1 v FC2) ^ C5
-  // \code
-  // Then, we can track dependencies between flow conditions (e.g. above `FC3`
-  // depends on `FC1` and `FC2`) and modify `flowConditionImplies` to construct
-  // a formula that includes the bi-conditionals for all flow condition atoms in
-  // the transitive set, before invoking the solver.
+  // in the respective path conditions.
   //
   // FIXME: Does not work for backedges, since the two (or more) paths will not
   // have mutually exclusive conditions.
   if (auto *Expr1 = dyn_cast(Val1)) {
-for (BoolValue *Constraint : Env1.getFlowConditionConstraints()) {
-  Expr1 = &MergedEnv.makeAnd(*Expr1, *Constraint);
-}
 auto *Expr2 = cast(Val2);
-for (BoolValue *Constraint : Env2.getFlowConditionConstraints()) {
-  Expr2 = &MergedEnv.makeAnd(*Expr2, *Constraint);
-}
-return &MergedEnv.makeOr(*Expr1, *Expr2);
+return &Env1.makeOr(Env1.makeAnd(Env1.getFlowConditionToken(), *Expr1),
+Env1.makeAnd(Env2.getFlowConditionToken(), *Expr2));
   }
 
   // FIXME: Consider destroying `MergedValue` immediately if `ValueModel::merge`
@@ -156,63 +137,6 @@
   }
 }
 
-/// Returns constraints that represent the disjunction of `Constraints1` and
-/// `Constraints2`.
-///
-/// Requirements:
-///
-///  The elements of `Constraints1` and `Constraints2` must not be null.
-llvm::DenseSet
-joinConstraints(DataflowAnalysisContext *Context,
-const llvm::DenseSet &Constraints1,
-const llvm::DenseSet &Constraints2) {
-  // `(X ^ Y) v (X ^ Z)` is logically equivalent to `X ^ (Y v Z)`. Therefore, to
-  // avoid unnecessarily expanding the resulting set of constraints, we will add
-  // all common constraints of `Constraints1` and `Constraints2` directly and
-  // add a disjunction of the constraints that are not common.
-
-  llvm::DenseSet JoinedConstraints;
-
-  if (Constraints1.empty() || Constraints2.empty()) {
-// Disjunction of empty set and non-empty set is represented as empty set.
-return JoinedConstraints;
-  }
-
-  BoolValue *Val1 = nullptr;
-  for (BoolValue *Constraint : Constraints1) {
-if (Constraints2.contains(Constraint)) {
-  // Add common constraints directly to `JoinedConstraints`.
-  JoinedConstraints.insert(Constraint);
-} else if (Val1 == nullptr) {
-  Val1 = Constraint;
-} else {
-  Val1 = &Context->getOrCreateConjunctionValue(*Val1, *Constraint);
-}
-  }
-
-  BoolValue *Val2 = nullptr;
-  for (BoolValue *Constraint : Constraints2) {
-// Common constraints are added to `JoinedConstraints` above.
-if (Constraints1.contains(Constraint)) {
-  continue;
-}
-if (Val2 == nullptr) {
-  Val2 = Constraint;
-} else {
-  Val2 = &Context->getOrCreateConjunctionValue(*Val2, *Constraint);
-}
-  }
-
-  // An empty set of constraints (represented as a null value) is interpreted as
-  // `true` and `true v X` is logically equivalent to `true` so we need to add a
-  // constraint only if both `Val1` and `Val2` are not null.
-  if (Val1 != nullptr && Val2 != nullptr)
-JoinedConstraints.insert(
-&Context->getOrCreateDisjunctionValue(*Val1, *Val2));
-
-  return JoinedConstraints;
-}
-
 static void
 getFieldsFromClassHierarchy(QualType Type, bool IgnorePrivateFields,
 llvm::DenseSet &Fields) {
@@ -249,6 +173,26 @@
   return Fields;
 }
 
+Environment::Environment(DataflowAnalysisContext &DACtx)
+: DACtx(&DACtx)

[PATCH] D121299: [NVPTX] Disable DWARF .file directory for PTX

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

I'm okay with doing it this way.  My impression over the years (as a debug-info 
guy, not as a ptxas user) is that ptxas evolves slowly and this is not really 
"busy work."


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121299

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

[PATCH] D124186: [RISCV] Fix incorrect policy implement for unmasked vslidedown and vslideup.

[Zakk Chen via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGffe03ff75c26: [RISCV] Fix incorrect policy implement for 
unmasked vslidedown and vslideup. (authored by khchen).

Changed prior to commit:
  https://reviews.llvm.org/D124186?vs=424517&id=424934#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124186

Files:
  clang/include/clang/Basic/riscv_vector.td
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vslidedown.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vslideup.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vslidedown.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vslideup.c
  llvm/include/llvm/IR/IntrinsicsRISCV.td
  llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td
  llvm/lib/Target/RISCV/RISCVInstrInfoVVLPatterns.td
  llvm/test/CodeGen/RISCV/rvv/emergency-slot.mir
  llvm/test/CodeGen/RISCV/rvv/extract-subvector.ll
  llvm/test/CodeGen/RISCV/rvv/fixed-vectors-calling-conv.ll
  llvm/test/CodeGen/RISCV/rvv/fixed-vectors-extload-truncstore.ll
  llvm/test/CodeGen/RISCV/rvv/fixed-vectors-extract-subvector.ll
  llvm/test/CodeGen/RISCV/rvv/fixed-vectors-fp-buildvec.ll
  llvm/test/CodeGen/RISCV/rvv/fixed-vectors-fp-conv.ll
  llvm/test/CodeGen/RISCV/rvv/fixed-vectors-fp-setcc.ll
  llvm/test/CodeGen/RISCV/rvv/fixed-vectors-fp-shuffles.ll
  llvm/test/CodeGen/RISCV/rvv/fixed-vectors-fp2i.ll
  llvm/test/CodeGen/RISCV/rvv/fixed-vectors-i2fp.ll
  llvm/test/CodeGen/RISCV/rvv/fixed-vectors-insert-i1.ll
  llvm/test/CodeGen/RISCV/rvv/fixed-vectors-insert-subvector.ll
  llvm/test/CodeGen/RISCV/rvv/fixed-vectors-insert.ll
  llvm/test/CodeGen/RISCV/rvv/fixed-vectors-int-buildvec.ll
  llvm/test/CodeGen/RISCV/rvv/fixed-vectors-int-exttrunc.ll
  llvm/test/CodeGen/RISCV/rvv/fixed-vectors-int-shuffles.ll
  llvm/test/CodeGen/RISCV/rvv/fixed-vectors-int.ll
  llvm/test/CodeGen/RISCV/rvv/fixed-vectors-mask-buildvec.ll
  llvm/test/CodeGen/RISCV/rvv/fixed-vectors-mask-load-store.ll
  llvm/test/CodeGen/RISCV/rvv/fixed-vectors-mask-splat.ll
  llvm/test/CodeGen/RISCV/rvv/fixed-vectors-masked-gather.ll
  llvm/test/CodeGen/RISCV/rvv/fixed-vectors-unaligned.ll
  llvm/test/CodeGen/RISCV/rvv/fixed-vectors-vpgather.ll
  llvm/test/CodeGen/RISCV/rvv/insert-subvector.ll
  llvm/test/CodeGen/RISCV/rvv/insertelt-fp-rv32.ll
  llvm/test/CodeGen/RISCV/rvv/insertelt-fp-rv64.ll
  llvm/test/CodeGen/RISCV/rvv/insertelt-i1.ll
  llvm/test/CodeGen/RISCV/rvv/insertelt-int-rv32.ll
  llvm/test/CodeGen/RISCV/rvv/insertelt-int-rv64.ll
  llvm/test/CodeGen/RISCV/rvv/masked-tama.ll
  llvm/test/CodeGen/RISCV/rvv/setcc-fp.ll
  llvm/test/CodeGen/RISCV/rvv/setcc-integer.ll
  llvm/test/CodeGen/RISCV/rvv/unmasked-ta.ll
  llvm/test/CodeGen/RISCV/rvv/vector-splice.ll
  llvm/test/CodeGen/RISCV/rvv/vslidedown-rv32.ll
  llvm/test/CodeGen/RISCV/rvv/vslidedown-rv64.ll
  llvm/test/CodeGen/RISCV/rvv/vslideup-rv32.ll
  llvm/test/CodeGen/RISCV/rvv/vslideup-rv64.ll

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

[PATCH] D124063: [LegacyPM] Rename and deprecate populateModulePassManager

[Arthur Eubanks via Phabricator via cfe-commits]

aeubanks added a comment.

I'd first fix up as many in-tree users as nikic pointed out

(I'll work on bugpoint soonish since that requires a bit more work)




Comment at: llvm/include/llvm-c/Transforms/PassManagerBuilder.h:72
+#ifdef __GNUC__
+__attribute__((deprecated("Migrate to new pass manager for optimization 
pipeline. This header will be removed soon.")))
+#endif

can we use `llvm/include/llvm-c/Deprecated.h` for the C API here? looks like we 
only have on in-tree user in 
`llvm/unittests/ExecutionEngine/MCJIT/MCJITCAPITest.cpp` that we should be able 
to clean up


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124063

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

[PATCH] D124262: compile commands header to source heuristic lower-cases filenames before inferring file types

[Ishaan Gandhi via Phabricator via cfe-commits]

ishaangandhi added a comment.

In D124262#3472079 , @sammccall wrote:

> Thanks!
> I can land this for you if you don't have commit access - can you provide the 
> name/email to use for the commit?

Thank you!

Ishaan Gandhi
ishaangan...@gmail.com


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

https://reviews.llvm.org/D124262

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

[PATCH] D124250: [Serialization] write expr dependence bits as a single integer

[Jake Egan via Phabricator via cfe-commits]

Jake-Egan added a comment.

Thanks @sammccall, the test is passing now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124250

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

[PATCH] D124026: [Headers][MSVC] Define wchar_t in stddef.h like MSVC if not using the builtin type

[Eli Friedman via Phabricator via cfe-commits]

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124026

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

[clang] 9727c77 - [NFC] Rename Instrinsic to Intrinsic

[David Green via cfe-commits]

Author: David Green
Date: 2022-04-25T18:13:23+01:00
New Revision: 9727c77d58ac920a4158d08c15659470e52ddda4

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

LOG: [NFC] Rename Instrinsic to Intrinsic

Added: 


Modified: 
clang/lib/CodeGen/CGBuiltin.cpp
clang/test/CodeGen/builtins-nvptx-mma.py
clang/test/CodeGenCUDA/fp-contract.cu
clang/test/Profile/c-avoid-direct-call.c
clang/test/Profile/c-indirect-call.c
clang/test/Profile/cxx-indirect-call.cpp
llvm/include/llvm/Analysis/VectorUtils.h
llvm/include/llvm/CodeGen/MachineInstr.h
llvm/include/llvm/CodeGen/ReplaceWithVeclib.h
llvm/include/llvm/IR/InstVisitor.h
llvm/include/llvm/IR/IntrinsicsARM.td
llvm/include/llvm/IR/Metadata.h
llvm/include/llvm/Transforms/Scalar/ScalarizeMaskedMemIntrin.h
llvm/include/llvm/Transforms/Utils/Local.h
llvm/lib/Analysis/CallGraphSCCPass.cpp
llvm/lib/Analysis/ConstantFolding.cpp
llvm/lib/Analysis/IVDescriptors.cpp
llvm/lib/Analysis/VectorUtils.cpp
llvm/lib/CodeGen/ReplaceWithVeclib.cpp
llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
llvm/lib/CodeGen/SjLjEHPrepare.cpp
llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
llvm/lib/Target/AArch64/AArch64InstrInfo.td
llvm/lib/Target/AArch64/AArch64StackTagging.cpp
llvm/lib/Target/BPF/BPFAbstractMemberAccess.cpp
llvm/lib/Target/Mips/MipsISelLowering.h
llvm/lib/Target/NVPTX/NVPTXIntrinsics.td
llvm/lib/Target/NVPTX/NVPTXTargetTransformInfo.cpp
llvm/lib/Target/PowerPC/README_P9.txt
llvm/lib/Target/X86/X86LowerAMXType.cpp
llvm/lib/Transforms/IPO/GlobalOpt.cpp
llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
llvm/lib/Transforms/Scalar/LoopIdiomRecognize.cpp
llvm/lib/Transforms/Scalar/ScalarizeMaskedMemIntrin.cpp
llvm/lib/Transforms/Scalar/Scalarizer.cpp
llvm/lib/Transforms/Scalar/SpeculativeExecution.cpp
llvm/lib/Transforms/Utils/Local.cpp
llvm/lib/Transforms/Utils/PromoteMemoryToRegister.cpp
llvm/lib/Transforms/Utils/RelLookupTableConverter.cpp
llvm/lib/Transforms/Utils/SimplifyCFG.cpp
llvm/lib/Transforms/Utils/StripGCRelocates.cpp
llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp
llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
llvm/test/CodeGen/Hexagon/hvx-byte-store-double.ll
llvm/test/DebugInfo/WebAssembly/dbg-declare.ll
llvm/test/Instrumentation/DataFlowSanitizer/unordered_atomic_mem_intrins.ll
llvm/test/Instrumentation/MemorySanitizer/msan_basic.ll

llvm/test/Transforms/FunctionSpecialization/function-specialization-nodup2.ll
llvm/test/Transforms/Inline/inline_constprop.ll
llvm/test/Transforms/InstCombine/stacksave-debuginfo.ll
llvm/test/Transforms/SROA/basictest-opaque-ptrs.ll
llvm/test/Transforms/SROA/basictest.ll
llvm/unittests/Analysis/IRSimilarityIdentifierTest.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index fc2d32f3e26fe..f9966c1fd777c 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -18200,7 +18200,7 @@ RValue CodeGenFunction::EmitBuiltinIsAligned(const 
CallExpr *E) {
 
 /// Generate (x & ~(y-1)) to align down or ((x+(y-1)) & ~(y-1)) to align up.
 /// Note: For pointer types we can avoid ptrtoint/inttoptr pairs by using the
-/// llvm.ptrmask instrinsic (with a GEP before in the align_up case).
+/// llvm.ptrmask intrinsic (with a GEP before in the align_up case).
 /// TODO: actually use ptrmask once most optimization passes know about it.
 RValue CodeGenFunction::EmitBuiltinAlignTo(const CallExpr *E, bool AlignUp) {
   BuiltinAlignArgs Args(E, *this);

diff  --git a/clang/test/CodeGen/builtins-nvptx-mma.py 
b/clang/test/CodeGen/builtins-nvptx-mma.py
index dc40f04c11ce6..6c09910020278 100644
--- a/clang/test/CodeGen/builtins-nvptx-mma.py
+++ b/clang/test/CodeGen/builtins-nvptx-mma.py
@@ -1,5 +1,5 @@
 # This script generates all variants of wmma builtins, verifies that clang 
calls
-# correct LLVM instrinsics, and checks that availability of specific builtins 
is
+# correct LLVM intrinsics, and checks that availability of specific builtins is
 # constrained by the correct PTX version and the target GPU variant.
 
 # Dummy test run to avoid lit warnings.

diff  --git a/clang/test/CodeGenCUDA/fp-contract.cu 
b/clang/test/CodeGenCUDA/fp-contract.cu
index d466affded132..60824ba59ddfb 100644
--- a/clang/test/CodeGenCUDA/fp-contract.cu
+++ b/clang/test/CodeGenCUDA/fp-contract.cu
@@ -105,7 +105,7 @@
 
 // Explicit -ffp-contract=on -- fusing by front-end.
 // In IR,
-//mult/add in the same statement - llvm.fmuladd instrinsic emitted
+//mult/add in the same statement - llvm.fmuladd intrinsic emitt

[PATCH] D122920: [Clang][CodeGen]Fix __builtin_dump_struct missing record type field name

[Andy Soffer via Phabricator via cfe-commits]

asoffer added a comment.

In D122920#3471865 , @yihanaa wrote:

> In D122920#3471654 , @erichkeane 
> wrote:
>
>> @yihanaa : I'd suggest seeing the conversation that @rsmith @aaron.ballman 
>> and I are having about this builtin here: https://reviews.llvm.org/D124221
>>
>> In general it looks like the three of us think that this builtin needs an 
>> overhaul in implementation/functionality in order to be further useful. 
>> While we very much appreciate your attempts to try to improve it, we believe 
>> there needs to be larger-scale changes.
>
> Thanks for your reply, I would like to get involved and help if needed

While I eagerly await `__builtin_reflect_struct`, this change still provides 
significant value in fixing a regression with `__builtin_dump_struct`. Should 
we proceed with this change and that proposal independently?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122920

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

[PATCH] D123831: [POC][WIP] Use relative include in extract-api

[Zixu Wang via Phabricator via cfe-commits]

zixuw updated this revision to Diff 424959.
zixuw added a comment.

- Use first match in `getRelativeIncludeName`
- Try to get Preprocessor and reverse lookup headermaps in 
`getRelativeIncludeName`
- Use `getRelativeIncludeName` to check for known files in `LocationFileChecker`
- Misc fixes & adjustments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123831

Files:
  clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
  clang/test/ExtractAPI/relative_include.m

Index: clang/test/ExtractAPI/relative_include.m
===
--- /dev/null
+++ clang/test/ExtractAPI/relative_include.m
@@ -0,0 +1,130 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+
+// Setup framework root
+// RUN: mkdir -p %t/Frameworks/MyFramework.framework/Headers
+// RUN: cp %t/MyFramework.h %t/Frameworks/MyFramework.framework/Headers/
+// RUN: cp %t/MyHeader.h %t/Frameworks/MyFramework.framework/Headers/
+
+// RUN: sed -e "s@SRCROOT@%{/t:regex_replacement}@g" \
+// RUN: %t/reference.output.json.in >> %t/reference.output.json
+
+// Headermap maps headers to the source root SRCROOT
+// RUN: sed -e "s@SRCROOT@%{/t:regex_replacement}@g" \
+// RUN: %t/headermap.hmap.json.in >> %t/headermap.hmap.json
+// RUN: %hmaptool write %t/headermap.hmap.json %t/headermap.hmap
+
+// Input headers use paths to the framework root/DSTROOT
+// RUN: %clang -extract-api --product-name=MyFramework -target arm64-apple-macosx \
+// RUN: -I%t/headermap.hmap -F%t/Frameworks \
+// RUN: %t/Frameworks/MyFramework.framework/Headers/MyFramework.h \
+// RUN: %t/Frameworks/MyFramework.framework/Headers/MyHeader.h \
+// RUN: -o %t/output.json | FileCheck -allow-empty %s
+
+// Generator version is not consistent across test runs, normalize it.
+// RUN: sed -e "s@\"generator\": \".*\"@\"generator\": \"?\"@g" \
+// RUN: %t/output.json >> %t/output-normalized.json
+// RUN: diff %t/reference.output.json %t/output-normalized.json
+
+// CHECK-NOT: error:
+// CHECK-NOT: warning:
+
+//--- headermap.hmap.json.in
+{
+  "mappings" :
+{
+ "MyFramework/MyHeader.h" : "SRCROOT/MyHeader.h"
+}
+}
+
+//--- MyFramework.h
+// Umbrella for MyFramework
+#import 
+
+//--- MyHeader.h
+#import 
+int MyInt;
+
+//--- Frameworks/OtherFramework.framework/Headers/OtherHeader.h
+int OtherInt;
+
+//--- reference.output.json.in
+{
+  "metadata": {
+"formatVersion": {
+  "major": 0,
+  "minor": 5,
+  "patch": 3
+},
+"generator": "?"
+  },
+  "module": {
+"name": "MyFramework",
+"platform": {
+  "architecture": "arm64",
+  "operatingSystem": {
+"minimumVersion": {
+  "major": 11,
+  "minor": 0,
+  "patch": 0
+},
+"name": "macosx"
+  },
+  "vendor": "apple"
+}
+  },
+  "relationships": [],
+  "symbols": [
+{
+  "accessLevel": "public",
+  "declarationFragments": [
+{
+  "kind": "typeIdentifier",
+  "preciseIdentifier": "c:I",
+  "spelling": "int"
+},
+{
+  "kind": "text",
+  "spelling": " "
+},
+{
+  "kind": "identifier",
+  "spelling": "MyInt"
+}
+  ],
+  "identifier": {
+"interfaceLanguage": "c",
+"precise": "c:@MyInt"
+  },
+  "kind": {
+"displayName": "Global Variable",
+"identifier": "c.var"
+  },
+  "location": {
+"position": {
+  "character": 5,
+  "line": 2
+},
+"uri": "file://SRCROOT/MyHeader.h"
+  },
+  "names": {
+"navigator": [
+  {
+"kind": "identifier",
+"spelling": "MyInt"
+  }
+],
+"subHeading": [
+  {
+"kind": "identifier",
+"spelling": "MyInt"
+  }
+],
+"title": "MyInt"
+  },
+  "pathComponents": [
+"MyInt"
+  ]
+}
+  ]
+}
Index: clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
===
--- clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
+++ clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
@@ -38,7 +38,10 @@
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallVector.h"
+#include "llvm/Support/FileSystem.h"
 #include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/Path.h"
+#include "llvm/Support/Regex.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
 #include 
@@ -55,10 +58,128 @@
   return {};
 }
 
+Optional getRelativeIncludeName(const CompilerInstance &CI,
+ StringRef File) {
+  using namespace llvm::sys;
+  // Matches framework include patterns
+  const llvm::Regex Rule("/(.+)\\.framework/(.+)?Headers/(.+)");
+
+  // FIXME: WorkingDir might not be set at this point.
+  StringRef WorkingDir = CI.getFileSystemOpts().WorkingDir;
+
+  SmallString<128> FilePath(Fi

[clang] ef7439b - [Basic] SourceManager docs: macro expansion SLocs aren't a single token. NFC

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2022-04-25T19:57:47+02:00
New Revision: ef7439bdf923e59095449c3bc8cfe72d8101eea5

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

LOG: [Basic] SourceManager docs: macro expansion SLocs aren't a single token. 
NFC

And haven't been since 2011: 
https://github.com/llvm/llvm-project/commit/eeca36fe9ad767380b2eab76a6fe5ba410a47393

Added: 


Modified: 
clang/include/clang/Basic/SourceManager.h
clang/lib/AST/ASTImporter.cpp
clang/lib/Basic/SourceManager.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/SourceManager.h 
b/clang/include/clang/Basic/SourceManager.h
index 3f3f1bb65c2c1..73e6353109d92 100644
--- a/clang/include/clang/Basic/SourceManager.h
+++ b/clang/include/clang/Basic/SourceManager.h
@@ -900,22 +900,26 @@ class SourceManager : public 
RefCountedBase {
   FileID getOrCreateFileID(const FileEntry *SourceFile,
SrcMgr::CharacteristicKind FileCharacter);
 
-  /// Return a new SourceLocation that encodes the
-  /// fact that a token from SpellingLoc should actually be referenced from
-  /// ExpansionLoc, and that it represents the expansion of a macro argument
-  /// into the function-like macro body.
-  SourceLocation createMacroArgExpansionLoc(SourceLocation Loc,
+  /// Creates an expansion SLocEntry for the substitution of an argument into a
+  /// function-like macro's body. Returns the start of the expansion.
+  ///
+  /// The macro argument was written at \p SpellingLoc with length \p Length.
+  /// \p ExpansionLoc is the parameter name in the (expanded) macro body.
+  SourceLocation createMacroArgExpansionLoc(SourceLocation SpellingLoc,
 SourceLocation ExpansionLoc,
-unsigned TokLength);
+unsigned Length);
 
-  /// Return a new SourceLocation that encodes the fact
-  /// that a token from SpellingLoc should actually be referenced from
-  /// ExpansionLoc.
-  SourceLocation
-  createExpansionLoc(SourceLocation Loc, SourceLocation ExpansionLocStart,
- SourceLocation ExpansionLocEnd, unsigned TokLength,
- bool ExpansionIsTokenRange = true, int LoadedID = 0,
- SourceLocation::UIntTy LoadedOffset = 0);
+  /// Creates an expansion SLocEntry for a macro use. Returns its start.
+  ///
+  /// The macro body begins at \p SpellingLoc with length \p Length.
+  /// The macro use spans [ExpansionLocStart, ExpansionLocEnd].
+  SourceLocation createExpansionLoc(SourceLocation SpellingLoc,
+SourceLocation ExpansionLocStart,
+SourceLocation ExpansionLocEnd,
+unsigned Length,
+bool ExpansionIsTokenRange = true,
+int LoadedID = 0,
+SourceLocation::UIntTy LoadedOffset = 0);
 
   /// Return a new SourceLocation that encodes that the token starting
   /// at \p TokenStart ends prematurely at \p TokenEnd.
@@ -1803,7 +1807,7 @@ class SourceManager : public 
RefCountedBase {
   /// the SLocEntry table and producing a source location that refers to it.
   SourceLocation
   createExpansionLocImpl(const SrcMgr::ExpansionInfo &Expansion,
- unsigned TokLength, int LoadedID = 0,
+ unsigned Length, int LoadedID = 0,
  SourceLocation::UIntTy LoadedOffset = 0);
 
   /// Return true if the specified FileID contains the

diff  --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index 537ccc22ed050..e2294088908c5 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -9293,13 +9293,13 @@ Expected ASTImporter::Import(FileID FromID, 
bool IsBuiltin) {
 ExpectedSLoc ToExLocS = Import(FromEx.getExpansionLocStart());
 if (!ToExLocS)
   return ToExLocS.takeError();
-unsigned TokenLen = FromSM.getFileIDSize(FromID);
+unsigned ExLength = FromSM.getFileIDSize(FromID);
 SourceLocation MLoc;
 if (FromEx.isMacroArgExpansion()) {
-  MLoc = ToSM.createMacroArgExpansionLoc(*ToSpLoc, *ToExLocS, TokenLen);
+  MLoc = ToSM.createMacroArgExpansionLoc(*ToSpLoc, *ToExLocS, ExLength);
 } else {
   if (ExpectedSLoc ToExLocE = Import(FromEx.getExpansionLocEnd()))
-MLoc = ToSM.createExpansionLoc(*ToSpLoc, *ToExLocS, *ToExLocE, 
TokenLen,
+MLoc = ToSM.createExpansionLoc(*ToSpLoc, *ToExLocS, *ToExLocE, 
ExLength,
FromEx.isExpansionTokenRange());
   else
 return ToExLocE.takeError();

diff  --git a/clang/lib/Basic/SourceManager.cpp 
b/clan

[PATCH] D119544: Deferred Concept Instantiation Implementation

[Erich Keane via Phabricator via cfe-commits]

erichkeane updated this revision to Diff 424963.
erichkeane added a comment.
This revision is now accepted and ready to land.

Here is my current 'WIP' patch, with only `ChecksMemberVar::foo` failing.  I 
noticed that the problem so far starts in `TransformMemberExpr`, which calls 
`TransformDecl` (TreeTransform.h: ~11018). THAT ends up being a 'pass through' 
and just calls `SemaRef.FindInstantiatedDecl`, which attempts to get the 
instantiated version of `S` in its call to `FindInstantiatedContext` (see 
SemaTemplateInstantiateDecl.cpp:6160).

THAT calls `FindInstantiatedDecl` again on the `ParentDC` (which is the 
uninstantiated `CXXRecordDecl` of `S`), and ends up going down past the 
`ClassTemplate` calculation (~6083, which is DeclContext *DC = CurContext), and 
everything is the same up to there.

That is the point that I have ended looking into it at the moment, but I think 
the problem/or at least something that should suggest the fix is there, since 
it manages to instead return the primary template  instead of the 
instantiation-of-int.


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

https://reviews.llvm.org/D119544

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/AST/Decl.h
  clang/include/clang/AST/DeclBase.h
  clang/include/clang/Sema/Sema.h
  clang/include/clang/Sema/Template.h
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/Decl.cpp
  clang/lib/AST/DeclBase.cpp
  clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
  clang/lib/Sema/SemaConcept.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/lib/Sema/SemaTemplateDeduction.cpp
  clang/lib/Sema/SemaTemplateInstantiate.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/test/CXX/temp/temp.constr/temp.constr.constr/non-function-templates.cpp
  clang/test/SemaTemplate/concepts.cpp
  clang/test/SemaTemplate/deferred-concept-inst.cpp
  clang/test/SemaTemplate/instantiate-requires-clause.cpp
  clang/test/SemaTemplate/trailing-return-short-circuit.cpp

Index: clang/test/SemaTemplate/trailing-return-short-circuit.cpp
===
--- /dev/null
+++ clang/test/SemaTemplate/trailing-return-short-circuit.cpp
@@ -0,0 +1,62 @@
+// RUN: %clang_cc1 -std=c++20 -verify %s
+
+template 
+  requires(sizeof(T) > 2) || T::value // #FOO_REQ
+void Foo(T){};// #FOO
+
+template 
+void TrailingReturn(T)   // #TRAILING
+  requires(sizeof(T) > 2) || // #TRAILING_REQ
+  T::value   // #TRAILING_REQ_VAL
+{};
+template 
+struct HasValue {
+  static constexpr bool value = B;
+};
+static_assert(sizeof(HasValue) <= 2);
+
+template 
+struct HasValueLarge {
+  static constexpr bool value = B;
+  int I;
+};
+static_assert(sizeof(HasValueLarge) > 2);
+
+void usage() {
+  // Passes the 1st check, short-circuit so the 2nd ::value is not evaluated.
+  Foo(1.0);
+  TrailingReturn(1.0);
+
+  // Fails the 1st check, but has a ::value, so the check happens correctly.
+  Foo(HasValue{});
+  TrailingReturn(HasValue{});
+
+  // Passes the 1st check, but would have passed the 2nd one.
+  Foo(HasValueLarge{});
+  TrailingReturn(HasValueLarge{});
+
+  // Fails the 1st check, fails 2nd because there is no ::value.
+  Foo(true);
+  // expected-error@-1{{no matching function for call to 'Foo'}}
+  // expected-note@#FOO{{candidate template ignored: constraints not satisfied [with T = bool]}}
+  // expected-note@#FOO_REQ{{because 'sizeof(_Bool) > 2' (1 > 2) evaluated to false}}
+  // expected-note@#FOO_REQ{{because substituted constraint expression is ill-formed: type 'bool' cannot be used prior to '::' because it has no members}}
+
+  TrailingReturn(true);
+  // expected-error@-1{{no matching function for call to 'TrailingReturn'}}
+  // expected-note@#TRAILING{{candidate template ignored: constraints not satisfied [with T = bool]}}
+  // expected-note@#TRAILING_REQ{{because 'sizeof(_Bool) > 2' (1 > 2) evaluated to false}}
+  // expected-note@#TRAILING_REQ_VAL{{because substituted constraint expression is ill-formed: type 'bool' cannot be used prior to '::' because it has no members}}
+
+  // Fails the 1st check, fails 2nd because ::value is false.
+  Foo(HasValue{});
+  // expected-error@-1 {{no matching function for call to 'Foo'}}
+  // expected-note@#FOO{{candidate template ignored: constraints not satisfied [with T = HasValue]}}
+  // expected-note@#FOO_REQ{{because 'sizeof(HasValue) > 2' (1 > 2) evaluated to false}}
+  // expected-note@#FOO_REQ{{and 'HasValue::value' evaluated to false}}
+  TrailingReturn(HasValue{});
+  // expected-error@-1 {{no matching function for call to 'TrailingReturn'}}
+  // expected-note@#TRAILING{{candidate template ignored: constraints not satisfied [with T = HasValue]}}
+  // expected-note@#TRAILING_REQ{{because 'sizeof(HasValue) > 2' (1 > 2) evaluated to false}}
+  // expected-note@#TRAILING_REQ_VAL{{and 'HasValue::val

[PATCH] D124396: [HIP] Fix diag msg about sanitizer

[Artem Belevich via Phabricator via cfe-commits]

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

Wording nit. LGTM otherwise.




Comment at: clang/include/clang/Basic/DiagnosticDriverKinds.td:113-114
 def warn_drv_unsupported_option_for_offload_arch_req_feature : Warning<
-  "ignoring '%0' option as it is not currently supported for "
-  "offload arch '%1'. Use it with an offload arch containing '%2' instead">,
+  "ignoring '%0' option for offload arch '%1' as it is not currently supported 
for "
+  "that offload arch. Use it with an offload arch containing '%2' instead">,
   InGroup;

'there' would be more concise and less repetitive.


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

https://reviews.llvm.org/D124396

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

[PATCH] D119544: Deferred Concept Instantiation Implementation

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

Here is the example I'm playing with:

  template
  constexpr bool constraint = true;
  
  #define BROKE_REQUIRES 1
  #define WORKING_REQUIRES 2
  #define WORKING_BODY 3
   
  #define TEST BROKE_REQUIRES
  //#define TEST WORKING_REQUIRES
  //#define TEST WORKING_BODY
   
  template
  struct S {
T t;
void foo()
  #if TEST == BROKE_REQUIRES
  requires (constraint)
  #endif
  {
  #if TEST == WORKING_BODY
  using G = decltype(t);
  #endif
}
  #if TEST == WORKING_REQUIRES
template
void foo2() requires (constraint){}
  #endif
  };
   
  void bar() {
S s;
  #if TEST == BROKE_REQUIRES || TEST == WORKING_BODY
s.foo();
  #endif
  #if TEST == WORKING_REQUIRES
s.foo2();
  #endif
  }

For all 3 versions (see the definitions of TEST), they get to 6083 on the 1st 
breakpoint.


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

https://reviews.llvm.org/D119544

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

[PATCH] D119544: Deferred Concept Instantiation Implementation

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

Alright, a touch more:  It looks like the looping through the `CurContext` at 
line 6084 finds the wrong thing in evaluating this `CXXMethodDecl`!  The 
"broken" version has its CurContext as `bar` in the example above, the 
"working" versions have `foo` or `foo2`.  Therefore the 2nd time through 
the loop, those both end up on the `ClassTemplateSpecializationDecl`, but the 
broken one finds the `isFileContext` version.

So I don't know HOW to fix it, but it seems that the `CheckFunctionConstraints` 
probably has to change the `CurContext`.  I suspect the instantiation of the 
`WorkingRequires` does something like this correctly.

I'm poking on/off on it today, but if @ChuanqiXu has an idea/help, it would be 
greatly appreciated.


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

https://reviews.llvm.org/D119544

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

[PATCH] D124396: [HIP] Fix diag msg about sanitizer

[Yaxun Liu via Phabricator via cfe-commits]

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



Comment at: clang/include/clang/Basic/DiagnosticDriverKinds.td:113-114
 def warn_drv_unsupported_option_for_offload_arch_req_feature : Warning<
-  "ignoring '%0' option as it is not currently supported for "
-  "offload arch '%1'. Use it with an offload arch containing '%2' instead">,
+  "ignoring '%0' option for offload arch '%1' as it is not currently supported 
for "
+  "that offload arch. Use it with an offload arch containing '%2' instead">,
   InGroup;

tra wrote:
> 'there' would be more concise and less repetitive.
will do when committing


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

https://reviews.llvm.org/D124396

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

[clang] 87468e8 - compile commands header to source heuristic lower-cases filenames before inferring file types

[Sam McCall via cfe-commits]

Author: Ishaan Gandhi
Date: 2022-04-25T20:40:56+02:00
New Revision: 87468e85fcdcf1bd5055466e34d17436a79017a2

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

LOG: compile commands header to source heuristic lower-cases filenames before 
inferring file types

This leads to ".C" files being rewritten as ".c" files and being inferred to be 
"c" files as opposed to "c++" files.

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

Reviewed By: sammccall

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

Added: 


Modified: 
clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
clang/unittests/Tooling/CompilationDatabaseTest.cpp

Removed: 




diff  --git a/clang/lib/Tooling/InterpolatingCompilationDatabase.cpp 
b/clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
index 51e8439b6b79b..0143b5f8df6b6 100644
--- a/clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
+++ b/clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
@@ -328,7 +328,7 @@ class FileIndex {
   StringRef Path = Strings.save(StringRef(OriginalPaths[I]).lower());
 
   Paths.emplace_back(Path, I);
-  Types.push_back(foldType(guessType(Path)));
+  Types.push_back(foldType(guessType(OriginalPaths[I])));
   Stems.emplace_back(sys::path::stem(Path), I);
   auto Dir = ++sys::path::rbegin(Path), DirEnd = sys::path::rend(Path);
   for (int J = 0; J < DirectorySegmentsIndexed && Dir != DirEnd; ++J, 
++Dir)

diff  --git a/clang/unittests/Tooling/CompilationDatabaseTest.cpp 
b/clang/unittests/Tooling/CompilationDatabaseTest.cpp
index adb9de0c1cbaa..8194dd3953c20 100644
--- a/clang/unittests/Tooling/CompilationDatabaseTest.cpp
+++ b/clang/unittests/Tooling/CompilationDatabaseTest.cpp
@@ -836,6 +836,14 @@ TEST_F(InterpolateTest, Case) {
   EXPECT_EQ(getProxy("foo/bar/baz/shout.C"), "FOO/BAR/BAZ/SHOUT.cc");
 }
 
+TEST_F(InterpolateTest, LanguagePreference) {
+  add("foo/bar/baz/exact.C");
+  add("foo/bar/baz/exact.c");
+  add("other/random/path.cpp");
+  // Proxies for ".H" files are ".C" files, and not ".c files".
+  EXPECT_EQ(getProxy("foo/bar/baz/exact.H"), "foo/bar/baz/exact.C");
+}
+
 TEST_F(InterpolateTest, Aliasing) {
   add("foo.cpp", "-faligned-new");
 



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