Re: r367055 - Revert "Revert "CodeGen: ensure placeholder instruction for cleanup is created""

[Hans Wennborg via cfe-commits]

Merged to release_90 in r367209.

On Thu, Jul 25, 2019 at 10:59 PM Saleem Abdulrasool via cfe-commits
 wrote:
>
> Author: compnerd
> Date: Thu Jul 25 13:59:48 2019
> New Revision: 367055
>
> URL: http://llvm.org/viewvc/llvm-project?rev=367055&view=rev
> Log:
> Revert "Revert "CodeGen: ensure placeholder instruction for cleanup is 
> created""
>
> This reverts commit fd1274fa78cb0fd32cc1fa2e6f5bb8e62d29df19.
>
> Add an explicit triple for the test which is pattern matching overly
> aggressively.
>
> Added:
> cfe/trunk/test/CodeGenCXX/pr40771-ctad-with-lambda-copy-capture.cpp
> Modified:
> cfe/trunk/lib/CodeGen/CGExprAgg.cpp
>
> Modified: cfe/trunk/lib/CodeGen/CGExprAgg.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExprAgg.cpp?rev=367055&r1=367054&r2=367055&view=diff
> ==
> --- cfe/trunk/lib/CodeGen/CGExprAgg.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CGExprAgg.cpp Thu Jul 25 13:59:48 2019
> @@ -1495,6 +1495,13 @@ void AggExprEmitter::VisitInitListExpr(I
>// initializers throws an exception.
>SmallVector cleanups;
>llvm::Instruction *cleanupDominator = nullptr;
> +  auto addCleanup = [&](const EHScopeStack::stable_iterator &cleanup) {
> +cleanups.push_back(cleanup);
> +if (!cleanupDominator) // create placeholder once needed
> +  cleanupDominator = CGF.Builder.CreateAlignedLoad(
> +  CGF.Int8Ty, llvm::Constant::getNullValue(CGF.Int8PtrTy),
> +  CharUnits::One());
> +  };
>
>unsigned curInitIndex = 0;
>
> @@ -1519,7 +1526,7 @@ void AggExprEmitter::VisitInitListExpr(I
>if (QualType::DestructionKind dtorKind =
>Base.getType().isDestructedType()) {
>  CGF.pushDestroy(dtorKind, V, Base.getType());
> -cleanups.push_back(CGF.EHStack.stable_begin());
> +addCleanup(CGF.EHStack.stable_begin());
>}
>  }
>}
> @@ -1596,15 +1603,9 @@ void AggExprEmitter::VisitInitListExpr(I
>= field->getType().isDestructedType()) {
>assert(LV.isSimple());
>if (CGF.needsEHCleanup(dtorKind)) {
> -if (!cleanupDominator)
> -  cleanupDominator = CGF.Builder.CreateAlignedLoad(
> -  CGF.Int8Ty,
> -  llvm::Constant::getNullValue(CGF.Int8PtrTy),
> -  CharUnits::One()); // placeholder
> -
>  CGF.pushDestroy(EHCleanup, LV.getAddress(), field->getType(),
>  CGF.getDestroyer(dtorKind), false);
> -cleanups.push_back(CGF.EHStack.stable_begin());
> +addCleanup(CGF.EHStack.stable_begin());
>  pushedCleanup = true;
>}
>  }
> @@ -1620,6 +1621,8 @@ void AggExprEmitter::VisitInitListExpr(I
>
>// Deactivate all the partial cleanups in reverse order, which
>// generally means popping them.
> +  assert((cleanupDominator || cleanups.empty()) &&
> + "Missing cleanupDominator before deactivating cleanup blocks");
>for (unsigned i = cleanups.size(); i != 0; --i)
>  CGF.DeactivateCleanupBlock(cleanups[i-1], cleanupDominator);
>
>
> Added: cfe/trunk/test/CodeGenCXX/pr40771-ctad-with-lambda-copy-capture.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/pr40771-ctad-with-lambda-copy-capture.cpp?rev=367055&view=auto
> ==
> --- cfe/trunk/test/CodeGenCXX/pr40771-ctad-with-lambda-copy-capture.cpp 
> (added)
> +++ cfe/trunk/test/CodeGenCXX/pr40771-ctad-with-lambda-copy-capture.cpp Thu 
> Jul 25 13:59:48 2019
> @@ -0,0 +1,55 @@
> +// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm --std=c++17 
> -fcxx-exceptions -fexceptions -discard-value-names %s -o - | FileCheck %s
> +
> +struct Q { Q(); };
> +struct R { R(Q); ~R(); };
> +struct S { S(Q); ~S(); };
> +struct T : R, S {};
> +
> +Q q;
> +T t { R{q}, S{q} };
> +
> +// CHECK-LABEL: define internal void @__cxx_global_var_init.1() {{.*}} {
> +// CHECK-NEXT: [[TMP_R:%[a-z0-9.]+]] = alloca %struct.R, align 1
> +// CHECK-NEXT: [[TMP_Q1:%[a-z0-9.]+]] = alloca %struct.Q, align 1
> +// CHECK-NEXT: [[TMP_S:%[a-z0-9.]+]] = alloca %struct.S, align 1
> +// CHECK-NEXT: [[TMP_Q2:%[a-z0-9.]+]] = alloca %struct.Q, align 1
> +// CHECK-NEXT: [[XPT:%[a-z0-9.]+]] = alloca i8*
> +// CHECK-NEXT: [[SLOT:%[a-z0-9.]+]] = alloca i32
> +// CHECK-NEXT: [[ACTIVE:%[a-z0-9.]+]] = alloca i1, align 1
> +// CHECK-NEXT: call void @_ZN1RC1E1Q(%struct.R* [[TMP_R]])
> +// CHECK-NEXT: store i1 true, i1* [[ACTIVE]], align 1
> +// CHECK-NEXT: invoke void @_ZN1SC1E1Q(%struct.S* [[TMP_S]])
> +// CHECK-NEXT:   to label %[[L1:[a-z0-9.]+]] unwind label %[[L2:[a-z0-9.]+]]
> +// CHECK-EMPTY:
> +// CHECK-NEXT: [[L1]]:
> +// CHECK-NEXT: store i1 false, i1* [[ACTIVE]], align 1
> +// CHECK-NEXT: call void @_ZN1SD1Ev(%struct.S*
> +// CHECK-NEXT: call void @_ZN1RD1Ev(%struct.R*
> +// CHECK-NEXT: [[EXIT:%[a-z0-9.]+]] = call i32 @__cxa_atexit(
> +// CHECK-NEXT: ret void
> +// CHECK-EMPT

r367219 - doc: Fix Google C++ Style Guide link.

[Rafael Stahl via cfe-commits]

Author: r.stahl
Date: Mon Jul 29 04:00:23 2019
New Revision: 367219

URL: http://llvm.org/viewvc/llvm-project?rev=367219&view=rev
Log:
doc: Fix Google C++ Style Guide link.

Modified:
cfe/trunk/docs/ClangFormatStyleOptions.rst

Modified: cfe/trunk/docs/ClangFormatStyleOptions.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ClangFormatStyleOptions.rst?rev=367219&r1=367218&r2=367219&view=diff
==
--- cfe/trunk/docs/ClangFormatStyleOptions.rst (original)
+++ cfe/trunk/docs/ClangFormatStyleOptions.rst Mon Jul 29 04:00:23 2019
@@ -138,7 +138,7 @@ the configuration (without a prefix: ``A
 `_
   * ``Google``
 A style complying with `Google's C++ style guide
-`_
+`_
   * ``Chromium``
 A style complying with `Chromium's style guide
 `_


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

[libunwind] r367223 - Creating release candidate rc1 from release_900 branch

[Hans Wennborg via cfe-commits]

Author: hans
Date: Mon Jul 29 06:02:31 2019
New Revision: 367223

URL: http://llvm.org/viewvc/llvm-project?rev=367223&view=rev
Log:
Creating release candidate rc1 from release_900 branch

Added:
libunwind/tags/RELEASE_900/
libunwind/tags/RELEASE_900/rc1/
  - copied from r367222, libunwind/branches/release_90/

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

[libclc] r367223 - Creating release candidate rc1 from release_900 branch

[Hans Wennborg via cfe-commits]

Author: hans
Date: Mon Jul 29 06:02:31 2019
New Revision: 367223

URL: http://llvm.org/viewvc/llvm-project?rev=367223&view=rev
Log:
Creating release candidate rc1 from release_900 branch

Added:
libclc/tags/RELEASE_900/
libclc/tags/RELEASE_900/rc1/
  - copied from r367222, libclc/branches/release_90/

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

r367225 - clang-format clang/lib/Format

[Nico Weber via cfe-commits]

Author: nico
Date: Mon Jul 29 06:26:48 2019
New Revision: 367225

URL: http://llvm.org/viewvc/llvm-project?rev=367225&view=rev
Log:
clang-format clang/lib/Format

Modified:
cfe/trunk/lib/Format/BreakableToken.cpp
cfe/trunk/lib/Format/Format.cpp
cfe/trunk/lib/Format/TokenAnnotator.h
cfe/trunk/lib/Format/UnwrappedLineParser.cpp
cfe/trunk/lib/Format/WhitespaceManager.cpp

Modified: cfe/trunk/lib/Format/BreakableToken.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/BreakableToken.cpp?rev=367225&r1=367224&r2=367225&view=diff
==
--- cfe/trunk/lib/Format/BreakableToken.cpp (original)
+++ cfe/trunk/lib/Format/BreakableToken.cpp Mon Jul 29 06:26:48 2019
@@ -342,8 +342,8 @@ BreakableBlockComment::BreakableBlockCom
 
   StringRef TokenText(Tok.TokenText);
   assert(TokenText.startswith("/*") && TokenText.endswith("*/"));
-  TokenText.substr(2, TokenText.size() - 4).split(Lines,
-  UseCRLF ? "\r\n" : "\n");
+  TokenText.substr(2, TokenText.size() - 4)
+  .split(Lines, UseCRLF ? "\r\n" : "\n");
 
   int IndentDelta = StartColumn - OriginalStartColumn;
   Content.resize(Lines.size());
@@ -456,10 +456,9 @@ BreakableBlockComment::BreakableBlockCom
   });
 }
 
-BreakableToken::Split
-BreakableBlockComment::getSplit(unsigned LineIndex, unsigned TailOffset,
-   unsigned ColumnLimit, unsigned ContentStartColumn,
-   llvm::Regex &CommentPragmasRegex) const {
+BreakableToken::Split BreakableBlockComment::getSplit(
+unsigned LineIndex, unsigned TailOffset, unsigned ColumnLimit,
+unsigned ContentStartColumn, llvm::Regex &CommentPragmasRegex) const {
   // Don't break lines matching the comment pragmas regex.
   if (CommentPragmasRegex.match(Content[LineIndex]))
 return Split(StringRef::npos, 0);

Modified: cfe/trunk/lib/Format/Format.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=367225&r1=367224&r2=367225&view=diff
==
--- cfe/trunk/lib/Format/Format.cpp (original)
+++ cfe/trunk/lib/Format/Format.cpp Mon Jul 29 06:26:48 2019
@@ -608,8 +608,8 @@ static FormatStyle expandPresets(const F
 return Style;
   FormatStyle Expanded = Style;
   Expanded.BraceWrapping = {false, false, false, false, false, false,
-false, false, false, false, false,
-false, false, true,  true,  true};
+false, false, false, false, false, false,
+false, true,  true,  true};
   switch (Style.BreakBeforeBraces) {
   case FormatStyle::BS_Linux:
 Expanded.BraceWrapping.AfterClass = true;
@@ -687,8 +687,8 @@ FormatStyle getLLVMStyle(FormatStyle::La
   LLVMStyle.BreakBeforeTernaryOperators = true;
   LLVMStyle.BreakBeforeBraces = FormatStyle::BS_Attach;
   LLVMStyle.BraceWrapping = {false, false, false, false, false, false,
- false, false, false, false, false,
- false, false, true,  true,  true};
+ false, false, false, false, false, false,
+ false, true,  true,  true};
   LLVMStyle.BreakAfterJavaFieldAnnotations = false;
   LLVMStyle.BreakConstructorInitializers = FormatStyle::BCIS_BeforeColon;
   LLVMStyle.BreakInheritanceList = FormatStyle::BILS_BeforeColon;
@@ -1774,8 +1774,8 @@ FindCursorIndex(const SmallVectorImpl &Includes,
 ArrayRef Ranges, StringRef 
FileName,
-StringRef Code,
-tooling::Replacements &Replaces, unsigned *Cursor) 
{
+StringRef Code, tooling::Replacements &Replaces,
+unsigned *Cursor) {
   unsigned IncludesBeginOffset = Includes.front().Offset;
   unsigned IncludesEndOffset =
   Includes.back().Offset + Includes.back().Text.size();

Modified: cfe/trunk/lib/Format/TokenAnnotator.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.h?rev=367225&r1=367224&r2=367225&view=diff
==
--- cfe/trunk/lib/Format/TokenAnnotator.h (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.h Mon Jul 29 06:26:48 2019
@@ -114,8 +114,7 @@ public:
 
   /// \c true if this line starts a namespace definition.
   bool startsWithNamespace() const {
-return startsWith(tok::kw_namespace) ||
-   startsWith(TT_NamespaceMacro) ||
+return startsWith(tok::kw_namespace) || startsWith(TT_NamespaceMacro) ||
startsWith(tok::kw_inline, tok::kw_namespace) ||
startsWith(tok::kw_export, tok::kw_namespace);
   }

Modified: cfe/trunk/lib/Format/UnwrappedLineParser.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/UnwrappedLineParser.cpp

[PATCH] D64416: [AArch64] Add support for Transactional Memory Extension (TME)

[Momchil Velikov via Phabricator via cfe-commits]

chill added a comment.

If there are no objections, I'll go ahead with committing that Soon(tm) on the 
basis of previous acceptance.


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

https://reviews.llvm.org/D64416



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

[PATCH] D65387: [clangd] Add a callback mechanism for handling responses from client.

[Haojian Wu via Phabricator via cfe-commits]

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

The callback will be invoked in clangd when we receive a reply from the client.

unfortunately, we don't have any unittest for ClangdLSPServer .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D65387

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdLSPServer.h


Index: clang-tools-extra/clangd/ClangdLSPServer.h
===
--- clang-tools-extra/clangd/ClangdLSPServer.h
+++ clang-tools-extra/clangd/ClangdLSPServer.h
@@ -146,7 +146,16 @@
   std::unique_ptr MsgHandler;
   std::atomic NextCallID = {0};
   std::mutex TranspWriter;
-  void call(StringRef Method, llvm::json::Value Params);
+
+  // A callback that will be invoked when receiving a client reply for a server
+  // request.
+  using ReplyCallback = std::function)>;
+  std::mutex ReplyCallbacksMutex;
+  // Request ID => reply Callbacks
+  llvm::StringMap ReplyCallbacks;
+
+  void call(StringRef Method, llvm::json::Value Params, ReplyCallback CB);
+  void onReply(llvm::json::Value ID, llvm::Expected Result);
   void notify(StringRef Method, llvm::json::Value Params);
 
   const FileSystemProvider &FSProvider;
Index: clang-tools-extra/clangd/ClangdLSPServer.cpp
===
--- clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -146,11 +146,9 @@
   bool onReply(llvm::json::Value ID,
llvm::Expected Result) override {
 WithContext HandlerContext(handlerContext());
-// We ignore replies, just log them.
-if (Result)
-  log("<-- reply({0})", ID);
-else
-  log("<-- reply({0}) error: {1}", ID, llvm::toString(Result.takeError()));
+// Log the reply.
+log("<-- reply({0})", ID);
+Server.onReply(std::move(ID), std::move(Result));
 return true;
   }
 
@@ -310,14 +308,40 @@
 };
 
 // call(), notify(), and reply() wrap the Transport, adding logging and 
locking.
-void ClangdLSPServer::call(llvm::StringRef Method, llvm::json::Value Params) {
+void ClangdLSPServer::call(llvm::StringRef Method, llvm::json::Value Params,
+   ReplyCallback CB) {
   auto ID = NextCallID++;
+  auto StrID = llvm::to_string(ID);
+  {
+std::lock_guard Lock(ReplyCallbacksMutex);
+ReplyCallbacks[StrID] = std::move(CB);
+  }
   log("--> {0}({1})", Method, ID);
-  // We currently don't handle responses, so no need to store ID anywhere.
   std::lock_guard Lock(TranspWriter);
   Transp.call(Method, std::move(Params), ID);
 }
 
+void ClangdLSPServer::onReply(llvm::json::Value ID,
+  llvm::Expected Result) {
+  auto StrID = llvm::to_string(ID);
+  ReplyCallback CB = nullptr;
+  {
+std::lock_guard Lock(ReplyCallbacksMutex);
+CB = ReplyCallbacks.lookup(StrID);
+ReplyCallbacks.erase(StrID);
+  }
+  // No callback for the client reply, just log them.
+  if (!CB) {
+if (Result)
+  log("no corresponding callback for the reply ({0})", ID);
+else
+  log("no corresponding callback for the reply ({0}) error: {1}", ID,
+  llvm::toString(Result.takeError()));
+return;
+  }
+  CB(std::move(Result));
+}
+
 void ClangdLSPServer::notify(llvm::StringRef Method, llvm::json::Value Params) 
{
   log("--> {0}", Method);
   std::lock_guard Lock(TranspWriter);
@@ -501,7 +525,7 @@
 Edit.edit = std::move(WE);
 // Ideally, we would wait for the response and if there is no error, we
 // would reply success/failure to the original RPC.
-call("workspace/applyEdit", Edit);
+call("workspace/applyEdit", Edit, /*ReplyCallback=*/nullptr);
   };
   if (Params.command == ExecuteCommandParams::CLANGD_APPLY_FIX_COMMAND &&
   Params.workspaceEdit) {


Index: clang-tools-extra/clangd/ClangdLSPServer.h
===
--- clang-tools-extra/clangd/ClangdLSPServer.h
+++ clang-tools-extra/clangd/ClangdLSPServer.h
@@ -146,7 +146,16 @@
   std::unique_ptr MsgHandler;
   std::atomic NextCallID = {0};
   std::mutex TranspWriter;
-  void call(StringRef Method, llvm::json::Value Params);
+
+  // A callback that will be invoked when receiving a client reply for a server
+  // request.
+  using ReplyCallback = std::function)>;
+  std::mutex ReplyCallbacksMutex;
+  // Request ID => reply Callbacks
+  llvm::StringMap ReplyCallbacks;
+
+  void call(StringRef Method, llvm::json::Value Params, ReplyCallback CB);
+  void onReply(llvm::json::Value ID, llvm::Expected Result);
   void notify(StringRef Method, llvm::json::Value Params);
 
   const FileSystemProvider &FSProvider;
Index: clang-tools-extra/clangd/ClangdLSPServer.cpp
===
--- clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ clan

[PATCH] D50256: [Analyzer] Basic support for multiplication and division in the constraint manager (for == and != only)

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

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



Comment at: test/Analysis/multiplicative-folding.c:140-142
+clang_analyzer_eval(n == -1); //expected-warning{{FALSE}}
+clang_analyzer_eval(n == 0); //expected-warning{{TRUE}}
+clang_analyzer_eval(n == 1); //expected-warning{{FALSE}}

NoQ wrote:
> Something's not right here. All three can potentially be true, because both 
> `1/2 == 0` and `(-1)/2 == 0`.
Thank you for noticing this! It is strange that I forgot about 0 for `==` and 
`!=` but not for the rest of the comparison operators in my subsequent patch. 
Now I fixed it.


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

https://reviews.llvm.org/D50256



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

[PATCH] D64696: Adds a warning when an inline Doxygen comment has no argument

[Dmitri Gribenko via Phabricator via cfe-commits]

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

Thank you! Do you have commit access or would you like me to commit this patch 
for you?


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

https://reviews.llvm.org/D64696



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

[PATCH] D50256: [Analyzer] Basic support for multiplication and division in the constraint manager (for == and != only)

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

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

Fixed error when the result of an integer division is compared to zero.


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

https://reviews.llvm.org/D50256

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h
  lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
  lib/StaticAnalyzer/Core/RangedConstraintManager.cpp
  test/Analysis/multiplicative-folding.c

Index: test/Analysis/multiplicative-folding.c
===
--- /dev/null
+++ test/Analysis/multiplicative-folding.c
@@ -0,0 +1,689 @@
+// RUN: %clang_analyze_cc1 --std=c11 -analyzer-checker=core,debug.ExprInspection -verify %s
+
+void clang_analyzer_eval(int);
+void clang_analyzer_warnIfReached(void);
+
+#define UINT_MAX (~0U)
+#define INT_MAX (int)(UINT_MAX & (UINT_MAX >> 1))
+#define INT_MIN (-INT_MAX - 1)
+
+#define ULONG_LONG_MAX (~0UL)
+#define LONG_LONG_MAX (long long)(ULONG_LONG_MAX & (ULONG_LONG_MAX >> 1))
+#define LONG_LONG_MIN (-LONG_LONG_MAX - 1)
+
+extern void __assert_fail (__const char *__assertion, __const char *__file,
+unsigned int __line, __const char *__function)
+ __attribute__ ((__noreturn__));
+#define assert(expr) \
+  ((expr)  ? (void)(0)  : __assert_fail (#expr, __FILE__, __LINE__, __func__))
+
+typedef int int32_t;
+typedef unsigned int uint32_t;
+typedef long long int64_t;
+typedef unsigned long long uint64_t;
+
+void signed_multiplication_eq(int32_t n) {
+  if (n * 2 == 3) {
+clang_analyzer_warnIfReached(); // no-warning
+
+  } else if (n * 2 == 4) {
+const int32_t C1 = 0x8002, C2 = 2;
+
+assert(C1 * 2 == 4);
+assert(C2 * 2 == 4);
+
+clang_analyzer_eval(n == INT_MIN); //expected-warning{{FALSE}}
+clang_analyzer_eval(n == C1 - 1); //expected-warning{{FALSE}}
+clang_analyzer_eval(n == C1); //expected-warning{{FALSE}}
+  //expected-warning@-1{{TRUE}}
+clang_analyzer_eval(n == C1 + 1); //expected-warning{{FALSE}}
+clang_analyzer_eval(n == 0); //expected-warning{{FALSE}}
+clang_analyzer_eval(n == C2 - 1); //expected-warning{{FALSE}}
+clang_analyzer_eval(n == C2); //expected-warning{{FALSE}}
+  //expected-warning@-1{{TRUE}}
+clang_analyzer_eval(n == C2 + 1); //expected-warning{{FALSE}}
+clang_analyzer_eval(n == INT_MAX); //expected-warning{{FALSE}}
+
+  } else if (n * 3 == 4) {
+const int32_t C1 = 0xaaac;
+
+assert(C1 * 3 == 4);
+
+clang_analyzer_eval(n == INT_MIN); //expected-warning{{FALSE}}
+clang_analyzer_eval(n == C1 - 1); //expected-warning{{FALSE}}
+clang_analyzer_eval(n == C1); //expected-warning{{TRUE}}
+clang_analyzer_eval(n == C1 + 1); //expected-warning{{FALSE}}
+clang_analyzer_eval(n == 0); //expected-warning{{FALSE}}
+clang_analyzer_eval(n == INT_MAX); //expected-warning{{FALSE}}
+
+  } else if (n * 4 == -5) {
+clang_analyzer_warnIfReached(); // no-warning
+
+  } else if (n * 4 == -8) {
+const int32_t C1 = 0xbffe, C2 = 0xfffe,
+  C3 = 0x3ffe, C4 = 0x7ffe;
+
+assert(C1 * 4 == -8);
+assert(C2 * 4 == -8);
+assert(C3 * 4 == -8);
+assert(C4 * 4 == -8);
+
+clang_analyzer_eval(n == INT_MIN); //expected-warning{{FALSE}}
+clang_analyzer_eval(n == C1 - 1); //expected-warning{{FALSE}}
+clang_analyzer_eval(n == C1); //expected-warning{{FALSE}}
+  //expected-warning@-1{{TRUE}}
+clang_analyzer_eval(n == C1 + 1); //expected-warning{{FALSE}}
+clang_analyzer_eval(n == C2 - 1); //expected-warning{{FALSE}}
+clang_analyzer_eval(n == C2); //expected-warning{{FALSE}}
+  //expected-warning@-1{{TRUE}}
+clang_analyzer_eval(n == C2 + 1); //expected-warning{{FALSE}}
+clang_analyzer_eval(n == 0); //expected-warning{{FALSE}}
+clang_analyzer_eval(n == C3 - 1); //expected-warning{{FALSE}}
+clang_analyzer_eval(n == C3); //expected-warning{{FALSE}}
+  //expected-warning@-1{{TRUE}}
+clang_analyzer_eval(n == C3 + 1); //expected-warning{{FALSE}}
+clang_analyzer_eval(n == C4 - 1); //expected-warning{{FALSE}}
+clang_analyzer_eval(n == C4); //expected-warning{{FALSE}}
+  //expected-warning@-1{{TRUE}}
+clang_analyzer_eval(n == C4 + 1); //expected-warning{{FALSE}}
+clang_analyzer_eval(n == INT_MAX); //expected-warning{{FALSE}}
+
+  } else if (n * 6 == -7) {
+clang_analyzer_warnIfReached(); // no-warning
+
+  } else if (n * 6 == -2) {
+const int32_t C1 = 0xd555, C2 = 0x;
+
+assert(C1 * 6 == -2);
+assert(C2 * 6 == -2);
+
+clang_analyzer_eval(n == INT_MIN); //expected-warning{{FALSE}}
+clang_analyzer_eval(n == C1 - 1); //expected-warning{{FALSE}}
+clang_analyzer_eval(n == C1); //expected-warning{{FALSE}}
+  

[PATCH] D65182: [analyzer] WIP: Add fix-it hint support.

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

baloghadamsoftware added a comment.

Hmm, I was thinking on the same for some time but I wonder how many checkers 
could find the correct fixits? Maybe the removal fixits of double frees or 
double file closes, but I am afraid that for most of our path-sensitive checks 
there are no obvious fixits. Even `clang-tidy` cannot provide a fixit for most 
of its findings. However, generally I like the idea, even for the few checkers 
it can be applied to.


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

https://reviews.llvm.org/D65182



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

[PATCH] D65395: [clangd] Added a skeleton for a semantic highlighting feature to the vscode extension.

[Johan Vikström via Phabricator via cfe-commits]

jvikstrom created this revision.
jvikstrom added reviewers: hokein, ilya-biryukov.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay.
Herald added a project: clang.

This adds a SemanticHighlighting feature to the vscode extension that does not 
do anything. It adds the notification handler to the clangd client and 
registers the feature as an experimental feature sending the relevant data to 
the clangd server.
The notification handler does not currently do anything if/when clangd sends a 
semantic highlighting notification.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D65395

Files:
  clang-tools-extra/clangd/clients/clangd-vscode/.gitignore
  clang-tools-extra/clangd/clients/clangd-vscode/src/SemanticHighlighting.ts
  clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts

Index: clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts
===
--- clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts
+++ clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts
@@ -1,5 +1,6 @@
 import * as vscode from 'vscode';
 import * as vscodelc from 'vscode-languageclient';
+import { SemanticHighlighting } from './SemanticHighlighting';
 
 /**
  * Method to get workspace configuration option
@@ -12,9 +13,9 @@
 }
 
 namespace SwitchSourceHeaderRequest {
-export const type =
-new vscodelc.RequestType('textDocument/switchSourceHeader');
+export const type =
+new vscodelc.RequestType('textDocument/switchSourceHeader');
 }
 
 class FileStatus {
@@ -32,8 +33,8 @@
 const path = vscode.window.activeTextEditor.document.fileName;
 const status = this.statuses.get(path);
 if (!status) {
-  this.statusBarItem.hide();
-  return;
+this.statusBarItem.hide();
+return;
 }
 this.statusBarItem.text = `clangd: ` + status.state;
 this.statusBarItem.show();
@@ -73,25 +74,30 @@
 // However, VSCode does not have CUDA as a supported language yet, so we
 // cannot add a corresponding activationEvent for CUDA files and clangd will
 // *not* load itself automatically on '.cu' files.
-const cudaFilePattern: string = '**/*.{' +['cu'].join()+ '}';
+const cudaFilePattern: string = '**/*.{' + ['cu'].join() + '}';
 const clientOptions: vscodelc.LanguageClientOptions = {
 // Register the server for c-family and cuda files.
 documentSelector: [
 { scheme: 'file', language: 'c' },
 { scheme: 'file', language: 'cpp' },
-{ scheme: 'file', language: 'objective-c'},
-{ scheme: 'file', language: 'objective-cpp'},
+{ scheme: 'file', language: 'objective-c' },
+{ scheme: 'file', language: 'objective-cpp' },
 { scheme: 'file', pattern: cudaFilePattern },
 ],
 synchronize: !syncFileEvents ? undefined : {
-// FIXME: send sync file events when clangd provides implemenatations.
+// FIXME: send sync file events when clangd provides implemenatations.
 },
 initializationOptions: { clangdFileStatus: true },
 // Do not switch to output window when clangd returns output
 revealOutputChannelOn: vscodelc.RevealOutputChannelOn.Never
 };
 
-const clangdClient = new vscodelc.LanguageClient('Clang Language Server',serverOptions, clientOptions);
+const clangdClient = new vscodelc.LanguageClient('Clang Language Server', serverOptions, clientOptions);
+const semanticHighlightingFeature = new SemanticHighlighting.Feature();
+clangdClient.registerFeature(semanticHighlightingFeature);
+// The notification handler must be registered after the client is ready or the client will crash.
+clangdClient.onReady().then(() => clangdClient.onNotification(SemanticHighlighting.NotificationType, semanticHighlightingFeature.handleNotification));
+
 console.log('Clang Language Server is now active!');
 context.subscriptions.push(clangdClient.start());
 context.subscriptions.push(vscode.commands.registerCommand(
@@ -131,5 +137,5 @@
 // An empty place holder for the activate command, otherwise we'll get an
 // "command is not registered" error.
 context.subscriptions.push(vscode.commands.registerCommand(
-'clangd-vscode.activate', async () => {}));
+'clangd-vscode.activate', async () => { }));
 }
Index: clang-tools-extra/clangd/clients/clangd-vscode/src/SemanticHighlighting.ts
===
--- /dev/null
+++ clang-tools-extra/clangd/clients/clangd-vscode/src/SemanticHighlighting.ts
@@ -0,0 +1,36 @@
+import * as vscodelc from 'vscode-languageclient';
+import { DocumentSelector } from 'vscode-languageclient';
+
+export namespace SemanticHighlighting {
+interface HighlightingLineInformation {
+line: number,
+tokens: string,
+}

[PATCH] D64256: Teach some warnings to respect gsl::Pointer and gsl::Owner attributes

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr accepted this revision.
gribozavr added inline comments.



Comment at: clang/lib/Sema/SemaInit.cpp:7095
   auto *MTE = dyn_cast(L);
+  if (IsGslPtrInitWithGslTempOwner) {
+Diag(DiagLoc, diag::warn_dangling_lifetime_pointer) << DiagRange;

It is unclear to me why `if (IsGslPtrInitWithGslTempOwner)` is before `if 
(!MTE)`, seems like that exclusion should apply to our case, too.


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

https://reviews.llvm.org/D64256



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

[PATCH] D49074: [Analyzer] Basic support for multiplication and division in the constraint manager

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

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

Rebased.


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

https://reviews.llvm.org/D49074

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h
  lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
  lib/StaticAnalyzer/Core/RangedConstraintManager.cpp
  test/Analysis/multiplicative-folding.c

Index: test/Analysis/multiplicative-folding.c
===
--- test/Analysis/multiplicative-folding.c
+++ test/Analysis/multiplicative-folding.c
@@ -573,6 +573,1478 @@
   }
 }
 
+void signed_multiplication_lt_0(int32_t n) {
+  if (n * 2 < 3) {
+int32_t  U1 = 0x8001,
+L2 = 0xc000, U2 = 1,
+L3 = 0x4000;
+
+assert(INT_MIN * 2 < 3);
+assert(U1 * 2 < 3);
+assert((U1 + 1) * 2 >= 3);
+assert(L2 * 2 < 3);
+assert((L2 - 1) * 2 >= 3);
+assert(U2 * 2 < 3);
+assert((U2 + 1) * 2 >= 3);
+assert(L3 * 2 < 3);
+assert((L3 - 1) * 2 >= 3);
+assert(INT_MAX * 2 < 3);
+
+if (n < INT_MIN / 2) {
+  clang_analyzer_eval(n == INT_MIN); //expected-warning{{FALSE}}
+ //expected-warning@-1{{TRUE}}
+  clang_analyzer_eval(n <= U1); //expected-warning{{TRUE}}
+  clang_analyzer_eval(n > U1); //expected-warning{{FALSE}}
+} else if (n < INT_MAX / 2){
+  clang_analyzer_eval(n < L2); //expected-warning{{FALSE}}
+  clang_analyzer_eval(n >= L2); //expected-warning{{TRUE}}
+  clang_analyzer_eval(n <= U2); //expected-warning{{TRUE}}
+  clang_analyzer_eval(n > U2); //expected-warning{{FALSE}}
+} else {
+  clang_analyzer_eval(n < L3); //expected-warning{{FALSE}}
+  clang_analyzer_eval(n >= L3); //expected-warning{{TRUE}}
+  clang_analyzer_eval(n == INT_MAX); //expected-warning{{FALSE}}
+ //expected-warning@-1{{TRUE}}
+}
+  }
+}
+
+void signed_multiplication_lt_1(int32_t n) {
+  if (n * 2 < 4) {
+int32_t  U1 = 0x8001,
+L2 = 0xc000, U2 = 1,
+L3 = 0x4000;
+
+assert(INT_MIN * 2 < 4);
+assert(U1 * 2 < 4);
+assert((U1 + 1) * 2 >= 4);
+assert(L2 * 2 < 4);
+assert((L2 - 1) * 2 >= 4);
+assert(U2 * 2 < 4);
+assert((U2 + 1) * 2 >= 4);
+assert(L3 * 2 < 4);
+assert((L3 - 1) * 2 >= 4);
+assert(INT_MAX * 2 < 4);
+
+if (n < INT_MIN / 2) {
+  clang_analyzer_eval(n == INT_MIN); //expected-warning{{FALSE}}
+ //expected-warning@-1{{TRUE}}
+  clang_analyzer_eval(n <= U1); //expected-warning{{TRUE}}
+  clang_analyzer_eval(n > U1); //expected-warning{{FALSE}}
+} else if (n < INT_MAX / 2){
+  clang_analyzer_eval(n < L2); //expected-warning{{FALSE}}
+  clang_analyzer_eval(n >= L2); //expected-warning{{TRUE}}
+  clang_analyzer_eval(n <= U2); //expected-warning{{TRUE}}
+  clang_analyzer_eval(n > U2); //expected-warning{{FALSE}}
+} else {
+  clang_analyzer_eval(n < L3); //expected-warning{{FALSE}}
+  clang_analyzer_eval(n >= L3); //expected-warning{{TRUE}}
+  clang_analyzer_eval(n == INT_MAX); //expected-warning{{FALSE}}
+ //expected-warning@-1{{TRUE}}
+}
+  }
+}
+
+void signed_multiplication_lt_2(int32_t n) {
+  if (n * 2 < 5) {
+int32_t  U1 = 0x8002,
+L2 = 0xc000, U2 = 2,
+L3 = 0x4000;
+
+assert(INT_MIN * 2 < 5);
+assert(U1 * 2 < 5);
+assert((U1 + 1) * 2 >= 5);
+assert(L2 * 2 < 5);
+assert((L2 - 1) * 2 >= 5);
+assert(U2 * 2 < 5);
+assert((U2 + 1) * 2 >= 5);
+assert(L3 * 2 < 5);
+assert((L3 - 1) * 2 >= 5);
+assert(INT_MAX * 2 < 5);
+
+if (n < INT_MIN / 2) {
+  clang_analyzer_eval(n == INT_MIN); //expected-warning{{FALSE}}
+ //expected-warning@-1{{TRUE}}
+  clang_analyzer_eval(n <= U1); //expected-warning{{TRUE}}
+  clang_analyzer_eval(n > U1); //expected-warning{{FALSE}}
+} else if (n < INT_MAX / 2){
+  clang_analyzer_eval(n < L2); //expected-warning{{FALSE}}
+  clang_analyzer_eval(n >= L2); //expected-warning{{TRUE}}
+  clang_analyzer_eval(n <= U2); //expected-warning{{TRUE}}
+  clang_analyzer_eval(n > U2); //expected-warning{{FALSE}}
+} else {
+  clang_analyzer_eval(n < L3); //expected-warning{{FALSE}}
+  clang_analyzer_eval(n >= L3); //expected-warning{{TRUE}}
+  clang_analyzer_eval(n == INT_MAX); //expected-warning{{FALSE}}
+ //expected-warning@-1{{TRUE}}
+}
+  }
+}
+
+void signed_multiplication_lt_3(int32_t n) {
+  if (n * 3 < 4) {
+int32_t  U1 = 0xaaab,
+L2 = 0xd556, U2 = 1,
+L3 = 0x2aab, U3 = 0x5556;
+
+assert(INT_MIN * 3 < 4);
+assert(U1

[PATCH] D65361: [analyzer] Trust global initializers when analyzing main().

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

baloghadamsoftware added a comment.

Thank you for working on this! I agree, we should trust global initializers in 
`main()` in C. Can we maybe detect whether GNU global constructors are enabled 
or better: used?




Comment at: clang/lib/StaticAnalyzer/Core/RegionStore.cpp:395
 
   StoreRef getInitialStore(const LocationContext *InitLoc) override {
+bool IsMainAnalysis = false;

I think this function deserves now some doc comments since it is not a trivial 
getter anymore.



Comment at: clang/lib/StaticAnalyzer/Core/RegionStore.cpp:630
+(const RegionBindings::TreeTy *)((uintptr_t)store & ~(uintptr_t)1),
+RBFactory.getTreeFactory(), (bool)((uintptr_t)store & (uintptr_t)1));
   }

`(uintptr_t)1` look like a bit like some kind of magic number. Could we define 
it as a constant instead?



Comment at: clang/lib/StaticAnalyzer/Core/RegionStore.cpp:1693
 const VarDecl *VD = VR->getDecl();
 // Either the array or the array element has to be const.
+if (VD->getType().isConstQualified() ||

Please, update the comment as well.



Comment at: clang/lib/StaticAnalyzer/Core/RegionStore.cpp:1785
 unsigned Index = FD->getFieldIndex();
 // Either the record variable or the field has to be const qualified.
+if (RecordVarTy.isConstQualified() || Ty.isConstQualified() ||

This one as well.



Comment at: clang/lib/StaticAnalyzer/Core/RegionStore.cpp:2005
 
+if (B.isMainAnalysis())
+  if (const Expr *Init = VD->getAnyInitializer())

Please add a comment to the block as well.



Comment at: clang/test/Analysis/main.c:21
+  return 0;
+}

Please add a negative test case (i.e. function that is not `main`) as well.


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

https://reviews.llvm.org/D65361



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

[PATCH] D65337: [clangd] Disallow extraction of expression-statements.

[Shaurya Gupta via Phabricator via cfe-commits]

SureYeaah added a comment.

What was the bug in getCallExpr() ?




Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp:391
+return false;
+  Outer->dumpColor();
+  // Blacklist the most common places where an expr can appear but be unused.

Remove dump.



Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp:445
+
+  // FIXME: ban extracting the RHS of an assignment: `a = [[foo()]]`
   return true;

Check if parent is an assignment binaryoperator or a vardecl?



Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp:461
+  // For function and member function DeclRefs, extract the whole call.
+  if (const DeclRefExpr *DeclRef = dyn_cast_or_null(SelectedExpr))
 TargetNode = getCallExpr(N);

Can we combine both these IFs and remove the unused assignment?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65337



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

[PATCH] D65337: [clangd] Disallow extraction of expression-statements.

[Shaurya Gupta via Phabricator via cfe-commits]

SureYeaah added inline comments.



Comment at: clang-tools-extra/clangd/unittests/TweakTests.cpp:329
   while(a < [[1]])
-[[a++]];
+a++;
   // do while

Change to a=[[1]];?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65337



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

[PATCH] D65349: [analyzer] Be more careful with destructors of non-regions.

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

baloghadamsoftware added a comment.

Yes, it crashes! :-) I tried it because I did not believe it, but it does. Is 
there any real-world use-case for casting concrete integers to class instances? 
How did you find this crashing case?




Comment at: clang/lib/StaticAnalyzer/Core/ExprEngine.cpp:1055
+  Loc ThisLoc = State->getSVal(ThisVal).castAs();
+  SVal FieldLoc = State->getLValue(Member, ThisLoc);
 

Why do we rename it from `FieldVal` to `FieldLoc`? Are we sure that this is a 
`Loc`? If so, then we should apply `castAs()` and change the type to `Loc` 
as well. (Or `auto` since `castAs()` is enough according to the LLVM 
coding standards.) If we are not sure, then I think it is better to leave it 
`FieldVal`.



Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp:627
 
+  if (!Dest) {
+CallOpts.IsCtorOrDtorWithImproperlyModeledTargetRegion = true;

Could you please add some lines of comments to this code block?


Repository:
  rC Clang

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

https://reviews.llvm.org/D65349



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

[PATCH] D64488: [Driver] Support -fsanitize=function on Solaris/x86

[Rainer Orth via Phabricator via cfe-commits]

ro added a comment.

Ping^3?  Three weeks and counting...


Repository:
  rC Clang

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

https://reviews.llvm.org/D64488



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

[PATCH] D64793: [Driver] Properly use values-X[ca].o, values-xpg[46].o on Solaris

[Rainer Orth via Phabricator via cfe-commits]

ro added a comment.

Ping^2?  It's been two weeks now.  Who can decide if the LangStandards code can 
be used from lib/Frontend as is or needs to be moved
elsewhere (lib/Basic?) for use by driver code?


Repository:
  rC Clang

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

https://reviews.llvm.org/D64793



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

[PATCH] D65341: [OpenMP] Add support for close map modifier in Clang

[Alexey Bataev via Phabricator via cfe-commits]

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

LG


Repository:
  rC Clang

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

https://reviews.llvm.org/D65341



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

[PATCH] D63954: Add lifetime categories attributes

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr added inline comments.



Comment at: cfe/trunk/include/clang/Basic/AttrDocs.td:4252
+
+The argument ``T`` is optional and currently ignored.
+This attribute may be used by analysis tools and has no effect on code

"and is currently ignored"

even better:

"and is ignored"

It does not matter for the user that we might change it in future. We might 
change *anything* in future, and yet we don't hedge everywhere.



Comment at: cfe/trunk/include/clang/Basic/AttrDocs.td:4278
+
+The argument ``T`` is optional and currently ignored.
+This attribute may be used by analysis tools and has no effect on code

ditto



Comment at: cfe/trunk/test/SemaCXX/attr-gsl-owner-pointer.cpp:34
+class [[gsl::Owner(void)]] OwnerVoidDerefType{};
+// expected-error@-1 {{'void' is an invalid argument to attribute 'Owner'}}
+class [[gsl::Pointer(void)]] PointerVoidDerefType{};

Technically correct, but not very helpful for the user.

It would be better if the message was tailored for the attribute, for example,

owner types can't own an object of type 'void'


Repository:
  rL LLVM

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

https://reviews.llvm.org/D63954



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

[PATCH] D65120: More warnings regarding gsl::Pointer and gsl::Owner attributes

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr added inline comments.



Comment at: clang/lib/Sema/SemaInit.cpp:6563
 
+static bool shouldTrackImplicitObjectArg(const CXXMethodDecl *Callee) {
+  if (auto *Conv = dyn_cast_or_null(Callee))

This looks like an ad-hoc rule. Is there a way to express it with attributes 
instead?



Comment at: clang/lib/Sema/SemaInit.cpp:6572
+return false;
+  return llvm::StringSwitch(Callee->getName())
+  .Cases("begin", "rbegin", "cbegin", "crbegin", true)

Should we also check that `Callee->getParent` is an owner?

Otherwise the check would complain about `begin()` on, for example, a 
`std::string_view`.



Comment at: clang/test/Sema/warn-lifetime-analysis-nocfg.cpp:170
+  return std::unique_ptr().get(); // expected-warning {{returning address 
of local temporary object}}
+}

Please add a newline.


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

https://reviews.llvm.org/D65120



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

[PATCH] D65341: [OpenMP] Add support for close map modifier in Clang

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld added subscribers: saghir, Hahnfeld.
Hahnfeld requested changes to this revision.
Hahnfeld added a comment.
This revision now requires changes to proceed.

There's already D55892  with a better set of 
tests, including `target enter data` / `target exit data`.


Repository:
  rC Clang

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

https://reviews.llvm.org/D65341



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

[PATCH] D65234: [CodeGen]: don't treat structures returned in registers as memory inputs

[Alexander Potapenko via Phabricator via cfe-commits]

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



Comment at: clang/test/CodeGen/PR42672.c:50
+  struct {
+long long int v1, v2, v3, v4;
+  } str;

efriedma wrote:
> glider wrote:
> > The acceptable size actually depends on the target platform. Not sure we 
> > can test for all of them, and we'll probably need to restrict this test to 
> > e.g. x86
> The interesting case for a 32-byte struct would actually be something like 
> `"=x"(str)`... which currently passes the clang frontend, since 32 is a legal 
> size for that constraint (although it eventually fails in the backend).
Changing "=r" to "=X" indeed makes this particular test pass (there's nothing 
to fail in the backend, as we don't actually generate instructions that write 
to memory)
I'm however unsure adding a test for "=X" makes any difference, as our patch is 
irrelevant to this constraint.



Comment at: clang/test/CodeGen/PR42672.c:40
+unsigned short first;
+unsigned char second;
+  } str;

efriedma wrote:
> This isn't a three-byte struct; it's a four-byte struct where one of the 
> bytes is only used for padding. 
You're right. Making this structure packed yields an error, which conforms to 
GCC behavior.
I've added a test for that as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65234



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

[PATCH] D65234: [CodeGen]: don't treat structures returned in registers as memory inputs

[Alexander Potapenko via Phabricator via cfe-commits]

glider updated this revision to Diff 212164.
glider added a comment.

Addressed Eli's comments, added a test for a packed struct


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65234

Files:
  clang/lib/CodeGen/CGStmt.cpp
  clang/test/CodeGen/asm-attrs.c
  clang/test/CodeGen/x86_64-PR42672.c

Index: clang/test/CodeGen/x86_64-PR42672.c
===
--- /dev/null
+++ clang/test/CodeGen/x86_64-PR42672.c
@@ -0,0 +1,79 @@
+// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -DSTRUCT -emit-llvm %s -o - | FileCheck %s --check-prefix=CHECK-STRUCT
+// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -USTRUCT -emit-llvm %s -o - | FileCheck %s --check-prefix=CHECK-NOSTRUCT
+// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -DIMPOSSIBLE_ODD -emit-llvm %s -o - 2> %t || true
+// RUN: grep "impossible constraint in asm" %t
+// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -DIMPOSSIBLE_BIG -emit-llvm %s -o - 2> %t || true
+// RUN: grep "impossible constraint in asm" %t
+
+// Make sure Clang doesn't treat |lockval| as asm input.
+void _raw_spin_lock(void) {
+#if STRUCT
+  struct {
+unsigned short owner, next;
+  } lockval;
+  lockval.owner = 1;
+  lockval.next = 2;
+#else
+  int lockval;
+  lockval = 3;
+#endif
+  asm("nop"
+  : "=r"(lockval));
+}
+// CHECK-LABEL: _raw_spin_lock
+// CHECK-LABEL: entry:
+
+// CHECK-STRUCT:  %lockval = alloca %struct.anon, align 2
+// CHECK-STRUCT:  store i16 1
+// CHECK-STRUCT:  store i16 2
+// CHECK-STRUCT: [[RES:%[0-9]+]] = call i32 asm "nop", "=r,~{dirflag},~{fpsr},~{flags}"()
+// CHECK-STRUCT: [[CAST:%[0-9]+]] = bitcast %struct.anon* %lockval to i32*
+// CHECK-STRUCT: store i32 [[RES]], i32* [[CAST]], align 2
+
+// CHECK-NOSTRUCT: %lockval = alloca i32, align 4
+// CHECK-NOSTRUCT: store i32 3
+// CHECK-NOSTRUCT:  [[RES:%[0-9]+]] = call i32 asm "nop", "=r,~{dirflag},~{fpsr},~{flags}"()
+// CHECK-NOSTRUCT: store i32 [[RES]], i32* %lockval, align 4
+
+// Check Clang correctly handles a structure with padding.
+void unusual_struct(void) {
+  struct {
+unsigned short first;
+unsigned char second;
+  } str;
+  asm("nop"
+  : "=r"(str));
+}
+
+// Check Clang reports an error if attempting to return a structure for which
+// no direct conversion to a register is possible.
+#ifdef IMPOSSIBLE_ODD
+void odd_struct(void) {
+  struct __attribute__((__packed__)) {
+unsigned short first;
+unsigned char second;
+  } str;
+  asm("nop"
+  : "=r"(str));
+}
+#endif
+
+// Check Clang reports an error if attempting to return a big structure via a register.
+#ifdef IMPOSSIBLE_BIG
+void big_struct(void) {
+  struct {
+long long int v1, v2, v3, v4;
+  } str;
+  asm("nop"
+  : "=r"(str));
+}
+#endif
+
+// Check Clang reports an error if attempting to return a big structure via a register.
+void big_struct(void) {
+  struct {
+long long int v1, v2, v3, v4;
+  } str;
+  asm("nop"
+  : "=X"(str));
+}
Index: clang/test/CodeGen/asm-attrs.c
===
--- clang/test/CodeGen/asm-attrs.c
+++ clang/test/CodeGen/asm-attrs.c
@@ -8,7 +8,7 @@
 // CHECK: call i32 asm "foo5", {{.*}} [[READONLY]]
 // CHECK: call i32 asm "foo6", {{.*}} [[NOATTRS]]
 // CHECK: call void asm sideeffect "foo7", {{.*}} [[NOATTRS]]
-// CHECK: call void asm "foo8", {{.*}} [[NOATTRS]]
+// CHECK: call i32 asm "foo8", {{.*}} [[READNONE]]
 
 // CHECK: attributes [[READNONE]] = { nounwind readnone }
 // CHECK: attributes [[NOATTRS]] = { nounwind }
Index: clang/lib/CodeGen/CGStmt.cpp
===
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -1986,6 +1986,7 @@
   std::vector ResultTruncRegTypes;
   std::vector ArgTypes;
   std::vector Args;
+  llvm::BitVector ResultTypeRequiresCast;
 
   // Keep track of inout constraints.
   std::string InOutConstraints;
@@ -2024,13 +2025,23 @@
 
 // If this is a register output, then make the inline asm return it
 // by-value.  If this is a memory result, return the value by-reference.
-if (!Info.allowsMemory() && hasScalarEvaluationKind(OutExpr->getType())) {
+bool isScalarizableAggregate =
+hasAggregateEvaluationKind(OutExpr->getType());
+if (!Info.allowsMemory() && (hasScalarEvaluationKind(OutExpr->getType()) ||
+ isScalarizableAggregate)) {
   Constraints += "=" + OutputConstraint;
   ResultRegQualTys.push_back(OutExpr->getType());
   ResultRegDests.push_back(Dest);
-  ResultRegTypes.push_back(ConvertTypeForMem(OutExpr->getType()));
-  ResultTruncRegTypes.push_back(ResultRegTypes.back());
-
+  ResultTruncRegTypes.push_back(ConvertTypeForMem(OutExpr->getType()));
+  if (Info.allowsRegister() && isScalarizableAggregate) {
+ResultTypeRequiresCast.push_back(true);
+unsigned Size = getContext().getTy

[PATCH] D65341: [OpenMP] Add support for close map modifier in Clang

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D65341#1604411 , @Hahnfeld wrote:

> There's already D55892  with a better set of 
> tests, including `target enter data` / `target exit data`.


Better to merge those two patches into one.


Repository:
  rC Clang

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

https://reviews.llvm.org/D65341



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

[PATCH] D62574: Initial draft of target-configurable address spaces.

[Bevin Hansson via Phabricator via cfe-commits]

ebevhan marked 4 inline comments as done.
ebevhan added a comment.

Sorry for the very late response to this!




Comment at: include/clang/AST/ASTContext.h:2598
+  /// Returns true if address space A overlaps with B.
+  bool isAddressSpaceOverlapping(LangAS A, LangAS B) const {
+// A overlaps with B if either is a superset of the other.

Anastasia wrote:
> ebevhan wrote:
> > Anastasia wrote:
> > > Is there any advantage of keeping superset&subset concept? Amd if yes, 
> > > how do we position the new functionality with explicit cast?
> > > 
> > > I think I am missing a bit conceptual view... because I think we 
> > > originally discussed to switch to implicit/explicit conversion model. 
> > > Perhaps there is no reason to do it but I would just like to understand 
> > > why? 
> > Yes, I know the original discussion was regarding the implicit/explicit 
> > model, but I came to realize during the implementation that all that was 
> > needed to get the superspace model to work generically with the current 
> > behavior was an override on the explicit conversion.
> > 
> > The implicit/explicit model also has the drawback that it's a bit too 
> > expressive. You can express semantics that just don't really make sense, 
> > like permitting implicit conversion but not explicit conversion. The 
> > superspace model doesn't let you do that, and the additions I've made here 
> > still don't: If implicit conversion is allowed, explicit conversion must 
> > also be allowed. It just becomes possible to allow explicit conversion for 
> > ASes that don't overlap.
> > 
> > Since the superspace model is what OpenCL and Embedded-C use in their 
> > specification, it's probably better to use that anyway.
> > The implicit/explicit model also has the drawback that it's a bit too 
> > expressive. You can express semantics that just don't really make sense, 
> > like permitting implicit conversion but not explicit conversion. The 
> > superspace model doesn't let you do that, and the additions I've made here 
> > still don't: If implicit conversion is allowed, explicit conversion must 
> > also be allowed. It just becomes possible to allow explicit conversion for 
> > ASes that don't overlap.
> 
> Ok, I think we could define the new model something like - explicit 
> conversions are automatically allowed for all implicit conversions... targets 
> won't have to specify those but only extra comversions that are not allowed 
> implicitly. 
> 
> Just to understand in the current patch when are we supposed to use 
> `isAddressSpaceOverlapping` and when `isExplicitAddrSpaceConversionLegal`. 
> Can't we just always use `isExplicitAddrSpaceConversionLegal`?
> 
> > 
> > Since the superspace model is what OpenCL and Embedded-C use in their 
> > specification, it's probably better to use that anyway.
> 
> I agree the advantage of following spec is really huge. However, Clang is 
> already broken for Emdedded C isn't it? Because it allows any explicit 
> conversions?
> 
> As for OpenCL it might be reasonable to provide new documentation if needed 
> as soon as the new rules don't invalidate all behavior.
> 
> 
> Ok, I think we could define the new model something like - explicit 
> conversions are automatically allowed for all implicit conversions... targets 
> won't have to specify those but only extra comversions that are not allowed 
> implicitly.

Yes, this is how it's defined. Converting explicitly between two ASes where 
either one is a superset of the other is always legal.

> Just to understand in the current patch when are we supposed to use 
> isAddressSpaceOverlapping and when isExplicitAddrSpaceConversionLegal. Can't 
> we just always use isExplicitAddrSpaceConversionLegal?

I guess the distinction between `isAddressSpaceOverlapping` and 
`isExplicitAddrSpaceConversionLegal` is pretty subtle. You would want the 
former when you need to know if **implicit conversion A->B or B->A** is 
permitted, and the latter when you need to know if **explicit conversion A->B** 
is permitted.

Though in most cases, I think the latter is probably the most common.

> I agree the advantage of following spec is really huge. However, Clang is 
> already broken for Emdedded C isn't it? Because it allows any explicit 
> conversions?

No, the current behavior of permitting all explicit conversions is according to 
spec: "A non-null pointer into an address space A can be cast to a pointer into 
another address space B, but such a cast is undefined if the source pointer 
does not point to a location in B." The addition of 
`isExplicitAddrSpaceConversionLegal` lets targets override this behavior and 
make such casts non-legal.



Comment at: lib/Sema/SemaCast.cpp:2224
+// the cast is explicitly legal as well.
+if (CStyle ? !Self.Context.isExplicitAddrSpaceConversionLegal(SrcQ, DestQ)
+   : !Self.Context.isAddressSpaceSupersetOf(DestQ, SrcQ)) {

Anastasia wrote:
> ebev

[PATCH] D64448: gsl::Owner/gsl::Pointer: Add implicit annotations for some std types

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr added inline comments.



Comment at: clang/include/clang/Sema/Sema.h:6097
+
+  /// Add [[gsl::Owner]] and [[gsl::Pointer]] attributes for std:: types.
+  void addDefaultGslPointerAttribute(TypedefNameDecl *TD);

It seems like this function does not add gsl::Owner.



Comment at: clang/lib/Sema/SemaTemplate.cpp:1689
   AddPushedVisibilityAttribute(NewClass);
+  addDefaultGslOwnerPointerAttribute(NewClass);
 

It shouldn't be necessary to perform inference here, instead, the attributes 
should be instantiated, see `instantiateDependentAlignedAttr` in 
SemaTemplateInstantiateDecl.cpp for an example.



Comment at: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:702
   SemaRef.InstantiateAttrs(TemplateArgs, D, Typedef);
+  SemaRef.addDefaultGslPointerAttribute(Typedef);
 

Ditto, should not be necessary to perform inference here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64448



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

r367229 - [OpenCL] Rename builtin definition classes

[Sven van Haastregt via cfe-commits]

Author: svenvh
Date: Mon Jul 29 07:55:29 2019
New Revision: 367229

URL: http://llvm.org/viewvc/llvm-project?rev=367229&view=rev
Log:
[OpenCL] Rename builtin definition classes

Factor out some of the renames from D63434 and D63442, and generate
half type convert_ builtins.

Patch by Pierre Gondois and Sven van Haastregt.

Modified:
cfe/trunk/lib/Sema/OpenCLBuiltins.td

Modified: cfe/trunk/lib/Sema/OpenCLBuiltins.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/OpenCLBuiltins.td?rev=367229&r1=367228&r2=367229&view=diff
==
--- cfe/trunk/lib/Sema/OpenCLBuiltins.td (original)
+++ cfe/trunk/lib/Sema/OpenCLBuiltins.td Mon Jul 29 07:55:29 2019
@@ -30,14 +30,14 @@ def CL20: Version<200>;
 // Address spaces
 // Pointer types need to be assigned an address space.
 class AddressSpace {
-  string AddrSpace = _AS;
+  string Name = _AS;
 }
-def default_as: AddressSpace<"clang::LangAS::Default">;
-def private_as: AddressSpace<"clang::LangAS::opencl_private">;
-def global_as : AddressSpace<"clang::LangAS::opencl_global">;
-def constant_as   : AddressSpace<"clang::LangAS::opencl_constant">;
-def local_as  : AddressSpace<"clang::LangAS::opencl_local">;
-def generic_as: AddressSpace<"clang::LangAS::opencl_generic">;
+def DefaultAS: AddressSpace<"clang::LangAS::Default">;
+def PrivateAS: AddressSpace<"clang::LangAS::opencl_private">;
+def GlobalAS : AddressSpace<"clang::LangAS::opencl_global">;
+def ConstantAS   : AddressSpace<"clang::LangAS::opencl_constant">;
+def LocalAS  : AddressSpace<"clang::LangAS::opencl_local">;
+def GenericAS: AddressSpace<"clang::LangAS::opencl_generic">;
 
 
 // Qualified Type. Allow to retrieve one ASTContext QualType.
@@ -79,10 +79,10 @@ class VectorType :
+class PointerType :
   Type<_Ty.Name, _Ty.QTName> {
   bit IsPointer = 1;
-  string AddrSpace = _AS.AddrSpace;
+  string AddrSpace = _AS.Name;
 }
 
 // OpenCL image types (e.g. image2d_t, ...)
@@ -156,37 +156,37 @@ multiclass Bif3
 // Definitions of OpenCL C types
 
//===--===//
 // OpenCL v1.2 s6.1.1: Built-in Scalar Data Types
-def bool_t  : Type<"bool", QualType<"BoolTy">>;
-def char_t  : Type<"char", QualType<"CharTy">>;
-def uchar_t : Type<"uchar", QualType<"UnsignedCharTy">>;
-def short_t : Type<"short", QualType<"ShortTy">>;
-def ushort_t: Type<"ushort", QualType<"UnsignedShortTy">>;
-def int_t   : Type<"int", QualType<"IntTy">>;
-def uint_t  : Type<"uint", QualType<"UnsignedIntTy">>;
-def long_t  : Type<"long", QualType<"LongTy">>;
-def ulong_t : Type<"ulong", QualType<"UnsignedLongTy">>;
-def float_t : Type<"float", QualType<"FloatTy">>;
-def double_t: Type<"double", QualType<"DoubleTy">>;
-def half_t  : Type<"half", QualType<"HalfTy">>;
-def size_t  : Type<"size_t",  QualType<"getSizeType()">>;
-def ptrdiff_t   : Type<"ptrdiff_t", QualType<"getPointerDiffType()">>;
-def intptr_t: Type<"intptr_t", QualType<"getIntPtrType()">>;
-def uintptr_t   : Type<"uintptr_t", QualType<"getUIntPtrType()">>;
-def void_t  : Type<"void", QualType<"VoidTy">>;
+def Bool  : Type<"bool",  QualType<"BoolTy">>;
+def Char  : Type<"char",  QualType<"CharTy">>;
+def UChar : Type<"uchar", QualType<"UnsignedCharTy">>;
+def Short : Type<"short", QualType<"ShortTy">>;
+def UShort: Type<"ushort",QualType<"UnsignedShortTy">>;
+def Int   : Type<"int",   QualType<"IntTy">>;
+def UInt  : Type<"uint",  QualType<"UnsignedIntTy">>;
+def Long  : Type<"long",  QualType<"LongTy">>;
+def ULong : Type<"ulong", QualType<"UnsignedLongTy">>;
+def Float : Type<"float", QualType<"FloatTy">>;
+def Double: Type<"double",QualType<"DoubleTy">>;
+def Half  : Type<"half",  QualType<"HalfTy">>;
+def Size  : Type<"size_t",QualType<"getSizeType()">>;
+def PtrDiff   : Type<"ptrdiff_t", QualType<"getPointerDiffType()">>;
+def IntPtr: Type<"intptr_t",  QualType<"getIntPtrType()">>;
+def UIntPtr   : Type<"uintPtr_t", QualType<"getUIntPtrType()">>;
+def Void  : Type<"void_t",QualType<"VoidTy">>;
 
 // OpenCL v1.2 s6.1.2: Built-in Vector Data Types
 foreach v = [2, 3, 4, 8, 16] in {
-  def char#v#_t: VectorType;
-  def uchar#v#_t   : VectorType;
-  def short#v#_t   : VectorType;
-  def ushort#v#_t  : VectorType;
-  def "int"#v#_t   : VectorType;
-  def uint#v#_t: VectorType;
-  def long#v#_t: VectorType;
-  def ulong#v#_t   : VectorType;
-  def float#v#_t   : VectorType;
-  def double#v#_t  : VectorType;
-  def half#v#_t: VectorType;
+  def char#v#_t: VectorType;
+  def uchar#v#_t   : VectorType;
+  def short#v#_t   : VectorType;
+  def ushort#v#_t  : VectorType;
+  def "int"#v#_t   : VectorType;
+  def uint#v#_t: VectorType;
+  def long#v#_t: 

[PATCH] D64849: Add platform guards to PPC vector intrinsics headers.

[Jinsong Ji via Phabricator via cfe-commits]

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

LGTM.


Repository:
  rC Clang

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

https://reviews.llvm.org/D64849



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

[PATCH] D65341: [OpenMP] Add support for close map modifier in Clang

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

gtbercea added a comment.

In D65341#1604440 , @ABataev wrote:

> In D65341#1604411 , @Hahnfeld wrote:
>
> > There's already D55892  with a better set 
> > of tests, including `target enter data` / `target exit data`.
>
>
> Better to merge those two patches into one.


How would you like me to proceed? It looks like the other patch has been 
sitting there approved for many many months.


Repository:
  rC Clang

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

https://reviews.llvm.org/D65341



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

[PATCH] D61027: Fix crash on switch conditions of non-integer types in templates

[Elizabeth Andrews via Phabricator via cfe-commits]

eandrews added a comment.

ping


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

https://reviews.llvm.org/D61027



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

[PATCH] D65341: [OpenMP] Add support for close map modifier in Clang

[Kelvin Li via Phabricator via cfe-commits]

kkwli0 added a comment.

In D65341#1604561 , @gtbercea wrote:

> In D65341#1604440 , @ABataev wrote:
>
> > In D65341#1604411 , @Hahnfeld 
> > wrote:
> >
> > > There's already D55892  with a better 
> > > set of tests, including `target enter data` / `target exit data`.
> >
> >
> > Better to merge those two patches into one.
>
>
> How would you like me to proceed? It looks like the other patch has been 
> sitting there approved for many many months.


Yes, the patch has been waiting for the runtime implementation first and then 
commit.


Repository:
  rC Clang

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

https://reviews.llvm.org/D65341



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

[PATCH] D65341: [OpenMP] Add support for close map modifier in Clang

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

gtbercea added a comment.

In D65341#1604562 , @kkwli0 wrote:

> In D65341#1604561 , @gtbercea wrote:
>
> > In D65341#1604440 , @ABataev wrote:
> >
> > > In D65341#1604411 , @Hahnfeld 
> > > wrote:
> > >
> > > > There's already D55892  with a better 
> > > > set of tests, including `target enter data` / `target exit data`.
> > >
> > >
> > > Better to merge those two patches into one.
> >
> >
> > How would you like me to proceed? It looks like the other patch has been 
> > sitting there approved for many many months.
>
>
> Yes, the patch has been waiting for the runtime implementation first and then 
> commit.


The runtime implementation is here: D65340 


Repository:
  rC Clang

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

https://reviews.llvm.org/D65341



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

[PATCH] D65341: [OpenMP] Add support for close map modifier in Clang

[Kelvin Li via Phabricator via cfe-commits]

kkwli0 added a comment.

In D65341#1604562 , @kkwli0 wrote:

> In D65341#1604561 , @gtbercea wrote:
>
> > In D65341#1604440 , @ABataev wrote:
> >
> > > In D65341#1604411 , @Hahnfeld 
> > > wrote:
> > >
> > > > There's already D55892  with a better 
> > > > set of tests, including `target enter data` / `target exit data`.
> > >
> > >
> > > Better to merge those two patches into one.
> >
> >
> > How would you like me to proceed? It looks like the other patch has been 
> > sitting there approved for many many months.
>
>
> Yes, the patch has been waiting for the runtime implementation first and then 
> commit.


I am not sure if it is easier to rebase D55892 
 and merge this patch to D55892 
.  I am okay to go either way.


Repository:
  rC Clang

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

https://reviews.llvm.org/D65341



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

[PATCH] D65203: [ASTImporter] Do not import FunctionTemplateDecl in record twice.

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

@a_sidorin This is a polite Ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65203



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

[PATCH] D64696: Adds a warning when an inline Doxygen comment has no argument

[Mark de Wever via Phabricator via cfe-commits]

Mordante added a comment.

Thanks for the review.

I don't have commit access. So yes please commit the patch for me.


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

https://reviews.llvm.org/D64696



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

[PATCH] D65130: [clang][OpenMP] Add clang-offload-wrapper tool

[Sergey Dmitriev via Phabricator via cfe-commits]

sdmitriev added a comment.

Any feedback?


Repository:
  rC Clang

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

https://reviews.llvm.org/D65130



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

[PATCH] D65403: [COFF, ARM64] Reorder handling of aarch64 MSVC builtins

[David Major via Phabricator via cfe-commits]

dmajor created this revision.
dmajor added reviewers: mgrang, efriedma, hans.
Herald added subscribers: kristina, jfb, kristof.beyls, javed.absar.
Herald added a project: clang.

In `CodeGenFunction::EmitAArch64BuiltinExpr()`, bulk move of all of the aarch64 
MSVC-builtin cases to an earlier point in the function (the `// Handle 
non-overloaded intrinsics first.` switch block) in order to avoid an 
unreachable in `GetNeonType()`. The NEON type-overloading logic is not 
appropriate for the Windows builtins.

Fixes https://llvm.org/pr42775


Repository:
  rC Clang

https://reviews.llvm.org/D65403

Files:
  lib/CodeGen/CGBuiltin.cpp
  test/CodeGen/arm64-microsoft-intrinsics.c

Index: test/CodeGen/arm64-microsoft-intrinsics.c
===
--- test/CodeGen/arm64-microsoft-intrinsics.c
+++ test/CodeGen/arm64-microsoft-intrinsics.c
@@ -8,6 +8,10 @@
   return _InterlockedAdd(Addend, Value);
 }
 
+long test_InterlockedAdd_constant(long volatile *Addend) {
+  return _InterlockedAdd(Addend, -1);
+}
+
 // CHECK-LABEL: define {{.*}} i32 @test_InterlockedAdd(i32* %Addend, i32 %Value) {{.*}} {
 // CHECK-MSVC: %[[OLDVAL:[0-9]+]] = atomicrmw add i32* %1, i32 %2 seq_cst
 // CHECK-MSVC: %[[NEWVAL:[0-9]+]] = add i32 %[[OLDVAL:[0-9]+]], %2
Index: lib/CodeGen/CGBuiltin.cpp
===
--- lib/CodeGen/CGBuiltin.cpp
+++ lib/CodeGen/CGBuiltin.cpp
@@ -8011,6 +8011,151 @@
 return Builder.CreateExtractElement(Ops[0], EmitScalarExpr(E->getArg(1)),
 "vgetq_lane");
   }
+  case AArch64::BI_BitScanForward:
+  case AArch64::BI_BitScanForward64:
+return EmitMSVCBuiltinExpr(MSVCIntrin::_BitScanForward, E);
+  case AArch64::BI_BitScanReverse:
+  case AArch64::BI_BitScanReverse64:
+return EmitMSVCBuiltinExpr(MSVCIntrin::_BitScanReverse, E);
+  case AArch64::BI_InterlockedAnd64:
+return EmitMSVCBuiltinExpr(MSVCIntrin::_InterlockedAnd, E);
+  case AArch64::BI_InterlockedExchange64:
+return EmitMSVCBuiltinExpr(MSVCIntrin::_InterlockedExchange, E);
+  case AArch64::BI_InterlockedExchangeAdd64:
+return EmitMSVCBuiltinExpr(MSVCIntrin::_InterlockedExchangeAdd, E);
+  case AArch64::BI_InterlockedExchangeSub64:
+return EmitMSVCBuiltinExpr(MSVCIntrin::_InterlockedExchangeSub, E);
+  case AArch64::BI_InterlockedOr64:
+return EmitMSVCBuiltinExpr(MSVCIntrin::_InterlockedOr, E);
+  case AArch64::BI_InterlockedXor64:
+return EmitMSVCBuiltinExpr(MSVCIntrin::_InterlockedXor, E);
+  case AArch64::BI_InterlockedDecrement64:
+return EmitMSVCBuiltinExpr(MSVCIntrin::_InterlockedDecrement, E);
+  case AArch64::BI_InterlockedIncrement64:
+return EmitMSVCBuiltinExpr(MSVCIntrin::_InterlockedIncrement, E);
+  case AArch64::BI_InterlockedExchangeAdd8_acq:
+  case AArch64::BI_InterlockedExchangeAdd16_acq:
+  case AArch64::BI_InterlockedExchangeAdd_acq:
+  case AArch64::BI_InterlockedExchangeAdd64_acq:
+return EmitMSVCBuiltinExpr(MSVCIntrin::_InterlockedExchangeAdd_acq, E);
+  case AArch64::BI_InterlockedExchangeAdd8_rel:
+  case AArch64::BI_InterlockedExchangeAdd16_rel:
+  case AArch64::BI_InterlockedExchangeAdd_rel:
+  case AArch64::BI_InterlockedExchangeAdd64_rel:
+return EmitMSVCBuiltinExpr(MSVCIntrin::_InterlockedExchangeAdd_rel, E);
+  case AArch64::BI_InterlockedExchangeAdd8_nf:
+  case AArch64::BI_InterlockedExchangeAdd16_nf:
+  case AArch64::BI_InterlockedExchangeAdd_nf:
+  case AArch64::BI_InterlockedExchangeAdd64_nf:
+return EmitMSVCBuiltinExpr(MSVCIntrin::_InterlockedExchangeAdd_nf, E);
+  case AArch64::BI_InterlockedExchange8_acq:
+  case AArch64::BI_InterlockedExchange16_acq:
+  case AArch64::BI_InterlockedExchange_acq:
+  case AArch64::BI_InterlockedExchange64_acq:
+return EmitMSVCBuiltinExpr(MSVCIntrin::_InterlockedExchange_acq, E);
+  case AArch64::BI_InterlockedExchange8_rel:
+  case AArch64::BI_InterlockedExchange16_rel:
+  case AArch64::BI_InterlockedExchange_rel:
+  case AArch64::BI_InterlockedExchange64_rel:
+return EmitMSVCBuiltinExpr(MSVCIntrin::_InterlockedExchange_rel, E);
+  case AArch64::BI_InterlockedExchange8_nf:
+  case AArch64::BI_InterlockedExchange16_nf:
+  case AArch64::BI_InterlockedExchange_nf:
+  case AArch64::BI_InterlockedExchange64_nf:
+return EmitMSVCBuiltinExpr(MSVCIntrin::_InterlockedExchange_nf, E);
+  case AArch64::BI_InterlockedCompareExchange8_acq:
+  case AArch64::BI_InterlockedCompareExchange16_acq:
+  case AArch64::BI_InterlockedCompareExchange_acq:
+  case AArch64::BI_InterlockedCompareExchange64_acq:
+return EmitMSVCBuiltinExpr(MSVCIntrin::_InterlockedCompareExchange_acq, E);
+  case AArch64::BI_InterlockedCompareExchange8_rel:
+  case AArch64::BI_InterlockedCompareExchange16_rel:
+  case AArch64::BI_InterlockedCompareExchange_rel:
+  case AArch64::BI_InterlockedCompareExchange64_rel:
+return EmitMSVCBuiltinExpr(MSVCIntrin::_InterlockedCompareExchange_rel, E);
+  case AArch64::BI_InterlockedCompar

[PATCH] D65404: [AArch64] Disable __ARM_FEATURE_SVE without ACLE.

[Sander de Smalen via Phabricator via cfe-commits]

sdesmalen created this revision.
sdesmalen added reviewers: c-rhodes, chill, rengolin, rovka.
Herald added subscribers: kristof.beyls, tschuett, javed.absar.

The Arm C Language Extensions for SVE document specifies that 
__ARM_FEATURE_SVE should be set when the compiler supports SVE and
implements all the extensions described in the document.

This is currently not yet the case, so the feature should be disabled
until the compiler can provide all the extensions as described.


https://reviews.llvm.org/D65404

Files:
  lib/Basic/Targets/AArch64.cpp
  test/Preprocessor/aarch64-target-features.c


Index: test/Preprocessor/aarch64-target-features.c
===
--- test/Preprocessor/aarch64-target-features.c
+++ test/Preprocessor/aarch64-target-features.c
@@ -88,7 +88,7 @@
 // RUN: %clang -target aarch64 -mtune=cyclone -### -c %s 2>&1 | FileCheck 
-check-prefix=CHECK-MTUNE-CYCLONE %s
 
 // RUN: %clang -target aarch64-none-linux-gnu -march=armv8-a+sve -x c -E -dM 
%s -o - | FileCheck --check-prefix=CHECK-SVE %s
-// CHECK-SVE: __ARM_FEATURE_SVE 1
+// CHECK-SVE-NOT: __ARM_FEATURE_SVE 1
 
 // RUN: %clang -target aarch64-none-linux-gnu -march=armv8.2a+dotprod -x c -E 
-dM %s -o - | FileCheck --check-prefix=CHECK-DOTPROD %s
 // CHECK-DOTPROD: __ARM_FEATURE_DOTPROD 1
Index: lib/Basic/Targets/AArch64.cpp
===
--- lib/Basic/Targets/AArch64.cpp
+++ lib/Basic/Targets/AArch64.cpp
@@ -196,9 +196,6 @@
 Builder.defineMacro("__ARM_NEON_FP", "0xE");
   }
 
-  if (FPU & SveMode)
-Builder.defineMacro("__ARM_FEATURE_SVE", "1");
-
   if (HasCRC)
 Builder.defineMacro("__ARM_FEATURE_CRC32", "1");
 


Index: test/Preprocessor/aarch64-target-features.c
===
--- test/Preprocessor/aarch64-target-features.c
+++ test/Preprocessor/aarch64-target-features.c
@@ -88,7 +88,7 @@
 // RUN: %clang -target aarch64 -mtune=cyclone -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-MTUNE-CYCLONE %s
 
 // RUN: %clang -target aarch64-none-linux-gnu -march=armv8-a+sve -x c -E -dM %s -o - | FileCheck --check-prefix=CHECK-SVE %s
-// CHECK-SVE: __ARM_FEATURE_SVE 1
+// CHECK-SVE-NOT: __ARM_FEATURE_SVE 1
 
 // RUN: %clang -target aarch64-none-linux-gnu -march=armv8.2a+dotprod -x c -E -dM %s -o - | FileCheck --check-prefix=CHECK-DOTPROD %s
 // CHECK-DOTPROD: __ARM_FEATURE_DOTPROD 1
Index: lib/Basic/Targets/AArch64.cpp
===
--- lib/Basic/Targets/AArch64.cpp
+++ lib/Basic/Targets/AArch64.cpp
@@ -196,9 +196,6 @@
 Builder.defineMacro("__ARM_NEON_FP", "0xE");
   }
 
-  if (FPU & SveMode)
-Builder.defineMacro("__ARM_FEATURE_SVE", "1");
-
   if (HasCRC)
 Builder.defineMacro("__ARM_FEATURE_CRC32", "1");
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65405: [Parser] Use special definition for pragma annotations

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff created this revision.
sepavloff added reviewers: rsmith, rjmccall.
Herald added a reviewer: jdoerfert.
Herald added a project: clang.

Previously pragma annotation tokens were described as any other
annotations in TokenKinds.def. This change introduces special macro
PRAGMA_ANNOTATION for the pragma descriptions. It allows implementing
checks that deal with pragma annotations only.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D65405

Files:
  clang/include/clang/Basic/TokenKinds.def
  clang/include/clang/Basic/TokenKinds.h
  clang/lib/Basic/TokenKinds.cpp

Index: clang/lib/Basic/TokenKinds.cpp
===
--- clang/lib/Basic/TokenKinds.cpp
+++ clang/lib/Basic/TokenKinds.cpp
@@ -45,3 +45,13 @@
   }
   return nullptr;
 }
+
+bool tok::isPragmaAnnotation(TokenKind Kind) {
+  switch (Kind) {
+#define PRAGMA_ANNOTATION(X) case annot_ ## X: return true;
+#include "clang/Basic/TokenKinds.def"
+  default:
+break;
+  }
+  return false;
+}
Index: clang/include/clang/Basic/TokenKinds.h
===
--- clang/include/clang/Basic/TokenKinds.h
+++ clang/include/clang/Basic/TokenKinds.h
@@ -98,6 +98,9 @@
   return false;
 }
 
+/// Return true if this is an annotation token representing a pragma.
+bool isPragmaAnnotation(TokenKind K);
+
 }  // end namespace tok
 }  // end namespace clang
 
Index: clang/include/clang/Basic/TokenKinds.def
===
--- clang/include/clang/Basic/TokenKinds.def
+++ clang/include/clang/Basic/TokenKinds.def
@@ -68,6 +68,9 @@
 #ifndef ANNOTATION
 #define ANNOTATION(X) TOK(annot_ ## X)
 #endif
+#ifndef PRAGMA_ANNOTATION
+#define PRAGMA_ANNOTATION(X) ANNOTATION(X)
+#endif
 
 //===--===//
 // Preprocessor keywords.
@@ -729,103 +732,103 @@
 // Annotation for #pragma unused(...)
 // For each argument inside the parentheses the pragma handler will produce
 // one 'pragma_unused' annotation token followed by the argument token.
-ANNOTATION(pragma_unused)
+PRAGMA_ANNOTATION(pragma_unused)
 
 // Annotation for #pragma GCC visibility...
 // The lexer produces these so that they only take effect when the parser
 // handles them.
-ANNOTATION(pragma_vis)
+PRAGMA_ANNOTATION(pragma_vis)
 
 // Annotation for #pragma pack...
 // The lexer produces these so that they only take effect when the parser
 // handles them.
-ANNOTATION(pragma_pack)
+PRAGMA_ANNOTATION(pragma_pack)
 
 // Annotation for #pragma clang __debug parser_crash...
 // The lexer produces these so that they only take effect when the parser
 // handles them.
-ANNOTATION(pragma_parser_crash)
+PRAGMA_ANNOTATION(pragma_parser_crash)
 
 // Annotation for #pragma clang __debug captured...
 // The lexer produces these so that they only take effect when the parser
 // handles them.
-ANNOTATION(pragma_captured)
+PRAGMA_ANNOTATION(pragma_captured)
 
 // Annotation for #pragma clang __debug dump...
 // The lexer produces these so that the parser and semantic analysis can
 // look up and dump the operand.
-ANNOTATION(pragma_dump)
+PRAGMA_ANNOTATION(pragma_dump)
 
 // Annotation for #pragma ms_struct...
 // The lexer produces these so that they only take effect when the parser
 // handles them.
-ANNOTATION(pragma_msstruct)
+PRAGMA_ANNOTATION(pragma_msstruct)
 
 // Annotation for #pragma align...
 // The lexer produces these so that they only take effect when the parser
 // handles them.
-ANNOTATION(pragma_align)
+PRAGMA_ANNOTATION(pragma_align)
 
 // Annotation for #pragma weak id
 // The lexer produces these so that they only take effect when the parser
 // handles them.
-ANNOTATION(pragma_weak)
+PRAGMA_ANNOTATION(pragma_weak)
 
 // Annotation for #pragma weak id = id
 // The lexer produces these so that they only take effect when the parser
 // handles them.
-ANNOTATION(pragma_weakalias)
+PRAGMA_ANNOTATION(pragma_weakalias)
 
 // Annotation for #pragma redefine_extname...
 // The lexer produces these so that they only take effect when the parser
 // handles them.
-ANNOTATION(pragma_redefine_extname)
+PRAGMA_ANNOTATION(pragma_redefine_extname)
 
 // Annotation for #pragma STDC FP_CONTRACT...
 // The lexer produces these so that they only take effect when the parser
 // handles them.
-ANNOTATION(pragma_fp_contract)
+PRAGMA_ANNOTATION(pragma_fp_contract)
 
 // Annotation for #pragma STDC FENV_ACCESS
 // The lexer produces these so that they only take effect when the parser
 // handles them.
-ANNOTATION(pragma_fenv_access)
+PRAGMA_ANNOTATION(pragma_fenv_access)
 
 // Annotation for #pragma pointers_to_members...
 // The lexer produces these so that they only take effect when the parser
 // handles them.
-ANNOTATION(pragma_ms_pointers_to_members)
+PRAGMA_ANNOTATION(pragma_ms_pointers_to_members)
 
 // Annotation for #pragma vtordisp...
 // The lexer produces these so that they only take effect when

[PATCH] D64146: [ConstExprPreter][WIP] Initial patch for the constexpr interpreter

[Nandor Licker via Phabricator via cfe-commits]

nand added a comment.

> How do you intend to represent pointers cast to integer types? Allocating 64 
> bits of state for a 64-bit integer is insufficient to model that case.

Is this ever going to be allowed in constexpr? If that is the case, we'll add a 
separate type for all integers which are large enough to hold a pointer, a 
tagged union indicating whether the value is a number or a pointer. This would 
add significant overhead, but I don't see any other way which can correctly 
diagnose UB when casting a random integer to a pointer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64146



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

[PATCH] D65406: [Parser] Change parameter type from int to enum

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff created this revision.
sepavloff added reviewers: rsmith, rjmccall.
Herald added a project: clang.

Some parser functions accept argument of type unsigned while it is
actually of type DeclSpec::TST. No functional changes.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D65406

Files:
  clang/include/clang/Parse/Parser.h
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/Parser.cpp


Index: clang/lib/Parse/Parser.cpp
===
--- clang/lib/Parse/Parser.cpp
+++ clang/lib/Parse/Parser.cpp
@@ -174,7 +174,7 @@
   return ExpectAndConsume(tok::semi, DiagID);
 }
 
-void Parser::ConsumeExtraSemi(ExtraSemiKind Kind, unsigned TST) {
+void Parser::ConsumeExtraSemi(ExtraSemiKind Kind, DeclSpec::TST TST) {
   if (!Tok.is(tok::semi)) return;
 
   bool HadMultipleSemis = false;
@@ -202,7 +202,7 @@
 
   if (Kind != AfterMemberFunctionDefinition || HadMultipleSemis)
 Diag(StartLoc, diag::ext_extra_semi)
-<< Kind << DeclSpec::getSpecifierName((DeclSpec::TST)TST,
+<< Kind << DeclSpec::getSpecifierName(TST,
 
Actions.getASTContext().getPrintingPolicy())
 << FixItHint::CreateRemoval(SourceRange(StartLoc, EndLoc));
   else
Index: clang/lib/Parse/ParseDeclCXX.cpp
===
--- clang/lib/Parse/ParseDeclCXX.cpp
+++ clang/lib/Parse/ParseDeclCXX.cpp
@@ -4337,7 +4337,7 @@
   while (Tok.isNot(tok::r_brace) && !isEofOrEom()) {
 // __if_exists, __if_not_exists can nest.
 if (Tok.isOneOf(tok::kw___if_exists, tok::kw___if_not_exists)) {
-  ParseMicrosoftIfExistsClassDeclaration((DeclSpec::TST)TagType,
+  ParseMicrosoftIfExistsClassDeclaration(TagType,
  AccessAttrs, CurAS);
   continue;
 }
Index: clang/lib/Parse/ParseDecl.cpp
===
--- clang/lib/Parse/ParseDecl.cpp
+++ clang/lib/Parse/ParseDecl.cpp
@@ -4098,7 +4098,7 @@
 /// [OBC]   '@' 'defs' '(' class-name ')'
 ///
 void Parser::ParseStructUnionBody(SourceLocation RecordLoc,
-  unsigned TagType, Decl *TagDecl) {
+  DeclSpec::TST TagType, Decl *TagDecl) {
   PrettyDeclStackTraceEntry CrashInfo(Actions.Context, TagDecl, RecordLoc,
   "parsing struct/union body");
   assert(!getLangOpts().CPlusPlus && "C++ declarations not supported");
Index: clang/include/clang/Parse/Parser.h
===
--- clang/include/clang/Parse/Parser.h
+++ clang/include/clang/Parse/Parser.h
@@ -972,7 +972,7 @@
   };
 
   /// Consume any extra semi-colons until the end of the line.
-  void ConsumeExtraSemi(ExtraSemiKind Kind, unsigned TST = TST_unspecified);
+  void ConsumeExtraSemi(ExtraSemiKind Kind, DeclSpec::TST T = TST_unspecified);
 
   /// Return false if the next token is an identifier. An 'expected identifier'
   /// error is emitted otherwise.
@@ -2160,7 +2160,7 @@
   const ParsedTemplateInfo &TemplateInfo,
   AccessSpecifier AS, DeclSpecContext DSC);
   void ParseEnumBody(SourceLocation StartLoc, Decl *TagDecl);
-  void ParseStructUnionBody(SourceLocation StartLoc, unsigned TagType,
+  void ParseStructUnionBody(SourceLocation StartLoc, DeclSpec::TST TagType,
 Decl *TagDecl);
 
   void ParseStructDeclaration(


Index: clang/lib/Parse/Parser.cpp
===
--- clang/lib/Parse/Parser.cpp
+++ clang/lib/Parse/Parser.cpp
@@ -174,7 +174,7 @@
   return ExpectAndConsume(tok::semi, DiagID);
 }
 
-void Parser::ConsumeExtraSemi(ExtraSemiKind Kind, unsigned TST) {
+void Parser::ConsumeExtraSemi(ExtraSemiKind Kind, DeclSpec::TST TST) {
   if (!Tok.is(tok::semi)) return;
 
   bool HadMultipleSemis = false;
@@ -202,7 +202,7 @@
 
   if (Kind != AfterMemberFunctionDefinition || HadMultipleSemis)
 Diag(StartLoc, diag::ext_extra_semi)
-<< Kind << DeclSpec::getSpecifierName((DeclSpec::TST)TST,
+<< Kind << DeclSpec::getSpecifierName(TST,
 Actions.getASTContext().getPrintingPolicy())
 << FixItHint::CreateRemoval(SourceRange(StartLoc, EndLoc));
   else
Index: clang/lib/Parse/ParseDeclCXX.cpp
===
--- clang/lib/Parse/ParseDeclCXX.cpp
+++ clang/lib/Parse/ParseDeclCXX.cpp
@@ -4337,7 +4337,7 @@
   while (Tok.isNot(tok::r_brace) && !isEofOrEom()) {
 // __if_exists, __if_not_exists can nest.
 if (Tok.isOneOf(tok::kw___if_exists, tok::kw___if_not_exists)) {
-  ParseMicrosoftIfExistsClassDeclaration((DeclSpec::TST)TagType,
+  ParseMicrosoftIfExistsClassDeclaration(TagType,
  AccessAttrs, CurAS);
   co

[PATCH] D65387: [clangd] Add a callback mechanism for handling responses from client.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Could you add a bit more context to the description of the change: why do we 
need the callback and what is the problem with what we have now?
I'm sure this was discussed offline and the change is justified, just wanted to 
make sure we write it down in the change history.




Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:334
+  // No callback for the client reply, just log them.
+  if (!CB) {
+if (Result)

NIT: simplify to
```
if (CB)
  return CB(std::move(Result));
// rest of the code.
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65387



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

[PATCH] D64932: [Parser] Emit descriptive diagnostic for misplaced pragma

[Serge Pavlov via Phabricator via cfe-commits]

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

Updated patch

Move logically independent changes to separate patches.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64932

Files:
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/lib/Basic/TokenKinds.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/test/Parser/pragma-attribute-context.cpp
  clang/test/Parser/pragma-fp-contract.c
  clang/test/Parser/pragma-fp-contract.cpp

Index: clang/test/Parser/pragma-fp-contract.cpp
===
--- /dev/null
+++ clang/test/Parser/pragma-fp-contract.cpp
@@ -0,0 +1,32 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+void f1(void) {
+  int x = 0;
+/* expected-error@+1 {{'#pragma fp_contract' can only appear at file scope or at the start of a compound statement}} */
+#pragma STDC FP_CONTRACT ON
+}
+
+void f2(void) {
+  #pragma STDC FP_CONTRACT OFF
+  #pragma STDC FP_CONTRACT ON 
+}
+
+struct S1 {
+// expected-error@+1 {{this pragma cannot appear in struct declaration}}
+#pragma STDC FP_CONTRACT ON
+  float f1;
+};
+
+union U1 {
+  float f1;
+  float f2;
+// expected-error@+1 {{this pragma cannot appear in union declaration}}
+#pragma STDC FP_CONTRACT ON
+};
+
+class C1 {
+  float f1;
+// expected-error@+1 {{this pragma cannot appear in class declaration}}
+#pragma STDC FP_CONTRACT ON
+  float f2;
+};
Index: clang/test/Parser/pragma-fp-contract.c
===
--- clang/test/Parser/pragma-fp-contract.c
+++ clang/test/Parser/pragma-fp-contract.c
@@ -10,3 +10,16 @@
   #pragma STDC FP_CONTRACT OFF
   #pragma STDC FP_CONTRACT ON 
 }
+
+struct S1 {
+// expected-error@+1 {{this pragma cannot appear in struct declaration}}
+#pragma STDC FP_CONTRACT ON
+  float f1;
+};
+
+union U1 {
+  float f1;
+  float f2;
+// expected-error@+1 {{this pragma cannot appear in union declaration}}
+#pragma STDC FP_CONTRACT ON
+};
Index: clang/test/Parser/pragma-attribute-context.cpp
===
--- clang/test/Parser/pragma-attribute-context.cpp
+++ clang/test/Parser/pragma-attribute-context.cpp
@@ -31,8 +31,7 @@
 
 struct InStruct {
   // FIXME: This asserts in Objective-C++!
-  // FIXME: This is a horrible diagnostic!
 #ifndef __OBJC__
-  BEGIN_PRAGMA // expected-error {{expected member name or ';' after declaration specifiers}}
+  BEGIN_PRAGMA // expected-error {{this pragma cannot appear in struct declaration}}
 #endif
 };
Index: clang/lib/Parse/ParseDeclCXX.cpp
===
--- clang/lib/Parse/ParseDeclCXX.cpp
+++ clang/lib/Parse/ParseDeclCXX.cpp
@@ -3134,6 +3134,13 @@
   TagDecl);
 
   default:
+if (tok::isPragmaAnnotation(Tok.getKind())) {
+  Diag(Tok.getLocation(), diag::err_pragma_misplaced_in_decl)
+  << DeclSpec::getSpecifierName(TagType,
+   Actions.getASTContext().getPrintingPolicy());
+  ConsumeAnnotationToken();
+  return nullptr;
+}
 return ParseCXXClassMemberDeclaration(AS, AccessAttrs);
   }
 }
Index: clang/lib/Parse/ParseDecl.cpp
===
--- clang/lib/Parse/ParseDecl.cpp
+++ clang/lib/Parse/ParseDecl.cpp
@@ -4148,6 +4148,14 @@
   continue;
 }
 
+if (tok::isPragmaAnnotation(Tok.getKind())) {
+  Diag(Tok.getLocation(), diag::err_pragma_misplaced_in_decl)
+  << DeclSpec::getSpecifierName(
+ TagType, Actions.getASTContext().getPrintingPolicy());
+  ConsumeAnnotationToken();
+  continue;
+}
+
 if (!Tok.is(tok::at)) {
   auto CFieldCallback = [&](ParsingFieldDeclarator &FD) {
 // Install the declarator into the current TagDecl.
Index: clang/lib/Basic/TokenKinds.cpp
===
--- clang/lib/Basic/TokenKinds.cpp
+++ clang/lib/Basic/TokenKinds.cpp
@@ -12,6 +12,7 @@
 
 #include "clang/Basic/TokenKinds.h"
 #include "llvm/Support/ErrorHandling.h"
+#include 
 using namespace clang;
 
 static const char * const TokNames[] = {
Index: clang/include/clang/Basic/DiagnosticParseKinds.td
===
--- clang/include/clang/Basic/DiagnosticParseKinds.td
+++ clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -976,6 +976,8 @@
 def warn_pragma_invalid_argument : Warning<
   "unexpected argument '%0' to '#pragma %1'%select{|; expected %3}2">, InGroup;
 
+def err_pragma_misplaced_in_decl : Error<"this pragma cannot appear in %0 declaration">;
+
 // '#pragma clang section' related errors
 def err_pragma_expected_clang_section_name : Error<
   "expected one of [bss|data|rodata|text] section kind in '#pragma %0'">;

[PATCH] D64838: [Attr] Support _attribute__ ((fallthrough))

[Nathan Huckleberry via Phabricator via cfe-commits]

Nathan-Huckleberry updated this revision to Diff 212189.
Nathan-Huckleberry added a comment.

- Fix test, formatting and conditional check


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64838

Files:
  clang/include/clang/Basic/Attr.td
  clang/lib/Parse/ParseStmt.cpp
  clang/lib/Sema/AnalysisBasedWarnings.cpp
  clang/test/Index/blocks.c
  clang/test/Index/load-exprs.c
  clang/test/Sema/fallthrough-attr.c
  clang/test/SemaCXX/switch-implicit-fallthrough.cpp
  clang/test/SemaCXX/warn-unused-label-error.cpp

Index: clang/test/SemaCXX/warn-unused-label-error.cpp
===
--- clang/test/SemaCXX/warn-unused-label-error.cpp
+++ clang/test/SemaCXX/warn-unused-label-error.cpp
@@ -18,9 +18,9 @@
   }
 
   void h() {
-D: // expected-warning {{unused label 'D'}}
-  #pragma weak unused_local_static
-  __attribute__((unused))  // expected-warning {{declaration does not declare anything}}
-  ;
+  D:
+#pragma weak unused_local_static
+__attribute__((unused)) // expected-error {{'unused' attribute cannot be applied to a statement}}
+;
   }
 }
Index: clang/test/SemaCXX/switch-implicit-fallthrough.cpp
===
--- clang/test/SemaCXX/switch-implicit-fallthrough.cpp
+++ clang/test/SemaCXX/switch-implicit-fallthrough.cpp
@@ -329,3 +329,15 @@
   }
   return n;
 }
+
+int fallthrough_attribute_spelling(int n) {
+  switch (n) {
+  case 0:
+n++;
+__attribute__((fallthrough));
+  case 1:
+n++;
+break;
+  }
+  return n;
+}
Index: clang/test/Sema/fallthrough-attr.c
===
--- /dev/null
+++ clang/test/Sema/fallthrough-attr.c
@@ -0,0 +1,24 @@
+// RUN: %clang_cc1 -fsyntax-only -std=gnu89 -verify -Wimplicit-fallthrough %s
+// RUN: %clang_cc1 -fsyntax-only -std=gnu99 -verify -Wimplicit-fallthrough %s
+// RUN: %clang_cc1 -fsyntax-only -std=c99 -verify -Wimplicit-fallthrough %s
+// RUN: %clang_cc1 -fsyntax-only -std=c11 -verify -Wimplicit-fallthrough %s
+// RUN: %clang_cc1 -fsyntax-only -std=c2x -DC2X -verify -Wimplicit-fallthrough %s
+
+int fallthrough_attribute_spelling(int n) {
+  switch (n) {
+  case 0:
+n++;
+  case 1:
+#if defined(C2X)
+// expected-warning@-2{{unannotated fall-through between switch labels}} expected-note@-2{{insert '[[fallthrough]];' to silence this warning}} expected-note@-2{{insert 'break;' to avoid fall-through}}
+#else
+// expected-warning@-4{{unannotated fall-through between switch labels}} expected-note@-4{{insert '__attribute__ ((fallthrough));' to silence this warning}} expected-note@-4{{insert 'break;' to avoid fall-through}}
+#endif
+n++;
+__attribute__((fallthrough));
+  case 2:
+n++;
+break;
+  }
+  return n;
+}
Index: clang/test/Index/load-exprs.c
===
--- clang/test/Index/load-exprs.c
+++ clang/test/Index/load-exprs.c
@@ -52,7 +52,7 @@
 // CHECK: load-exprs.c:7:23: DeclRefExpr=x:6:12 Extent=[7:23 - 7:24]
 // CHECK: load-exprs.c:10:5: FunctionDecl=test_blocks:10:5 (Definition) Extent=[10:1 - 21:2]
 // CHECK: load-exprs.c:10:21: ParmDecl=x:10:21 (Definition) Extent=[10:17 - 10:22]
-// CHECK: load-exprs.c:11:15: VarDecl=y:11:15 (Definition) Extent=[11:3 - 11:20]
+// CHECK: load-exprs.c:11:15: VarDecl=y:11:15 (Definition) Extent=[11:11 - 11:20]
 // CHECK: load-exprs.c:11:19: DeclRefExpr=x:10:21 Extent=[11:19 - 11:20]
 // CHECK: load-exprs.c:12:3: CallExpr= Extent=[12:3 - 19:7]
 // CHECK: load-exprs.c:13:17: VarDecl=z:13:17 (Definition) Extent=[13:6 - 13:22]
@@ -78,4 +78,3 @@
 // CHECK: load-exprs.c:31:32: MemberRef=array:24:12 Extent=[31:32 - 31:37]
 // CHECK: load-exprs.c:31:38: DeclRefExpr=StartIndex:27:8 Extent=[31:38 - 31:48]
 // CHECK: load-exprs.c:31:50: MemberRef=b:2:19 Extent=[31:50 - 31:51]
-
Index: clang/test/Index/blocks.c
===
--- clang/test/Index/blocks.c
+++ clang/test/Index/blocks.c
@@ -14,7 +14,7 @@
 // CHECK: blocks.c:7:3: DeclStmt= Extent=[7:3 - 7:26]
 // CHECK: blocks.c:7:21: VarDecl=_foo:7:21 (Definition) Extent=[7:3 - 7:25]
 // CHECK: blocks.c:7:17: TypeRef=struct foo:4:8 Extent=[7:17 - 7:20]
-// CHECK: blocks.c:8:11: VarDecl=i:8:11 (Definition) Extent=[8:3 - 8:16]
+// CHECK: blocks.c:8:11: VarDecl=i:8:11 (Definition) Extent=[8:11 - 8:16]
 // CHECK: blocks.c:8:15: IntegerLiteral= Extent=[8:15 - 8:16]
 // CHECK: blocks.c:9:3: CallExpr= Extent=[9:3 - 9:65]
 // CHECK: blocks.c:9:3: BlockExpr= Extent=[9:3 - 9:58]
@@ -31,4 +31,3 @@
 // CHECK: blocks.c:9:54: DeclRefExpr=i:8:11 Extent=[9:54 - 9:55]
 // CHECK: blocks.c:9:59: UnaryOperator= Extent=[9:59 - 9:64]
 // CHECK: blocks.c:9:60: DeclRefExpr=_foo:7:21 Extent=[9:60 - 9:64]
-
Index: clang/lib/Sema/AnalysisBasedWarnings.cpp
===
--- clang/l

[PATCH] D64863: [clangd] Ignore diags from builtin files

[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. Many thanks for fixing this.




Comment at: clang-tools-extra/clangd/Diagnostics.cpp:111
 
-void adjustDiagFromHeader(Diag &D, const clang::Diagnostic &Info,
+bool adjustDiagFromHeader(Diag &D, const clang::Diagnostic &Info,
   const LangOptions &LangOpts) {

NIT: add a small comment mentioning that returned value indicates if `Diag` was 
changed



Comment at: clang-tools-extra/clangd/Diagnostics.cpp:475
   bool InsideMainFile = isInsideMainFile(Info);
+  SourceManager &SM = Info.getSourceManager();
 

NIT: could use `auto &` here, the `SourceManager` is mentioned explicitly in 
the initializer anyway.



Comment at: clang-tools-extra/clangd/Diagnostics.cpp:563
+LastDiagWasAdjusted =
+!InsideMainFile && adjustDiagFromHeader(*LastDiag, Info, *LangOpts);
 

NIT: I suggest moving the control-flow (`&&`) into a separate `if` statement.
`adjustDiagFromHeader` mutates its arguments and having complex expressions in 
the same statement makes it a little hard to notice that things might get 
mutated there.

But up to you.



Comment at: clang-tools-extra/clangd/Diagnostics.h:148
   llvm::Optional LastDiag;
+  // Set by adjustDiagFromHeader to indicate the fact that LastDiag was not
+  // inside main file initially.

NIT: it's not `adjustDiagFromHeader` that actually sets the value, maybe 
rephrase to avoid confusion?
Something like 
```
/// Set iff adjustDiagFromHeader resulted in changes to LastDiag. 
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64863



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

[PATCH] D64838: [Attr] Support _attribute__ ((fallthrough))

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 added a comment.

Thanks! Better and better:)

Change in clang/test/Index/blocks.c seems not ideal, or is it okay? 
@aaron.ballman


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64838



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

[PATCH] D65407: Don't parse elements of extended vectors as template names

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak created this revision.
ahatanak added a reviewer: rsmith.
ahatanak added a project: clang.
Herald added subscribers: dexonsmith, jkorous.

This patch attempts to fix the bug introduced in r360308, which is causing 
clang to reject the following piece of code:

  typedef __attribute__((__ext_vector_type__(2))) float vector_float2;
  
  bool foo123(vector_float2 &A, vector_float2 &B)
  {
return A.x < B.x && B.y > A.y;
  }

The patch sets `ObjectType` to the type of the extended vector in 
`Sema::ActOnStartCXXMemberReference` so that `Sema::isTemplateName`, which is 
called later ,returns false.

rdar://problem/52619956


Repository:
  rC Clang

https://reviews.llvm.org/D65407

Files:
  lib/Sema/SemaExprCXX.cpp
  test/SemaCXX/vector.cpp


Index: test/SemaCXX/vector.cpp
===
--- test/SemaCXX/vector.cpp
+++ test/SemaCXX/vector.cpp
@@ -333,4 +333,10 @@
   const PR15730<8, char>::type2 PR15730_2 = {};
 }
 
+// This used to be rejected because the name lookup determined 'x' in 'a.x' was
+// a template name.
+bool templateName(char16_e a, char16_e b) {
+  return a.x < b.x && b.y > a.y;
+}
+
 } // namespace Templates
Index: lib/Sema/SemaExprCXX.cpp
===
--- lib/Sema/SemaExprCXX.cpp
+++ lib/Sema/SemaExprCXX.cpp
@@ -6798,7 +6798,15 @@
 ObjectType = ParsedType::make(BaseType);
 MayBePseudoDestructor = true;
 return Base;
-  } else if (!BaseType->isRecordType()) {
+  }
+
+  if (BaseType->isExtVectorType()) {
+ObjectType = ParsedType::make(BaseType);
+MayBePseudoDestructor = true;
+return Base;
+  }
+
+  if (!BaseType->isRecordType()) {
 ObjectType = nullptr;
 MayBePseudoDestructor = true;
 return Base;


Index: test/SemaCXX/vector.cpp
===
--- test/SemaCXX/vector.cpp
+++ test/SemaCXX/vector.cpp
@@ -333,4 +333,10 @@
   const PR15730<8, char>::type2 PR15730_2 = {};
 }
 
+// This used to be rejected because the name lookup determined 'x' in 'a.x' was
+// a template name.
+bool templateName(char16_e a, char16_e b) {
+  return a.x < b.x && b.y > a.y;
+}
+
 } // namespace Templates
Index: lib/Sema/SemaExprCXX.cpp
===
--- lib/Sema/SemaExprCXX.cpp
+++ lib/Sema/SemaExprCXX.cpp
@@ -6798,7 +6798,15 @@
 ObjectType = ParsedType::make(BaseType);
 MayBePseudoDestructor = true;
 return Base;
-  } else if (!BaseType->isRecordType()) {
+  }
+
+  if (BaseType->isExtVectorType()) {
+ObjectType = ParsedType::make(BaseType);
+MayBePseudoDestructor = true;
+return Base;
+  }
+
+  if (!BaseType->isRecordType()) {
 ObjectType = nullptr;
 MayBePseudoDestructor = true;
 return Base;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64838: [Attr] Support _attribute__ ((fallthrough))

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 added inline comments.



Comment at: clang/lib/Parse/ParseStmt.cpp:233
+GNUAttributeLoc = Tok.getLocation();
+MaybeParseGNUAttributes(Attrs);
+goto Retry;

Since you know that tok is kw_attr, I think you can use 'ParseGNUAttributes'.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64838



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

[PATCH] D64666: [Sema] Enable -Wimplicit-float-conversion for integral to floating point precision loss

[Ziang Wan via Phabricator via cfe-commits]

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



Comment at: clang/lib/Sema/SemaChecking.cpp:11656
+  IsListInit =
+  IsListInit || (isa(OrigE) && S.getLangOpts().CPlusPlus);
+

aaron.ballman wrote:
> Do you want to perform the `isa<>` on `OrigE` or on `E` which has had paren 
> and implicit casts stripped?
When we are dealing with Initialization list syntax expression, the out-most 
expression will be of type `InitListExpr`. Since the out-most expression is not 
`ImplicitCastExpr`, `IgnoreParenImpCasts()` has no effect on the `OrigE` 
object. In other words, `OrigE` and `E` will be the same when 
`isa(OrigE)` evaluates to true.

In this case, I personally prefer `OrigE` since it is the "non-processed" 
object. However, the rest of the function uses `E` only. I can change it to `E`.


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

https://reviews.llvm.org/D64666



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

[PATCH] D65404: [AArch64] Disable __ARM_FEATURE_SVE without ACLE.

[Renato Golin via Phabricator via cfe-commits]

rengolin added a comment.

I understand (and agree with) the reasoning of this patch, but wouldn't this 
also make it harder to test the current behaviour?

I mean, LLVM doesn't support officially SVE entirely, so there's no point in 
expecting anything to work for now. What is the coverage of the current ACLE 
implementation in Clang/LLVM?

If we already have the most used ones, how about adding a warning that SVE 
support is not complete, YMMV, etc. instead?


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

https://reviews.llvm.org/D65404



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

[PATCH] D64838: [Attr] Support _attribute__ ((fallthrough))

[Nathan Huckleberry via Phabricator via cfe-commits]

Nathan-Huckleberry updated this revision to Diff 212198.
Nathan-Huckleberry added a comment.

- Fix test case spacing


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64838

Files:
  clang/include/clang/Basic/Attr.td
  clang/lib/Parse/ParseStmt.cpp
  clang/lib/Sema/AnalysisBasedWarnings.cpp
  clang/test/Index/blocks.c
  clang/test/Index/load-exprs.c
  clang/test/Sema/fallthrough-attr.c
  clang/test/SemaCXX/switch-implicit-fallthrough.cpp
  clang/test/SemaCXX/warn-unused-label-error.cpp

Index: clang/test/SemaCXX/warn-unused-label-error.cpp
===
--- clang/test/SemaCXX/warn-unused-label-error.cpp
+++ clang/test/SemaCXX/warn-unused-label-error.cpp
@@ -18,9 +18,9 @@
   }
 
   void h() {
-D: // expected-warning {{unused label 'D'}}
-  #pragma weak unused_local_static
-  __attribute__((unused))  // expected-warning {{declaration does not declare anything}}
-  ;
+  D:
+#pragma weak unused_local_static
+__attribute__((unused)) // expected-error {{'unused' attribute cannot be applied to a statement}}
+;
   }
 }
Index: clang/test/SemaCXX/switch-implicit-fallthrough.cpp
===
--- clang/test/SemaCXX/switch-implicit-fallthrough.cpp
+++ clang/test/SemaCXX/switch-implicit-fallthrough.cpp
@@ -329,3 +329,15 @@
   }
   return n;
 }
+
+int fallthrough_attribute_spelling(int n) {
+  switch (n) {
+  case 0:
+n++;
+__attribute__((fallthrough));
+  case 1:
+n++;
+break;
+  }
+  return n;
+}
Index: clang/test/Sema/fallthrough-attr.c
===
--- /dev/null
+++ clang/test/Sema/fallthrough-attr.c
@@ -0,0 +1,24 @@
+// RUN: %clang_cc1 -fsyntax-only -std=gnu89 -verify -Wimplicit-fallthrough %s
+// RUN: %clang_cc1 -fsyntax-only -std=gnu99 -verify -Wimplicit-fallthrough %s
+// RUN: %clang_cc1 -fsyntax-only -std=c99 -verify -Wimplicit-fallthrough %s
+// RUN: %clang_cc1 -fsyntax-only -std=c11 -verify -Wimplicit-fallthrough %s
+// RUN: %clang_cc1 -fsyntax-only -std=c2x -DC2X -verify -Wimplicit-fallthrough %s
+
+int fallthrough_attribute_spelling(int n) {
+  switch (n) {
+  case 0:
+n++;
+  case 1:
+#if defined(C2X)
+// expected-warning@-2{{unannotated fall-through between switch labels}} expected-note@-2{{insert '[[fallthrough]];' to silence this warning}} expected-note@-2{{insert 'break;' to avoid fall-through}}
+#else
+// expected-warning@-4{{unannotated fall-through between switch labels}} expected-note@-4{{insert '__attribute__((fallthrough));' to silence this warning}} expected-note@-4{{insert 'break;' to avoid fall-through}}
+#endif
+n++;
+__attribute__((fallthrough));
+  case 2:
+n++;
+break;
+  }
+  return n;
+}
Index: clang/test/Index/load-exprs.c
===
--- clang/test/Index/load-exprs.c
+++ clang/test/Index/load-exprs.c
@@ -52,7 +52,7 @@
 // CHECK: load-exprs.c:7:23: DeclRefExpr=x:6:12 Extent=[7:23 - 7:24]
 // CHECK: load-exprs.c:10:5: FunctionDecl=test_blocks:10:5 (Definition) Extent=[10:1 - 21:2]
 // CHECK: load-exprs.c:10:21: ParmDecl=x:10:21 (Definition) Extent=[10:17 - 10:22]
-// CHECK: load-exprs.c:11:15: VarDecl=y:11:15 (Definition) Extent=[11:3 - 11:20]
+// CHECK: load-exprs.c:11:15: VarDecl=y:11:15 (Definition) Extent=[11:11 - 11:20]
 // CHECK: load-exprs.c:11:19: DeclRefExpr=x:10:21 Extent=[11:19 - 11:20]
 // CHECK: load-exprs.c:12:3: CallExpr= Extent=[12:3 - 19:7]
 // CHECK: load-exprs.c:13:17: VarDecl=z:13:17 (Definition) Extent=[13:6 - 13:22]
@@ -78,4 +78,3 @@
 // CHECK: load-exprs.c:31:32: MemberRef=array:24:12 Extent=[31:32 - 31:37]
 // CHECK: load-exprs.c:31:38: DeclRefExpr=StartIndex:27:8 Extent=[31:38 - 31:48]
 // CHECK: load-exprs.c:31:50: MemberRef=b:2:19 Extent=[31:50 - 31:51]
-
Index: clang/test/Index/blocks.c
===
--- clang/test/Index/blocks.c
+++ clang/test/Index/blocks.c
@@ -14,7 +14,7 @@
 // CHECK: blocks.c:7:3: DeclStmt= Extent=[7:3 - 7:26]
 // CHECK: blocks.c:7:21: VarDecl=_foo:7:21 (Definition) Extent=[7:3 - 7:25]
 // CHECK: blocks.c:7:17: TypeRef=struct foo:4:8 Extent=[7:17 - 7:20]
-// CHECK: blocks.c:8:11: VarDecl=i:8:11 (Definition) Extent=[8:3 - 8:16]
+// CHECK: blocks.c:8:11: VarDecl=i:8:11 (Definition) Extent=[8:11 - 8:16]
 // CHECK: blocks.c:8:15: IntegerLiteral= Extent=[8:15 - 8:16]
 // CHECK: blocks.c:9:3: CallExpr= Extent=[9:3 - 9:65]
 // CHECK: blocks.c:9:3: BlockExpr= Extent=[9:3 - 9:58]
@@ -31,4 +31,3 @@
 // CHECK: blocks.c:9:54: DeclRefExpr=i:8:11 Extent=[9:54 - 9:55]
 // CHECK: blocks.c:9:59: UnaryOperator= Extent=[9:59 - 9:64]
 // CHECK: blocks.c:9:60: DeclRefExpr=_foo:7:21 Extent=[9:60 - 9:64]
-
Index: clang/lib/Sema/AnalysisBasedWarnings.cpp
===
--- clang/lib/Sema/AnalysisBasedW

[PATCH] D64666: [Sema] Enable -Wimplicit-float-conversion for integral to floating point precision loss

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.

LGTM!




Comment at: clang/lib/Sema/SemaChecking.cpp:11656
+  IsListInit =
+  IsListInit || (isa(OrigE) && S.getLangOpts().CPlusPlus);
+

ziangwan wrote:
> aaron.ballman wrote:
> > Do you want to perform the `isa<>` on `OrigE` or on `E` which has had paren 
> > and implicit casts stripped?
> When we are dealing with Initialization list syntax expression, the out-most 
> expression will be of type `InitListExpr`. Since the out-most expression is 
> not `ImplicitCastExpr`, `IgnoreParenImpCasts()` has no effect on the `OrigE` 
> object. In other words, `OrigE` and `E` will be the same when 
> `isa(OrigE)` evaluates to true.
> 
> In this case, I personally prefer `OrigE` since it is the "non-processed" 
> object. However, the rest of the function uses `E` only. I can change it to 
> `E`.
Ah, I was thinking we had to contend with a case like `({1, 2, 3})` where 
there's a `ParenExpr` involved, but I am not certain that's a case we have to 
worry about.


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

https://reviews.llvm.org/D64666



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

[PATCH] D64475: [clangd] Duplicate lines of semantic highlightings sent removed.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

In D64475#1602195 , @jvikstrom wrote:

> In D64475#1593481 , @ilya-biryukov 
> wrote:
>
> > The fix for a race condition on remove has landed in rL366577 
> > , this revision would need a small 
> > update after it.
>
>
> Fixed to work with that patch.
>  Should the diffing be moved somewhere else that is not inside the publish 
> function though? That would require moving the state outside the LSP server 
> though and handle the onClose/onOpen events somewhere else though. 
>  Maybe it's ok to keep the diffing in the publish lock?


Yeah, let's keep it in publish for now. Diffing in there does not change 
complexity of the operations.
If it ever turns out that the constant factor is too high, we can figure out 
ways to fight this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64475



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

[PATCH] D64146: [ConstExprPreter][WIP] Initial patch for the constexpr interpreter

[JF Bastien via Phabricator via cfe-commits]

jfb added a comment.

In D64146#1604717 , @nand wrote:

> > How do you intend to represent pointers cast to integer types? Allocating 
> > 64 bits of state for a 64-bit integer is insufficient to model that case.
>
> Is this ever going to be allowed in constexpr? If that is the case, we'll add 
> a separate type for all integers which are large enough to hold a pointer, a 
> tagged union indicating whether the value is a number or a pointer. This 
> would add significant overhead, but I don't see any other way which can 
> correctly diagnose UB when casting a random integer to a pointer.


Most integers won't be in that category, you can therefore speculate that fact 
when emitting bytecode, and throw away byte code when the assumption turns out 
to be wrong (and re-generate the more expensive byte code).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64146



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

[PATCH] D64793: [Driver] Properly use values-X[ca].o, values-xpg[46].o on Solaris

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

In D64793#1597765 , @ro wrote:

> In D64793#1597550 , @jyknight wrote:
>
> > How about instead just adding "values-xpg6.o" unconditionally, alongside 
> > the current unconditional "values-Xa.o", and just forget about the xpg4 and 
> > Xc modes?
>
>
> If all else fails, that would have to be the last fallback.  I'd rather do 
> things correctly, though.


I think the cost of complexity in the solaris-specific part of the driver is 
low, so if the maintainer thinks it's worth having, we can certainly add it.




Comment at: lib/Driver/ToolChains/Solaris.cpp:16
 #include "clang/Driver/Options.h"
+#include "clang/Frontend/LangStandard.h"
 #include "llvm/Option/ArgList.h"

ro wrote:
> jyknight wrote:
> > I'm not sure that's an acceptable dependency -- I think Driver probably is 
> > not supposed to depend on Frontend. If so, I guess LangStandard should move 
> > to clang/Basic/. Which also means moving InputKind from 
> > clang/include/clang/Frontend/FrontendOptions.h.
> > 
> > (Maybe someone else can weigh in on this question?)
> I wondered about this myself, including frontend code in the
> driver might be considered a layering violation.  I certainly need
> guidance here what is and isn't acceptable here.
I see there are no other includes of Frontend from Driver, so I think 
LangStandards* does need to move to Basic. The only piece of InputKind that's 
needed is the language enum. I'm surprised there isn't already one somewhere 
else, but if there isn't, I think it would be reasonable to define the input 
kind languages in LangStandard.h and use them from FrontendOptions.



Comment at: lib/Frontend/LangStandards.cpp:31-37
   Kind K = llvm::StringSwitch(Name)
 #define LANGSTANDARD(id, name, lang, desc, features) \
 .Case(name, lang_##id)
+#define LANGSTANDARD_ALIAS(id, alias) \
+.Case(alias, lang_##id)
 #include "clang/Frontend/LangStandards.def"
 .Default(lang_unspecified);

I see that this code pattern is repeated in two other places, 
lib/Tooling/InterpolatingCompilationDatabase.cpp and 
lib/Frontend/CompilerInvocation.cpp. I think it would be good to factor out a 
string-to-kind helper and use it in the three places.


Repository:
  rC Clang

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

https://reviews.llvm.org/D64793



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

[PATCH] D65343: [clang-tidy] Fix the documentation for linuxkernel-must-use-errs.

[Tom Roeder via Phabricator via cfe-commits]

tmroeder updated this revision to Diff 212201.
tmroeder added a comment.

Sync to HEAD to prepare for commit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65343

Files:
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/linuxkernel-must-use-errs.rst


Index: clang-tools-extra/docs/clang-tidy/checks/linuxkernel-must-use-errs.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/linuxkernel-must-use-errs.rst
+++ clang-tools-extra/docs/clang-tidy/checks/linuxkernel-must-use-errs.rst
@@ -3,14 +3,16 @@
 linuxkernel-must-use-errs
 =
 
-Checks for cases where the kernel error functions ``ERR_PTR``,
-``PTR_ERR``, ``IS_ERR``, ``IS_ERR_OR_NULL``, ``ERR_CAST``, and
-``PTR_ERR_OR_ZERO`` are called but the results are not used. These
-functions are marked with ``__attribute__((warn_unused_result))``, but
-the compiler warning for this attribute is not always enabled.
-
-This also checks for unused values returned by functions that return
-``ERR_PTR``.
+Checks Linux kernel code to see if it uses the results from the functions in
+``linux/err.h``. Also checks to see if code uses the results from functions 
that
+directly return a value from one of these error functions.
+
+This is important in the Linux kernel because ``ERR_PTR``, ``PTR_ERR``,
+``IS_ERR``, ``IS_ERR_OR_NULL``, ``ERR_CAST``, and ``PTR_ERR_OR_ZERO`` return
+values must be checked, since positive pointers and negative error codes are
+being used in the same context. These functions are marked with
+``__attribute__((warn_unused_result))``, but some kernel versions do not have
+this warning enabled for clang.
 
 Examples:
 
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -71,15 +71,7 @@
   ` check.
 
   Checks Linux kernel code to see if it uses the results from the functions in
-  ``linux/err.h``. Also checks to see if code uses the results from functions 
that
-  directly return a value from one of these error functions.
-
-  This is important in the Linux kernel because ``ERR_PTR``, ``PTR_ERR``,
-  ``IS_ERR``, ``IS_ERR_OR_NULL``, ``ERR_CAST``, and ``PTR_ERR_OR_ZERO`` return
-  values must be checked, since positive pointers and negative error codes are
-  being used in the same context. These functions are marked with
-  ``__attribute__((warn_unused_result))``, but some kernel versions do not have
-  this warning enabled for clang.
+  ``linux/err.h``.
 
 Improvements to include-fixer
 -


Index: clang-tools-extra/docs/clang-tidy/checks/linuxkernel-must-use-errs.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/linuxkernel-must-use-errs.rst
+++ clang-tools-extra/docs/clang-tidy/checks/linuxkernel-must-use-errs.rst
@@ -3,14 +3,16 @@
 linuxkernel-must-use-errs
 =
 
-Checks for cases where the kernel error functions ``ERR_PTR``,
-``PTR_ERR``, ``IS_ERR``, ``IS_ERR_OR_NULL``, ``ERR_CAST``, and
-``PTR_ERR_OR_ZERO`` are called but the results are not used. These
-functions are marked with ``__attribute__((warn_unused_result))``, but
-the compiler warning for this attribute is not always enabled.
-
-This also checks for unused values returned by functions that return
-``ERR_PTR``.
+Checks Linux kernel code to see if it uses the results from the functions in
+``linux/err.h``. Also checks to see if code uses the results from functions that
+directly return a value from one of these error functions.
+
+This is important in the Linux kernel because ``ERR_PTR``, ``PTR_ERR``,
+``IS_ERR``, ``IS_ERR_OR_NULL``, ``ERR_CAST``, and ``PTR_ERR_OR_ZERO`` return
+values must be checked, since positive pointers and negative error codes are
+being used in the same context. These functions are marked with
+``__attribute__((warn_unused_result))``, but some kernel versions do not have
+this warning enabled for clang.
 
 Examples:
 
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -71,15 +71,7 @@
   ` check.
 
   Checks Linux kernel code to see if it uses the results from the functions in
-  ``linux/err.h``. Also checks to see if code uses the results from functions that
-  directly return a value from one of these error functions.
-
-  This is important in the Linux kernel because ``ERR_PTR``, ``PTR_ERR``,
-  ``IS_ERR``, ``IS_ERR_OR_NULL``, ``ERR_CAST``, and ``PTR_ERR_OR_ZERO`` return
-  values must be checked, since positive pointers and negative error codes are
-  being used in the same context. These functions are marked with
-  ``__attribute__((warn_un

[PATCH] D65410: [PassManager] First Pass implementation at -O1 pass pipeline

[Eric Christopher via Phabricator via cfe-commits]

echristo created this revision.
echristo added reviewers: chandlerc, hfinkel.
Herald added subscribers: cfe-commits, jfb, dexonsmith, steven_wu, hiraditya, 
javed.absar, mcrosier, mehdi_amini.
Herald added projects: clang, LLVM.

As a follow-up to my initial mail to llvm-dev here's a first pass at the O1 
 described there.

Some rough internal testing using a bootstrap and test of clang has shown a 
combined build and test time for clang with nearly equivalent performance to O3 
 and quite a speedup over O0 - it's 
currently a little slower than the existing O1 
, likely due to the clang+llvm 
testsuite use of the same binaries many times rather than a few for individual 
tests. Build time is a bit better. For a larger build and smaller test time 
(think a couple of unittests), this is a bit better than either O3 
, O0, or O1 
. Overall binary size drops 
significantly compared to O0.

This change doesn't include any change to move from selection dag to fast isel 
and that will come with other numbers that should help inform that decision. I 
also haven't done any real debuggability studies with this pipeline yet, I 
wanted to get the initial start done so that people could see it and we could 
start tweaking after.

Test updates: Outside of the newpm tests most of the updates are coming from 
either optimization passes not run anymore (and without a compelling argument 
at the moment) that were largely used for canonicalization in clang.

Original post:

http://lists.llvm.org/pipermail/llvm-dev/2019-April/131494.html


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D65410

Files:
  clang/test/CodeGen/2008-07-30-implicit-initialization.c
  clang/test/CodeGen/arm-fp16-arguments.c
  clang/test/CodeGen/arm-vfp16-arguments2.cpp
  clang/test/CodeGenCXX/auto-var-init.cpp
  clang/test/CodeGenCXX/stack-reuse.cpp
  llvm/include/llvm/Passes/PassBuilder.h
  llvm/lib/Passes/PassBuilder.cpp
  llvm/lib/Transforms/IPO/PassManagerBuilder.cpp
  llvm/test/Feature/optnone-opt.ll
  llvm/test/Other/new-pm-defaults.ll
  llvm/test/Other/new-pm-thinlto-defaults.ll
  llvm/test/Transforms/MemCpyOpt/lifetime.ll
  llvm/test/Transforms/PhaseOrdering/simplifycfg-options.ll

Index: llvm/test/Transforms/PhaseOrdering/simplifycfg-options.ll
===
--- llvm/test/Transforms/PhaseOrdering/simplifycfg-options.ll
+++ llvm/test/Transforms/PhaseOrdering/simplifycfg-options.ll
@@ -7,7 +7,7 @@
 
 define i1 @PR33605(i32 %a, i32 %b, i32* %c) {
 ; ALL-LABEL: @PR33605(
-; ALL-NEXT:  for.body:
+; ALL-NEXT:  entry:
 ; ALL-NEXT:[[OR:%.*]] = or i32 [[B:%.*]], [[A:%.*]]
 ; ALL-NEXT:[[ARRAYIDX:%.*]] = getelementptr inbounds i32, i32* [[C:%.*]], i64 1
 ; ALL-NEXT:[[TMP0:%.*]] = load i32, i32* [[ARRAYIDX]], align 4
@@ -18,7 +18,7 @@
 ; ALL-NEXT:tail call void @foo()
 ; ALL-NEXT:br label [[IF_END]]
 ; ALL:   if.end:
-; ALL-NEXT:[[CHANGED_1_OFF0:%.*]] = phi i1 [ true, [[IF_THEN]] ], [ false, [[FOR_BODY:%.*]] ]
+; ALL-NEXT:[[CHANGED_1_OFF0:%.*]] = phi i1 [ true, [[IF_THEN]] ], [ false, [[ENTRY:%.*]] ]
 ; ALL-NEXT:[[TMP1:%.*]] = load i32, i32* [[C]], align 4
 ; ALL-NEXT:[[CMP_1:%.*]] = icmp eq i32 [[OR]], [[TMP1]]
 ; ALL-NEXT:br i1 [[CMP_1]], label [[IF_END_1:%.*]], label [[IF_THEN_1:%.*]]
Index: llvm/test/Transforms/MemCpyOpt/lifetime.ll
===
--- llvm/test/Transforms/MemCpyOpt/lifetime.ll
+++ llvm/test/Transforms/MemCpyOpt/lifetime.ll
@@ -1,4 +1,4 @@
-; RUN: opt < %s -O1 -S | FileCheck %s
+; RUN: opt < %s -O2 -S | FileCheck %s
 
 ; performCallSlotOptzn in MemCpy should not exchange the calls to
 ; @llvm.lifetime.start and @llvm.memcpy.
Index: llvm/test/Other/new-pm-thinlto-defaults.ll
===
--- llvm/test/Other/new-pm-thinlto-defaults.ll
+++ llvm/test/Other/new-pm-thinlto-defaults.ll
@@ -108,13 +108,28 @@
 ; CHECK-O3-NEXT: Running pass: ArgumentPromotionPass
 ; CHECK-O-NEXT: Running pass: CGSCCToFunctionPassAdaptor<{{.*}}PassManager{{.*}}>
 ; CHECK-O-NEXT: Starting llvm::Function pass manager run.
-; CHECK-O-NEXT: Running pass: SROA
+; CHECK-O2-NEXT: Running pass: SROA
+; CHECK-O3-NEXT: Running pass: SROA
+; CHECK-Os-NEXT: Running pass: SROA
+; CHECK-Oz-NEXT: Running pass: SROA
 ; CHECK-O-NEXT: Running pass: EarlyCSEPass
 ; CHECK-O-NEXT: Running analysis: MemorySSAAnalysis
-; CHECK-O-NEXT: Running pass: SpeculativeExecutionPass
-; CHECK-O-NEXT: Running pass: JumpThreadingPass
-; CHECK-O-NEXT: Running analysis: LazyValueAnalysis
-; CHECK-O-NEXT: Running pass: CorrelatedValuePropagationPass
+; CHECK-O2-NEXT: Running pass: SpeculativeExecutionPass
+; CHECK-O3-NEXT: Running pass: SpeculativeExecutionPass
+; C

[PATCH] D64793: [Driver] Properly use values-X[ca].o, values-xpg[46].o on Solaris

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

> I fear it is necessary: at least it matches documented behaviour of both the 
> Sun/Oracle Studio compilers and gcc.

I will defer to your opinion here. But -- one last attempt at dissuading you. :)

Is this really doing something _important_, or is it just legacy cruft which 
can be safely ignored by now? With your "logb" example, it seems to me that it 
is probably preferable to always use the new correct "xpg6" implementation, and 
just ignore the legacy/incorrect version. Similarly, the example given in 
https://gcc.gnu.org/PR40411 of freopen -- again, seems like it'd be better to 
just use the new xpg6 behavior, always.

> The -std= options usually get passed to the linking step because CFLAGS is 
> added to the options as well

With gnu make they are not (unless it's doing a single-step compiling+linking 
step). Other tools like CMake also don't pass standards versions to linking. 
This makes sense, because a program can contain code compiled with multiple 
standards versions, and multiple languages. Thus, I'd expect most users to just 
get the default xpg6 and Xa objects, even if they do specify -std=c90 for 
compilation.


Repository:
  rC Clang

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

https://reviews.llvm.org/D64793



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

[PATCH] D64146: [ConstExprPreter][WIP] Initial patch for the constexpr interpreter

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

In D64146#1604717 , @nand wrote:

> > How do you intend to represent pointers cast to integer types? Allocating 
> > 64 bits of state for a 64-bit integer is insufficient to model that case.
>
> Is this ever going to be allowed in constexpr?


It's not sufficient for this to handle only the things that are allowed in 
constant expressions; you also need to allow the things that are allowed by 
Clang's current constant evaluator, which includes this case. There are also 
constructs that allow arbitrary constant folding within the context of constant 
expression evaluation (such as a `__builtin_constant_p` conditional). So yes, 
you need to deal with this.

> If that is the case, we'll add a separate type for all integers which are 
> large enough to hold a pointer, a tagged union indicating whether the value 
> is a number or a pointer. This would add significant overhead, but I don't 
> see any other way which can correctly diagnose UB when casting a random 
> integer to a pointer.

These cases are likely to be rare enough that separately-allocated storage for 
this case could work. You'll need at least one bit somewhere to track whether 
you're in the "pointer cast to integer" case, though.

(You also need to be able to distinguish between an integer value and an 
uninitialized integer and an out-of-lifetime integer, so you'll presumably need 
some side-table to track the state of subobjects anyway.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64146



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

[PATCH] D64256: Teach some warnings to respect gsl::Pointer and gsl::Owner attributes

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

xazax.hun updated this revision to Diff 212204.
xazax.hun marked an inline comment as done.
xazax.hun added a comment.

- A small refactoring based on an observation from a review comment.


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

https://reviews.llvm.org/D64256

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaInit.cpp
  clang/test/Sema/warn-lifetime-analysis-nocfg.cpp

Index: clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
===
--- /dev/null
+++ clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
@@ -0,0 +1,135 @@
+// RUN: %clang_cc1 -fsyntax-only -Wdangling -Wdangling-field -Wreturn-stack-address -verify %s
+struct [[gsl::Owner(int)]] MyIntOwner {
+  MyIntOwner();
+  int &operator*();
+  int *c_str() const;
+};
+
+struct [[gsl::Pointer(int)]] MyIntPointer {
+  MyIntPointer(int *p = nullptr);
+  // Conversion operator and constructor conversion will result in two
+  // different ASTs. The former is tested with another owner and 
+  // pointer type.
+  MyIntPointer(const MyIntOwner &);
+  int &operator*();
+  MyIntOwner toOwner();
+};
+
+struct [[gsl::Pointer(long)]] MyLongPointerFromConversion {
+  MyLongPointerFromConversion(long *p = nullptr);
+  long &operator*();
+};
+
+struct [[gsl::Owner(long)]] MyLongOwnerWithConversion {
+  MyLongOwnerWithConversion();
+  operator MyLongPointerFromConversion();
+  long &operator*();
+  MyIntPointer releaseAsMyPointer();
+  long *releaseAsRawPointer();
+};
+
+void danglingHeapObject() {
+  new MyLongPointerFromConversion(MyLongOwnerWithConversion{}); // expected-warning {{object backing the pointer will be destroyed at the end of the full-expression}}
+  new MyIntPointer(MyIntOwner{}); // expected-warning {{object backing the pointer will be destroyed at the end of the full-expression}}
+}
+
+void intentionalFalseNegative() {
+  int i;
+  MyIntPointer p{&i};
+  // In this case we do not have enough information in a statement local
+  // analysis to detect the problem.
+  new MyIntPointer(p);
+  new MyIntPointer(MyIntPointer{p});
+}
+
+MyIntPointer ownershipTransferToMyPointer() {
+  MyLongOwnerWithConversion t;
+  return t.releaseAsMyPointer(); // ok
+}
+
+long *ownershipTransferToRawPointer() {
+  MyLongOwnerWithConversion t;
+  return t.releaseAsRawPointer(); // ok
+}
+
+int *danglingRawPtrFromLocal() {
+  MyIntOwner t;
+  return t.c_str(); // TODO
+}
+
+int *danglingRawPtrFromTemp() {
+  MyIntPointer p;
+  return p.toOwner().c_str(); // TODO
+}
+
+struct Y {
+  int a[4];
+};
+
+void dangligGslPtrFromTemporary() {
+  MyIntPointer p = Y{}.a; // expected-warning {{temporary whose address is used as value of local variable 'p' will be destroyed at the end of the full-expression}}
+  (void)p;
+}
+
+struct DanglingGslPtrField {
+  MyIntPointer p; // expected-note 2{{pointer member declared here}}
+  MyLongPointerFromConversion p2; // expected-note {{pointer member declared here}}
+  DanglingGslPtrField(int i) : p(&i) {} // expected-warning {{initializing pointer member 'p' with the stack address of parameter 'i'}}
+  DanglingGslPtrField() : p2(MyLongOwnerWithConversion{}) {} // expected-warning {{initializing pointer member 'p2' to point to a temporary object whose lifetime is shorter than the lifetime of the constructed object}}
+  DanglingGslPtrField(double) : p(MyIntOwner{}) {} // expected-warning {{initializing pointer member 'p' to point to a temporary object whose lifetime is shorter than the lifetime of the constructed object}}
+};
+
+MyIntPointer danglingGslPtrFromLocal() {
+  int j;
+  return &j; // expected-warning {{address of stack memory associated with local variable 'j' returned}}
+}
+
+MyIntPointer returningLocalPointer() {
+  MyIntPointer localPointer;
+  return localPointer; // ok
+}
+
+MyIntPointer daglingGslPtrFromLocalOwner() {
+  MyIntOwner localOwner;
+  return localOwner; // expected-warning {{address of stack memory associated with local variable 'localOwner' returned}}
+}
+
+MyLongPointerFromConversion daglingGslPtrFromLocalOwnerConv() {
+  MyLongOwnerWithConversion localOwner;
+  return localOwner; // expected-warning {{address of stack memory associated with local variable 'localOwner' returned}}
+}
+
+MyIntPointer danglingGslPtrFromTemporary() {
+  return MyIntOwner{}; // expected-warning {{returning address of local temporary object}}
+}
+
+MyLongPointerFromConversion danglingGslPtrFromTemporaryConv() {
+  return MyLongOwnerWithConversion{}; // expected-warning {{returning address of local temporary object}}
+}
+
+int *noFalsePositive(MyIntOwner &o) {
+  MyIntPointer p = o;
+  return &*p; // ok
+}
+
+MyIntPointer global;
+MyLongPointerFromConversion global2;
+
+void initLocalGslPtrWithTempOwner() {
+  MyIntPointer p = MyIntOwner{}; // expected-warning {{object backing the pointer will be destroyed at the end of the full-expression}}
+  p = MyIntOwner{}; // TODO ?
+  global = MyIntOwner{}; // TODO ?
+  MyLongPointerFromConv

[PATCH] D64256: Teach some warnings to respect gsl::Pointer and gsl::Owner attributes

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

xazax.hun updated this revision to Diff 212205.
xazax.hun marked an inline comment as done.
xazax.hun added a comment.

- Actually move the code snippet in question.


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

https://reviews.llvm.org/D64256

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaInit.cpp
  clang/test/Sema/warn-lifetime-analysis-nocfg.cpp

Index: clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
===
--- /dev/null
+++ clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
@@ -0,0 +1,135 @@
+// RUN: %clang_cc1 -fsyntax-only -Wdangling -Wdangling-field -Wreturn-stack-address -verify %s
+struct [[gsl::Owner(int)]] MyIntOwner {
+  MyIntOwner();
+  int &operator*();
+  int *c_str() const;
+};
+
+struct [[gsl::Pointer(int)]] MyIntPointer {
+  MyIntPointer(int *p = nullptr);
+  // Conversion operator and constructor conversion will result in two
+  // different ASTs. The former is tested with another owner and 
+  // pointer type.
+  MyIntPointer(const MyIntOwner &);
+  int &operator*();
+  MyIntOwner toOwner();
+};
+
+struct [[gsl::Pointer(long)]] MyLongPointerFromConversion {
+  MyLongPointerFromConversion(long *p = nullptr);
+  long &operator*();
+};
+
+struct [[gsl::Owner(long)]] MyLongOwnerWithConversion {
+  MyLongOwnerWithConversion();
+  operator MyLongPointerFromConversion();
+  long &operator*();
+  MyIntPointer releaseAsMyPointer();
+  long *releaseAsRawPointer();
+};
+
+void danglingHeapObject() {
+  new MyLongPointerFromConversion(MyLongOwnerWithConversion{}); // expected-warning {{object backing the pointer will be destroyed at the end of the full-expression}}
+  new MyIntPointer(MyIntOwner{}); // expected-warning {{object backing the pointer will be destroyed at the end of the full-expression}}
+}
+
+void intentionalFalseNegative() {
+  int i;
+  MyIntPointer p{&i};
+  // In this case we do not have enough information in a statement local
+  // analysis to detect the problem.
+  new MyIntPointer(p);
+  new MyIntPointer(MyIntPointer{p});
+}
+
+MyIntPointer ownershipTransferToMyPointer() {
+  MyLongOwnerWithConversion t;
+  return t.releaseAsMyPointer(); // ok
+}
+
+long *ownershipTransferToRawPointer() {
+  MyLongOwnerWithConversion t;
+  return t.releaseAsRawPointer(); // ok
+}
+
+int *danglingRawPtrFromLocal() {
+  MyIntOwner t;
+  return t.c_str(); // TODO
+}
+
+int *danglingRawPtrFromTemp() {
+  MyIntPointer p;
+  return p.toOwner().c_str(); // TODO
+}
+
+struct Y {
+  int a[4];
+};
+
+void dangligGslPtrFromTemporary() {
+  MyIntPointer p = Y{}.a; // expected-warning {{temporary whose address is used as value of local variable 'p' will be destroyed at the end of the full-expression}}
+  (void)p;
+}
+
+struct DanglingGslPtrField {
+  MyIntPointer p; // expected-note 2{{pointer member declared here}}
+  MyLongPointerFromConversion p2; // expected-note {{pointer member declared here}}
+  DanglingGslPtrField(int i) : p(&i) {} // expected-warning {{initializing pointer member 'p' with the stack address of parameter 'i'}}
+  DanglingGslPtrField() : p2(MyLongOwnerWithConversion{}) {} // expected-warning {{initializing pointer member 'p2' to point to a temporary object whose lifetime is shorter than the lifetime of the constructed object}}
+  DanglingGslPtrField(double) : p(MyIntOwner{}) {} // expected-warning {{initializing pointer member 'p' to point to a temporary object whose lifetime is shorter than the lifetime of the constructed object}}
+};
+
+MyIntPointer danglingGslPtrFromLocal() {
+  int j;
+  return &j; // expected-warning {{address of stack memory associated with local variable 'j' returned}}
+}
+
+MyIntPointer returningLocalPointer() {
+  MyIntPointer localPointer;
+  return localPointer; // ok
+}
+
+MyIntPointer daglingGslPtrFromLocalOwner() {
+  MyIntOwner localOwner;
+  return localOwner; // expected-warning {{address of stack memory associated with local variable 'localOwner' returned}}
+}
+
+MyLongPointerFromConversion daglingGslPtrFromLocalOwnerConv() {
+  MyLongOwnerWithConversion localOwner;
+  return localOwner; // expected-warning {{address of stack memory associated with local variable 'localOwner' returned}}
+}
+
+MyIntPointer danglingGslPtrFromTemporary() {
+  return MyIntOwner{}; // expected-warning {{returning address of local temporary object}}
+}
+
+MyLongPointerFromConversion danglingGslPtrFromTemporaryConv() {
+  return MyLongOwnerWithConversion{}; // expected-warning {{returning address of local temporary object}}
+}
+
+int *noFalsePositive(MyIntOwner &o) {
+  MyIntPointer p = o;
+  return &*p; // ok
+}
+
+MyIntPointer global;
+MyLongPointerFromConversion global2;
+
+void initLocalGslPtrWithTempOwner() {
+  MyIntPointer p = MyIntOwner{}; // expected-warning {{object backing the pointer will be destroyed at the end of the full-expression}}
+  p = MyIntOwner{}; // TODO ?
+  global = MyIntOwner{}; // TODO ?
+  MyLongPointerFromConversion p2 = MyLongOwner

[PATCH] D64256: Teach some warnings to respect gsl::Pointer and gsl::Owner attributes

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

xazax.hun added inline comments.



Comment at: clang/lib/Sema/SemaInit.cpp:7095
   auto *MTE = dyn_cast(L);
+  if (IsGslPtrInitWithGslTempOwner) {
+Diag(DiagLoc, diag::warn_dangling_lifetime_pointer) << DiagRange;

gribozavr wrote:
> It is unclear to me why `if (IsGslPtrInitWithGslTempOwner)` is before `if 
> (!MTE)`, seems like that exclusion should apply to our case, too.
The reason was that I always skipped MTEs and was looking for the expr that 
actually initialized MTEs. Relying on MTEs, however, turned out to simplify the 
code a bit. Updating the diff, thanks!


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

https://reviews.llvm.org/D64256



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

[PATCH] D63082: [Diagnostics] Added support for -Wint-in-bool-context

[Dávid Bolvanský via Phabricator via cfe-commits]

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



Comment at: test/SemaCXX/warn-int-in-bool-context.cpp:96
+
+  if (f == apple || orange) // expected-warning {{enum constant in boolean 
context}}
+return a;

@aaron.ballman : In C (but not in C++ ugh?) mode we have: 'use of logical '||' 
with constant operand' here..

/home/xbolva00/LLVM/llvm/tools/clang/test/SemaCXX/warn-int-in-bool-context.cpp:106:18:
 warning: use of logical '||' with constant operand
  if (f == apple || orange) // expected-warning {{enum constant in boolean 
context}}
 ^  ~~
/home/xbolva00/LLVM/llvm/tools/clang/test/SemaCXX/warn-int-in-bool-context.cpp:106:18:
 note: use '|' for a bitwise operation
  if (f == apple || orange) // expected-warning {{enum constant in boolean 
context}}
 ^~
 |
/home/xbolva00/LLVM/llvm/tools/clang/test/SemaCXX/warn-int-in-bool-context.cpp:106:21:
 warning: enum constant in boolean context
  if (f == apple || orange) // expected-warning {{enum constant in boolean 
context}}






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

https://reviews.llvm.org/D63082



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

[PATCH] D65120: More warnings regarding gsl::Pointer and gsl::Owner attributes

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

xazax.hun updated this revision to Diff 212206.
xazax.hun marked 3 inline comments as done.
xazax.hun edited the summary of this revision.
xazax.hun added a comment.

- Add new line to end of file.
- Rebase, calls no longer need special handling.


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

https://reviews.llvm.org/D65120

Files:
  clang/lib/Sema/SemaInit.cpp
  clang/test/Analysis/inner-pointer.cpp
  clang/test/Sema/warn-lifetime-analysis-nocfg.cpp

Index: clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
===
--- clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
+++ clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
@@ -2,7 +2,6 @@
 struct [[gsl::Owner(int)]] MyIntOwner {
   MyIntOwner();
   int &operator*();
-  int *c_str() const;
 };
 
 struct [[gsl::Pointer(int)]] MyIntPointer {
@@ -52,16 +51,6 @@
   return t.releaseAsRawPointer(); // ok
 }
 
-int *danglingRawPtrFromLocal() {
-  MyIntOwner t;
-  return t.c_str(); // TODO
-}
-
-int *danglingRawPtrFromTemp() {
-  MyIntPointer p;
-  return p.toOwner().c_str(); // TODO
-}
-
 struct Y {
   int a[4];
 };
@@ -103,6 +92,12 @@
   return MyIntOwner{}; // expected-warning {{returning address of local temporary object}}
 }
 
+MyIntOwner makeTempOwner();
+
+MyIntPointer danglingGslPtrFromTemporary2() {
+  return makeTempOwner(); // expected-warning {{returning address of local temporary object}}
+}
+
 MyLongPointerFromConversion danglingGslPtrFromTemporaryConv() {
   return MyLongOwnerWithConversion{}; // expected-warning {{returning address of local temporary object}}
 }
@@ -124,12 +119,52 @@
   global2 = MyLongOwnerWithConversion{}; // TODO ?
 }
 
-struct IntVector {
-  int *begin();
-  int *end();
+namespace std {
+template 
+struct basic_iterator {};
+
+template 
+struct vector {
+  typedef basic_iterator iterator;
+  iterator begin();
+  T *data();
+};
+
+template
+struct basic_string {
+  const T *c_str() const;
+};
+
+template
+struct unique_ptr {
+  T *get() const;
 };
+}
 
 void modelIterators() {
-  int *it = IntVector{}.begin(); // TODO ?
+  std::vector::iterator it = std::vector().begin(); // expected-warning {{object backing the pointer will be destroyed at the end of the full-expression}}
   (void)it;
 }
+
+std::vector::iterator modelIteratorReturn() {
+  return std::vector().begin(); // expected-warning {{returning address of local temporary object}}
+}
+
+const char *danglingRawPtrFromLocal() {
+  std::basic_string s;
+  return s.c_str(); // expected-warning {{address of stack memory associated with local variable 's' returned}}
+}
+
+const char *danglingRawPtrFromTemp() {
+  return std::basic_string().c_str(); // expected-warning {{returning address of local temporary object}}
+}
+
+std::unique_ptr getUniquePtr();
+
+int *danglingUniquePtrFromTemp() {
+  return getUniquePtr().get(); // expected-warning {{returning address of local temporary object}}
+}
+
+int *danglingUniquePtrFromTemp2() {
+  return std::unique_ptr().get(); // expected-warning {{returning address of local temporary object}}
+}
Index: clang/test/Analysis/inner-pointer.cpp
===
--- clang/test/Analysis/inner-pointer.cpp
+++ clang/test/Analysis/inner-pointer.cpp
@@ -1,5 +1,6 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=cplusplus.InnerPointer \
-// RUN:   %s -analyzer-output=text -verify
+// RUN:   %s -analyzer-output=text -Wno-dangling -Wno-dangling-field \
+// RUN:   -Wno-return-stack-address -verify
 
 #include "Inputs/system-header-simulator-cxx.h"
 namespace std {
Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -6559,6 +6559,22 @@
   return false;
 }
 
+static bool shouldTrackImplicitObjectArg(const CXXMethodDecl *Callee) {
+  if (auto *Conv = dyn_cast_or_null(Callee))
+if (isRecordWithAttr(Conv->getConversionType()))
+  return true;
+  if (!Callee->getParent()->isInStdNamespace() || !Callee->getIdentifier())
+return false;
+  if (!isRecordWithAttr(Callee->getReturnType()) &&
+  !Callee->getReturnType()->isPointerType())
+return false;
+  return llvm::StringSwitch(Callee->getName())
+  .Cases("begin", "rbegin", "cbegin", "crbegin", true)
+  .Cases("end", "rend", "cend", "crend", true)
+  .Cases("c_str", "data", "get", true)
+  .Default(false);
+}
+
 static void handleGslAnnotatedTypes(IndirectLocalPath &Path, Expr *Call,
 LocalVisitor Visit) {
   auto VisitPointerArg = [&](const Decl *D, Expr *Arg) {
@@ -6572,15 +6588,14 @@
   };
 
   if (auto *MCE = dyn_cast(Call)) {
-const FunctionDecl *Callee = MCE->getDirectCallee();
-if (auto *Conv = dyn_cast_or_null(Callee))
-  if (isRecordWithAttr(Conv->getConversionType()))
-VisitPointerArg(Callee, MCE->getImplicitObjectArgument());
+const auto *MD = cast_or_null(MCE->getDirectCa

[PATCH] D65120: More warnings regarding gsl::Pointer and gsl::Owner attributes

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

xazax.hun added inline comments.



Comment at: clang/lib/Sema/SemaInit.cpp:6563
 
+static bool shouldTrackImplicitObjectArg(const CXXMethodDecl *Callee) {
+  if (auto *Conv = dyn_cast_or_null(Callee))

gribozavr wrote:
> This looks like an ad-hoc rule. Is there a way to express it with attributes 
> instead?
It is indeed an ad-hoc rule. The point is that we do know that `std` methods 
behave this way and function attributes will be able to express the same in the 
future. But we are still experimenting with what is the best spelling, 
representation etc for those function attributes. So given the value of these 
checks I do not want to wait until function annotations are upstreamed since we 
can achieve the same with relatively little code. I run these checks on ~300 
open source projects and not a single false positive was found in the latest 
run. 



Comment at: clang/lib/Sema/SemaInit.cpp:6572
+return false;
+  return llvm::StringSwitch(Callee->getName())
+  .Cases("begin", "rbegin", "cbegin", "crbegin", true)

gribozavr wrote:
> Should we also check that `Callee->getParent` is an owner?
> 
> Otherwise the check would complain about `begin()` on, for example, a 
> `std::string_view`.
This is intentional. We only warn if the initialization chain can be tracked 
back to a temporary (or local in some cases) owner. 
So we warn for the following code:
```
const char *trackThroughMultiplePointer() {
  return std::basic_string_view(std::basic_string()).begin(); // 
expected-warning {{returning address of local temporary object}}
}
```


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

https://reviews.llvm.org/D65120



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

[PATCH] D65127: Even more warnings utilizing gsl::Owner/gsl::Pointer annotations

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

xazax.hun marked an inline comment as done.
xazax.hun added inline comments.



Comment at: clang/lib/Sema/SemaInit.cpp:6581
+if (!Callee->getIdentifier()) {
+  auto OO = Callee->getOverloadedOperator();
+  return OO == OverloadedOperatorKind::OO_Subscript ||

If we want to relax the warnings to give more results we could extend the 
checking of these overloaded operators for annotated types. But this would 
imply that the user need to have the expected semantics for those types and can 
only suppress false positives by removing some gsl:::owner/poinnter annotations.


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

https://reviews.llvm.org/D65127



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

[PATCH] D65127: Even more warnings utilizing gsl::Owner/gsl::Pointer annotations

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

xazax.hun updated this revision to Diff 212207.
xazax.hun edited the summary of this revision.
xazax.hun added a comment.

- Rebase, add the results of testing on real world projects to the description.


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

https://reviews.llvm.org/D65127

Files:
  clang/lib/Sema/SemaInit.cpp
  clang/test/Sema/warn-lifetime-analysis-nocfg.cpp

Index: clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
===
--- clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
+++ clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
@@ -121,24 +121,47 @@
 
 namespace std {
 template 
-struct basic_iterator {};
+struct basic_iterator {
+  basic_iterator operator++();
+  T& operator*();
+};
+
+template
+bool operator!=(basic_iterator, basic_iterator);
 
 template 
 struct vector {
   typedef basic_iterator iterator;
   iterator begin();
+  iterator end();
   T *data();
+  T &at(int n);
+};
+
+template
+struct basic_string_view {
+  basic_string_view(const T *);
+  const T *begin() const;
 };
 
 template
 struct basic_string {
   const T *c_str() const;
+  operator basic_string_view () const;
 };
 
+
 template
 struct unique_ptr {
   T *get() const;
 };
+
+template
+struct optional {
+  optional();
+  optional(const T&);
+  T &operator*();
+};
 }
 
 void modelIterators() {
@@ -168,3 +191,36 @@
 int *danglingUniquePtrFromTemp2() {
   return std::unique_ptr().get(); // expected-warning {{returning address of local temporary object}}
 }
+
+void danglingReferenceFromTempOwner() {
+  int &r = *std::optional(); // expected-warning {{object backing the pointer will be destroyed at the end of the full-expression}}
+  int &r2 = *std::optional(5); // expected-warning {{object backing the pointer will be destroyed at the end of the full-expression}}
+  int &r3 = std::vector().at(3); // expected-warning {{object backing the pointer will be destroyed at the end of the full-expression}}
+}
+
+std::vector getTempVec();
+std::optional> getTempOptVec();
+
+void testLoops() {
+  for (auto i : getTempVec()) // ok
+;
+  for (auto i : *getTempOptVec()) // expected-warning {{object backing the pointer will be destroyed at the end of the full-expression}}
+;
+}
+
+int &usedToBeFalsePositive(std::vector &v) {
+  std::vector::iterator it = v.begin();
+  int& value = *it;
+  return value; // ok
+}
+
+int &doNotFollowReferencesForLocalOwner() {
+  std::unique_ptr localOwner;
+  int &p = *localOwner.get();
+  // In real world code localOwner is usually moved here.
+  return p; // ok
+}
+
+const char *trackThroughMultiplePointer() {
+  return std::basic_string_view(std::basic_string()).begin(); // expected-warning {{returning address of local temporary object}}
+}
Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -6563,16 +6563,30 @@
   if (auto *Conv = dyn_cast_or_null(Callee))
 if (isRecordWithAttr(Conv->getConversionType()))
   return true;
-  if (!Callee->getParent()->isInStdNamespace() || !Callee->getIdentifier())
+  if (!Callee->getParent()->isInStdNamespace())
 return false;
-  if (!isRecordWithAttr(Callee->getReturnType()) &&
-  !Callee->getReturnType()->isPointerType())
-return false;
-  return llvm::StringSwitch(Callee->getName())
-  .Cases("begin", "rbegin", "cbegin", "crbegin", true)
-  .Cases("end", "rend", "cend", "crend", true)
-  .Cases("c_str", "data", "get", true)
-  .Default(false);
+  if (Callee->getReturnType()->isPointerType() ||
+  isRecordWithAttr(Callee->getReturnType())) {
+if (!Callee->getIdentifier())
+  return false;
+return llvm::StringSwitch(Callee->getName())
+.Cases("begin", "rbegin", "cbegin", "crbegin", true)
+.Cases("end", "rend", "cend", "crend", true)
+.Cases("c_str", "data", "get", true)
+// Map and set types.
+.Cases("find", "equal_range", "lower_bound", "upper_bound", true)
+.Default(false);
+  } else if (Callee->getReturnType()->isReferenceType()) {
+if (!Callee->getIdentifier()) {
+  auto OO = Callee->getOverloadedOperator();
+  return OO == OverloadedOperatorKind::OO_Subscript ||
+ OO == OverloadedOperatorKind::OO_Star;
+}
+return llvm::StringSwitch(Callee->getName())
+.Cases("front", "back", "at", true)
+.Default(false);
+  }
+  return false;
 }
 
 static void handleGslAnnotatedTypes(IndirectLocalPath &Path, Expr *Call,
@@ -6592,6 +6606,12 @@
 if (MD && shouldTrackImplicitObjectArg(MD))
   VisitPointerArg(MD, MCE->getImplicitObjectArgument());
 return;
+  } else if (auto *OCE = dyn_cast(Call)) {
+FunctionDecl *Callee = OCE->getDirectCallee();
+if (Callee->isCXXInstanceMember() &&
+shouldTrackImplicitObjectArg(cast(Callee)))
+  VisitPointerArg(Callee, OCE->getArg(0));
+return;
   }
 
   if (auto *CCE = dyn

[PATCH] D64256: Teach some warnings to respect gsl::Pointer and gsl::Owner attributes

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

xazax.hun added a comment.

Thanks for the review!
Since I did some refactoring I will wait for an additional accept before 
committing.


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

https://reviews.llvm.org/D64256



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

r367249 - Fix taint-generic.c on Windows, handle case in OS error

[Reid Kleckner via cfe-commits]

Author: rnk
Date: Mon Jul 29 11:48:50 2019
New Revision: 367249

URL: http://llvm.org/viewvc/llvm-project?rev=367249&view=rev
Log:
Fix taint-generic.c on Windows, handle case in OS error

Modified:
cfe/trunk/test/Analysis/taint-generic.c

Modified: cfe/trunk/test/Analysis/taint-generic.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/taint-generic.c?rev=367249&r1=367248&r2=367249&view=diff
==
--- cfe/trunk/test/Analysis/taint-generic.c (original)
+++ cfe/trunk/test/Analysis/taint-generic.c Mon Jul 29 11:48:50 2019
@@ -32,7 +32,7 @@
 
 // CHECK-ILL-FORMED: (frontend): invalid input for checker option
 // CHECK-ILL-FORMED-SAME:
'alpha.security.taint.TaintPropagation:Config',
-// CHECK-ILL-FORMED-SAME:that expects a valid yaml file: Invalid 
argument
+// CHECK-ILL-FORMED-SAME:that expects a valid yaml file: 
{{[Ii]}}nvalid argument
 
 // RUN: not %clang_analyze_cc1 -verify %s \
 // RUN:   -analyzer-checker=alpha.security.taint \


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

[PATCH] D64475: [clangd] Duplicate lines of semantic highlightings sent removed.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Mostly final NITs from me, but there is an important one about removing the 
`erase` call on `didOpen`.




Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:467
+std::lock_guard Lock(HighlightingsMutex);
+FileToHighlightings.erase(File);
+  }

Now that the patch landed, this is obsolete. Could you remove?



Comment at: clang-tools-extra/clangd/ClangdServer.cpp:80
+FileID MainFileID = SM.getMainFileID();
+unsigned int FileSize = SM.getFileEntryForID(MainFileID)->getSize();
+int NLines = AST.getSourceManager().getLineNumber(MainFileID, FileSize);

NIT: use `unsigned` instead of `unsigned int`



Comment at: clang-tools-extra/clangd/ClangdServer.h:56
+  virtual void onHighlightingsReady(
+  PathRef File, std::vector Highlightings, int NLines) 
{}
 };

NIT: could you document `NLines`



Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:294
+  // ArrayRefs to the current line in the highlights.
+  ArrayRef NewLine(New.begin(),
+  /*length*/0UL);

Could we try to find a better name for this? Having `Line` and `NextLine()` 
which represent line numbers and `NewLine` which represents highlightings, 
creates some confusion.



Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:310
+  // highlightings beyond the end of the file. That might crash the client.
+  for (int Line = 0; Line != std::numeric_limits::max() && Line < NLines;
+   Line = NextLine()) {

`Line != intmax` is redundant (NLines is <= intmax by definition).
Maybe remove it altogether to simplify the loop condition?



Comment at: clang-tools-extra/clangd/SemanticHighlighting.h:80
+diffHighlightings(ArrayRef New,
+  ArrayRef Old, int NLines);
 

Could you document what `NLines` is? Specifically, whether it's the number of 
lines for `New` or `Old`.




Comment at: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp:60
+
+void checkDiffedHighlights(llvm::StringRef OldCode, llvm::StringRef NewCode) {
+  Annotations OldTest(OldCode);

NIT: add documentation on how this should be used
Most importantly, the fact that we need to put `^` on all changed lines should 
be mentioned.



Comment at: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp:66
+
+  std::map> ExpectedLines;
+  for (const Position &Point : NewTest.points()) {

NIT: use `llvm::DenseMap`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64475



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

Re: r367193 - Buildbot fix for r367190

[Galina Kistanova via cfe-commits]

Hello Gabor ,

It looks like this commit broke tests on couple builders:

http://lab.llvm.org:8011/builders/llvm-clang-x86_64-expensive-checks-win/builds/18867/steps/test-check-all/logs/stdio
http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast
. . .

Failing Tests (1):
Clang :: Analysis/taint-generic.c


Please have a look?
These builders were already red and did not send any notifications.

Thanks

Galina

On Sun, Jul 28, 2019 at 8:00 AM Gabor Borsik via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: boga95
> Date: Sun Jul 28 07:57:41 2019
> New Revision: 367193
>
> URL: http://llvm.org/viewvc/llvm-project?rev=367193&view=rev
> Log:
> Buildbot fix for r367190
>
> Modified:
> cfe/trunk/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
>
> Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp?rev=367193&r1=367192&r2=367193&view=diff
>
> ==
> --- cfe/trunk/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
> (original)
> +++ cfe/trunk/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp Sun Jul
> 28 07:57:41 2019
> @@ -811,7 +811,7 @@ void ento::registerGenericTaintChecker(C
>llvm::Optional Config =
>getConfiguration(Mgr, Checker, Option, ConfigFile);
>if (Config)
> -Checker->parseConfiguration(Mgr, Option,
> std::move(Config).getValue());
> +Checker->parseConfiguration(Mgr, Option,
> std::move(Config.getValue()));
>  }
>
>  bool ento::shouldRegisterGenericTaintChecker(const LangOptions &LO) {
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r367193 - Buildbot fix for r367190

[Aaron Ballman via cfe-commits]

On Mon, Jul 29, 2019 at 3:03 PM Galina Kistanova via cfe-commits
 wrote:
>
> Hello Gabor ,
>
> It looks like this commit broke tests on couple builders:
>
> http://lab.llvm.org:8011/builders/llvm-clang-x86_64-expensive-checks-win/builds/18867/steps/test-check-all/logs/stdio
> http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast
> . . .
> 
> Failing Tests (1):
> Clang :: Analysis/taint-generic.c
>
>
> Please have a look?
> These builders were already red and did not send any notifications.

I think Reid fixed this in r367249
(http://llvm.org/viewvc/llvm-project?view=revision&revision=367249).

~Aaron

>
> Thanks
>
> Galina
>
> On Sun, Jul 28, 2019 at 8:00 AM Gabor Borsik via cfe-commits 
>  wrote:
>>
>> Author: boga95
>> Date: Sun Jul 28 07:57:41 2019
>> New Revision: 367193
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=367193&view=rev
>> Log:
>> Buildbot fix for r367190
>>
>> Modified:
>> cfe/trunk/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
>>
>> Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
>> URL: 
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp?rev=367193&r1=367192&r2=367193&view=diff
>> ==
>> --- cfe/trunk/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp (original)
>> +++ cfe/trunk/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp Sun Jul 28 
>> 07:57:41 2019
>> @@ -811,7 +811,7 @@ void ento::registerGenericTaintChecker(C
>>llvm::Optional Config =
>>getConfiguration(Mgr, Checker, Option, ConfigFile);
>>if (Config)
>> -Checker->parseConfiguration(Mgr, Option, std::move(Config).getValue());
>> +Checker->parseConfiguration(Mgr, Option, std::move(Config.getValue()));
>>  }
>>
>>  bool ento::shouldRegisterGenericTaintChecker(const LangOptions &LO) {
>>
>>
>> ___
>> cfe-commits mailing list
>> cfe-commits@lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64089: [Driver] Introduce -stdlib++-isystem

[Louis Dionne via Phabricator via cfe-commits]

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

This looks fine to me, but I'd like to have someone else familiar with the 
Driver/Frontend design sign off too. The implementation is trivial but I'd like 
support on the fact that this is a good idea (which I think it is).




Comment at: clang/lib/Driver/ToolChains/Clang.cpp:1307
+  HasStdlibxxIsystem ? TC.AddClangCXXStdlibIsystemArgs(Args, CmdArgs)
+ : TC.AddClangCXXStdlibIncludeArgs(Args, CmdArgs);
+});

smeenai wrote:
> ldionne wrote:
> > So, before, we would populate `-stdlib=XXX` here unconditionally for `cc1`.
> > 
> > After the patch, we do _not_ populate `-stdlib=XXX` and instead we pass 
> > `-internal-isystem` to `cc1` with the contents of `-stdlib++-isystem` 
> > whenever that option is provided, otherwise we fall back to the previous 
> > behaviour.
> > 
> > Did I get that right? If so, could we investigate getting rid of 
> > `AddClangCXXStdlibIncludeArgs` altogether? I don't think it is needed 
> > anymore since my refactor of the driver for Darwin.
> Sorry, I'd missed this. You're right; we'll continue passing `-stdlib=XXX` in 
> the old case (i.e. when there's no `-stdlib++-isystem` argument), but not if 
> it's there.
> 
> When you say getting rid of `AddClangCXXStdlibIncludeArgs`, that's the one in 
> the driver (in the toolchains) right? I thought your Darwin refactor involved 
> moving C++ header search logic from the frontend into the driver ... did you 
> mean getting rid of the C++ header search path logic in the frontend?
Sorry, I should have been more clear. I meant specifically getting rid of this: 
https://github.com/llvm-project/clang/blob/master/lib/Driver/ToolChain.cpp#L832

In that snippet, we pass `-stdlib=XXX` down to the frontend. My understanding 
is that this was only necessary because the frontend on Darwin needed to know 
which stdlib was in use, in order to setup the header search paths correctly. 
Now that it's not needed anymore, I think we could get rid of that hack 
altogether.

However, looking at it further, changing that is way beyond the scope of your 
PR. I had not realized that other non-Darwin toolchain provided implementations 
of `AddClangCXXStdlibIncludeArgs` (which is `virtual` and meant to be 
overridden).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64089



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

[PATCH] D65403: [COFF, ARM64] Reorder handling of aarch64 MSVC builtins

[Eli Friedman via Phabricator via cfe-commits]

efriedma added inline comments.



Comment at: lib/CodeGen/CGBuiltin.cpp:8182
   switch (BuiltinID) {
   default: return nullptr;
   case NEON::BI__builtin_neon_vbsl_v:

I'm a little concerned about the overall code structure here; even if moving 
the code for the MSVC builtins is enough to fix those builtins specifically, is 
it actually impossible to hit this "default"?  If it is, can we convert it to 
an "unreachable"?


Repository:
  rC Clang

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

https://reviews.llvm.org/D65403



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

[PATCH] D59474: [OpenMP 5.0] Codegen support for user-defined mappers

[Lingda Li via Phabricator via cfe-commits]

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

Thanks Alexey! Could you look into the runtime patch D60972 
 then?




Comment at: test/OpenMP/declare_mapper_codegen.cpp:44-48
+// CK0-DAG: store i8* %0, i8** [[HANDLEADDR:%[^,]+]]
+// CK0-DAG: store i8* %1, i8** [[BPTRADDR:%[^,]+]]
+// CK0-DAG: store i8* %2, i8** [[VPTRADDR:%[^,]+]]
+// CK0-DAG: store i64 %3, i{{64|32}}* [[SIZEADDR:%[^,]+]]
+// CK0-DAG: store i64 %4, i64* [[TYPEADDR:%[^,]+]]

ABataev wrote:
> I would not rely on the predetermined indices here, better to use some kind 
> of patterns here just like in other places.
Could you give an example about what you suggest? For instance, some other 
tests I should look into.


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

https://reviews.llvm.org/D59474



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

Re: [clang-tools-extra] r367137 - [clang-format] Fix style of css file paths

[Galina Kistanova via cfe-commits]

Hello Diego,

This commit added broken test to one of our win builders:

http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast/builds/27202

. . .
Failing Tests (4):
Extra Tools Unit Tests ::
clang-doc/./ClangDocTests.exe/HTMLGeneratorTest.emitRecordHTML
. . .

Please have a look?
These builder was already red and did not send any notifications.

Thanks

Galina

On Fri, Jul 26, 2019 at 11:02 AM Diego Astiazaran via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: diegoastiazaran
> Date: Fri Jul 26 11:02:42 2019
> New Revision: 367137
>
> URL: http://llvm.org/viewvc/llvm-project?rev=367137&view=rev
> Log:
> [clang-format] Fix style of css file paths
>
> CSS files included in HTML should have a path in posix style, it should
> not be different for Windows.
>
> Differential Revision: https://reviews.llvm.org/D65309
>
> Modified:
> clang-tools-extra/trunk/clang-doc/HTMLGenerator.cpp
> clang-tools-extra/trunk/unittests/clang-doc/HTMLGeneratorTest.cpp
>
> Modified: clang-tools-extra/trunk/clang-doc/HTMLGenerator.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-doc/HTMLGenerator.cpp?rev=367137&r1=367136&r2=367137&view=diff
>
> ==
> --- clang-tools-extra/trunk/clang-doc/HTMLGenerator.cpp (original)
> +++ clang-tools-extra/trunk/clang-doc/HTMLGenerator.cpp Fri Jul 26
> 11:02:42 2019
> @@ -231,6 +231,8 @@ genStylesheetsHTML(StringRef InfoPath, c
>  SmallString<128> StylesheetPath = computeRelativePath("", InfoPath);
>  llvm::sys::path::append(StylesheetPath,
>  llvm::sys::path::filename(FilePath));
> +// Paths in HTML must be in posix-style
> +llvm::sys::path::native(StylesheetPath,
> llvm::sys::path::Style::posix);
>  LinkNode->Attributes.try_emplace("href", StylesheetPath);
>  Out.emplace_back(std::move(LinkNode));
>}
>
> Modified: clang-tools-extra/trunk/unittests/clang-doc/HTMLGeneratorTest.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clang-doc/HTMLGeneratorTest.cpp?rev=367137&r1=367136&r2=367137&view=diff
>
> ==
> --- clang-tools-extra/trunk/unittests/clang-doc/HTMLGeneratorTest.cpp
> (original)
> +++ clang-tools-extra/trunk/unittests/clang-doc/HTMLGeneratorTest.cpp Fri
> Jul 26 11:02:42 2019
> @@ -110,34 +110,23 @@ TEST(HTMLGeneratorTest, emitRecordHTML)
>ClangDocContext CDCtx = getClangDocContext();
>auto Err = G->generateDocForInfo(&I, Actual, CDCtx);
>assert(!Err);
> -  SmallString<16> PathToF;
> -  llvm::sys::path::native("../../../path/to/F.html", PathToF);
> -  SmallString<16> PathToInt;
> -  llvm::sys::path::native("../int.html", PathToInt);
> -  SmallString<16> PathToSylesheet;
> -  llvm::sys::path::native("../../../clang-doc-default-stylesheet.css",
> -  PathToSylesheet);
>std::string Expected = R"raw(
>  
>  class r
> - - std::string(PathToSylesheet.str()) +
> - R"raw("/>
> +
>  
>class r
>Defined at line 10 of test.cpp
>
>  Inherits from
> - - R"raw(">F
> +F
>  , G
>
>Members
>
>  
>private
> -   - R"raw(">int
> +  int
> X
>  
>
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63174: [clang][NewPM] Add RUNS for tests that produce slightly different IR under new PM

[Leonard Chan via Phabricator via cfe-commits]

leonardchan abandoned this revision.
leonardchan added a comment.

Replaced with D65110 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63174



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

[PATCH] D65403: [COFF, ARM64] Reorder handling of aarch64 MSVC builtins

[David Major via Phabricator via cfe-commits]

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



Comment at: lib/CodeGen/CGBuiltin.cpp:8182
   switch (BuiltinID) {
   default: return nullptr;
   case NEON::BI__builtin_neon_vbsl_v:

efriedma wrote:
> I'm a little concerned about the overall code structure here; even if moving 
> the code for the MSVC builtins is enough to fix those builtins specifically, 
> is it actually impossible to hit this "default"?  If it is, can we convert it 
> to an "unreachable"?
I'm not sure if this question was directed to me... this was a drive-by patch 
from my end so I'm not familiar with what other types of builtins there might 
be.

I should probably mention that I'm hoping to get a fix merged to 9.0 in order 
to unblock Firefox. Unless someone can tell me that the unreachable is 
definitely safe, I'd worry about adding instability into the release branch. 
Perhaps the unreachable could be done in a separate commit only on 10.0 trunk 
where the tolerance for surprises is generally better.



Repository:
  rC Clang

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

https://reviews.llvm.org/D65403



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

Re: r367193 - Buildbot fix for r367190

[Gabor Borsik via cfe-commits]

Looks good to me. Thanks for the fix.

Aaron Ballman  ezt írta (időpont: 2019. júl. 29.,
H, 21:06):

> On Mon, Jul 29, 2019 at 3:03 PM Galina Kistanova via cfe-commits
>  wrote:
> >
> > Hello Gabor ,
> >
> > It looks like this commit broke tests on couple builders:
> >
> >
> http://lab.llvm.org:8011/builders/llvm-clang-x86_64-expensive-checks-win/builds/18867/steps/test-check-all/logs/stdio
> >
> http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast
> > . . .
> > 
> > Failing Tests (1):
> > Clang :: Analysis/taint-generic.c
> >
> >
> > Please have a look?
> > These builders were already red and did not send any notifications.
>
> I think Reid fixed this in r367249
> (http://llvm.org/viewvc/llvm-project?view=revision&revision=367249).
>
> ~Aaron
>
> >
> > Thanks
> >
> > Galina
> >
> > On Sun, Jul 28, 2019 at 8:00 AM Gabor Borsik via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
> >>
> >> Author: boga95
> >> Date: Sun Jul 28 07:57:41 2019
> >> New Revision: 367193
> >>
> >> URL: http://llvm.org/viewvc/llvm-project?rev=367193&view=rev
> >> Log:
> >> Buildbot fix for r367190
> >>
> >> Modified:
> >> cfe/trunk/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
> >>
> >> Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
> >> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp?rev=367193&r1=367192&r2=367193&view=diff
> >>
> ==
> >> --- cfe/trunk/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
> (original)
> >> +++ cfe/trunk/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp Sun
> Jul 28 07:57:41 2019
> >> @@ -811,7 +811,7 @@ void ento::registerGenericTaintChecker(C
> >>llvm::Optional Config =
> >>getConfiguration(Mgr, Checker, Option, ConfigFile);
> >>if (Config)
> >> -Checker->parseConfiguration(Mgr, Option,
> std::move(Config).getValue());
> >> +Checker->parseConfiguration(Mgr, Option,
> std::move(Config.getValue()));
> >>  }
> >>
> >>  bool ento::shouldRegisterGenericTaintChecker(const LangOptions &LO) {
> >>
> >>
> >> ___
> >> cfe-commits mailing list
> >> cfe-commits@lists.llvm.org
> >> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
> >
> > ___
> > cfe-commits mailing list
> > cfe-commits@lists.llvm.org
> > https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65403: [COFF, ARM64] Reorder handling of aarch64 MSVC builtins

[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




Comment at: lib/CodeGen/CGBuiltin.cpp:8182
   switch (BuiltinID) {
   default: return nullptr;
   case NEON::BI__builtin_neon_vbsl_v:

dmajor wrote:
> efriedma wrote:
> > I'm a little concerned about the overall code structure here; even if 
> > moving the code for the MSVC builtins is enough to fix those builtins 
> > specifically, is it actually impossible to hit this "default"?  If it is, 
> > can we convert it to an "unreachable"?
> I'm not sure if this question was directed to me... this was a drive-by patch 
> from my end so I'm not familiar with what other types of builtins there might 
> be.
> 
> I should probably mention that I'm hoping to get a fix merged to 9.0 in order 
> to unblock Firefox. Unless someone can tell me that the unreachable is 
> definitely safe, I'd worry about adding instability into the release branch. 
> Perhaps the unreachable could be done in a separate commit only on 10.0 trunk 
> where the tolerance for surprises is generally better.
> 
It should be impossible to reach this normally, I think; any target-specific 
builtin which codegen supports should be handled earlier, and target-specific 
builtins only exist on the relevant target.  The issue would just be a weird 
crash if you add a new builtin Builtins.def without code generation support, 
instead of a nicer "unsupported" error.

But I'm okay taking this as-is without trying to refactor the code to handle 
that gracefully, if we need this for 9.0.


Repository:
  rC Clang

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

https://reviews.llvm.org/D65403



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

[PATCH] D49754: Add -m(no-)spe, and e500 CPU definitions and support to clang

[Justin Hibbits via Phabricator via cfe-commits]

jhibbits requested review of this revision.
jhibbits added a comment.

Should've marked it as need review earlier.


Repository:
  rC Clang

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

https://reviews.llvm.org/D49754



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

[PATCH] D63048: Update __VERSION__ to remove the hardcoded 4.2.1 version

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 added a comment.

Some ICC behaviour..

icc -dumpversion
19.0.3.199

printf("%d %d %s", __GNUC__, __GNUC_MINOR__, __VERSION__);
./a.out 
7 4 Intel(R) C++ gcc 7.4 mode

Should we bump __GNUC__, __GNUC_MINOR__ too?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D63048



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

r367253 - [driver][test] Update as-options.s to not write to a readonly tree

[Jordan Rupprecht via cfe-commits]

Author: rupprecht
Date: Mon Jul 29 12:57:31 2019
New Revision: 367253

URL: http://llvm.org/viewvc/llvm-project?rev=367253&view=rev
Log:
[driver][test] Update as-options.s to not write to a readonly tree

The as-options.s test writes to the build tree as of r367165. Some build 
systems configure this to be readonly, so this fails. Explicitly write to the 
output tree using `%t` to avoid this.

Modified:
cfe/trunk/test/Driver/as-options.s

Modified: cfe/trunk/test/Driver/as-options.s
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/as-options.s?rev=367253&r1=367252&r2=367253&view=diff
==
--- cfe/trunk/test/Driver/as-options.s (original)
+++ cfe/trunk/test/Driver/as-options.s Mon Jul 29 12:57:31 2019
@@ -74,12 +74,13 @@
 // -Wa flags shouldn't cause warnings without an assembler stage with
 // -fno-integrated-as either.
 // RUN: %clang -Wa,-mno-warn-deprecated -fno-integrated-as -x c++ %s -S 2>&1 \
+// RUN:   -o %t.o \
 // RUN:   | FileCheck --check-prefix=NOWARN --allow-empty %s
 
 // But -m flags for the integrated assembler _should_ warn if the integrated
 // assembler is not in use.
-// RUN: %clang -mrelax-all -fintegrated-as -x c++ %s -S 2>&1 \
+// RUN: %clang -mrelax-all -fintegrated-as -x c++ %s -S -o %t.o 2>&1 \
 // RUN:   | FileCheck --check-prefix=NOWARN --allow-empty %s
-// RUN: %clang -mrelax-all -fno-integrated-as -x c++ %s -S 2>&1 \
+// RUN: %clang -mrelax-all -fno-integrated-as -x c++ %s -S -o %t.o 2>&1 \
 // RUN:   | FileCheck --check-prefix=WARN --allow-empty %s
 // WARN: unused


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

[PATCH] D59474: [OpenMP 5.0] Codegen support for user-defined mappers

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: test/OpenMP/declare_mapper_codegen.cpp:44-48
+// CK0-DAG: store i8* %0, i8** [[HANDLEADDR:%[^,]+]]
+// CK0-DAG: store i8* %1, i8** [[BPTRADDR:%[^,]+]]
+// CK0-DAG: store i8* %2, i8** [[VPTRADDR:%[^,]+]]
+// CK0-DAG: store i64 %3, i{{64|32}}* [[SIZEADDR:%[^,]+]]
+// CK0-DAG: store i64 %4, i64* [[TYPEADDR:%[^,]+]]

lildmh wrote:
> ABataev wrote:
> > I would not rely on the predetermined indices here, better to use some kind 
> > of patterns here just like in other places.
> Could you give an example about what you suggest? For instance, some other 
> tests I should look into.
Just like in this test when you're using vars.


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

https://reviews.llvm.org/D59474



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

r367254 - When determining whether a lambda-expression is implicitly constexpr,

[Richard Smith via cfe-commits]

Author: rsmith
Date: Mon Jul 29 12:59:45 2019
New Revision: 367254

URL: http://llvm.org/viewvc/llvm-project?rev=367254&view=rev
Log:
When determining whether a lambda-expression is implicitly constexpr,
check the formal rules rather than seeing if the normal checks produce a
diagnostic.

This fixes the handling of C++2a extensions in lambdas in C++17 mode,
as well as some corner cases in earlier language modes where we issue
diagnostics for things other than not satisfying the formal constexpr
requirements.

Modified:
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/Sema/SemaDecl.cpp
cfe/trunk/lib/Sema/SemaDeclCXX.cpp
cfe/trunk/lib/Sema/SemaLambda.cpp
cfe/trunk/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp
cfe/trunk/test/SemaCXX/cxx1z-constexpr-lambdas.cpp

Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=367254&r1=367253&r2=367254&view=diff
==
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Mon Jul 29 12:59:45 2019
@@ -2076,8 +2076,16 @@ public:
  bool &AddToScope);
   bool AddOverriddenMethods(CXXRecordDecl *DC, CXXMethodDecl *MD);
 
-  bool CheckConstexprFunctionDecl(const FunctionDecl *FD);
-  bool CheckConstexprFunctionBody(const FunctionDecl *FD, Stmt *Body);
+  enum class CheckConstexprKind {
+/// Diagnose issues that are non-constant or that are extensions.
+Diagnose,
+/// Identify whether this function satisfies the formal rules for constexpr
+/// functions in the current lanugage mode (with no extensions).
+CheckValid
+  };
+
+  bool CheckConstexprFunctionDefinition(const FunctionDecl *FD,
+CheckConstexprKind Kind);
 
   void DiagnoseHiddenVirtualMethods(CXXMethodDecl *MD);
   void FindHiddenVirtualMethods(CXXMethodDecl *MD,

Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=367254&r1=367253&r2=367254&view=diff
==
--- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDecl.cpp Mon Jul 29 12:59:45 2019
@@ -13532,8 +13532,7 @@ Decl *Sema::ActOnFinishFunctionBody(Decl
 }
 
 if (!IsInstantiation && FD && FD->isConstexpr() && !FD->isInvalidDecl() &&
-(!CheckConstexprFunctionDecl(FD) ||
- !CheckConstexprFunctionBody(FD, Body)))
+!CheckConstexprFunctionDefinition(FD, CheckConstexprKind::Diagnose))
   FD->setInvalidDecl();
 
 if (FD && FD->hasAttr()) {

Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=367254&r1=367253&r2=367254&view=diff
==
--- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Mon Jul 29 12:59:45 2019
@@ -1569,11 +1569,34 @@ void Sema::CheckCXXDefaultArguments(Func
   }
 }
 
+/// Check that the given type is a literal type. Issue a diagnostic if not,
+/// if Kind is Diagnose.
+/// \return \c true if a problem has been found (and optionally diagnosed).
+template 
+static bool CheckLiteralType(Sema &SemaRef, Sema::CheckConstexprKind Kind,
+ SourceLocation Loc, QualType T, unsigned DiagID,
+ Ts &&...DiagArgs) {
+  if (T->isDependentType())
+return false;
+
+  switch (Kind) {
+  case Sema::CheckConstexprKind::Diagnose:
+return SemaRef.RequireLiteralType(Loc, T, DiagID,
+  std::forward(DiagArgs)...);
+
+  case Sema::CheckConstexprKind::CheckValid:
+return !T->isLiteralType(SemaRef.Context);
+  }
+
+  llvm_unreachable("unknown CheckConstexprKind");
+}
+
 // CheckConstexprParameterTypes - Check whether a function's parameter types
 // are all literal types. If so, return true. If not, produce a suitable
 // diagnostic and return false.
 static bool CheckConstexprParameterTypes(Sema &SemaRef,
- const FunctionDecl *FD) {
+ const FunctionDecl *FD,
+ Sema::CheckConstexprKind Kind) {
   unsigned ArgIndex = 0;
   const FunctionProtoType *FT = FD->getType()->getAs();
   for (FunctionProtoType::param_type_iterator i = FT->param_type_begin(),
@@ -1581,11 +1604,10 @@ static bool CheckConstexprParameterTypes
i != e; ++i, ++ArgIndex) {
 const ParmVarDecl *PD = FD->getParamDecl(ArgIndex);
 SourceLocation ParamLoc = PD->getLocation();
-if (!(*i)->isDependentType() &&
-SemaRef.RequireLiteralType(
-ParamLoc, *i, diag::err_constexpr_non_literal_param, ArgIndex + 1,
-PD->getSourceRange(), isa(FD),
-FD->isConsteval()))
+if (Ch

r367255 - Give the 'signed/unsigned wchar_t' extension a warning flag, and follow

[Richard Smith via cfe-commits]

Author: rsmith
Date: Mon Jul 29 13:00:46 2019
New Revision: 367255

URL: http://llvm.org/viewvc/llvm-project?rev=367255&view=rev
Log:
Give the 'signed/unsigned wchar_t' extension a warning flag, and follow
GCC 9 in promoting it to an error by default.

Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/Sema/SemaType.cpp
cfe/trunk/test/ASTMerge/exprs-cpp/test.cpp
cfe/trunk/test/Misc/warning-flags.c
cfe/trunk/test/SemaCXX/wchar_t.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=367255&r1=367254&r2=367255&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Mon Jul 29 13:00:46 
2019
@@ -8509,10 +8509,11 @@ def warn_sync_fetch_and_nand_semantics_c
   InGroup>;
 
 // Type
-def ext_invalid_sign_spec : Extension<"'%0' cannot be signed or unsigned">;
+def ext_wchar_t_sign_spec : ExtWarn<"'%0' cannot be signed or unsigned">,
+  InGroup>, DefaultError;
 def warn_receiver_forward_class : Warning<
-"receiver %0 is a forward class and corresponding @interface may not 
exist">,
-InGroup;
+  "receiver %0 is a forward class and corresponding @interface may not exist">,
+  InGroup;
 def note_method_sent_forward_class : Note<"method %0 is used for the forward 
class">;
 def ext_missing_declspec : ExtWarn<
   "declaration specifier missing, defaulting to 'int'">;

Modified: cfe/trunk/lib/Sema/SemaType.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaType.cpp?rev=367255&r1=367254&r2=367255&view=diff
==
--- cfe/trunk/lib/Sema/SemaType.cpp (original)
+++ cfe/trunk/lib/Sema/SemaType.cpp Mon Jul 29 13:00:46 2019
@@ -1290,14 +1290,14 @@ static QualType ConvertDeclSpecToType(Ty
 if (DS.getTypeSpecSign() == DeclSpec::TSS_unspecified)
   Result = Context.WCharTy;
 else if (DS.getTypeSpecSign() == DeclSpec::TSS_signed) {
-  S.Diag(DS.getTypeSpecSignLoc(), diag::ext_invalid_sign_spec)
+  S.Diag(DS.getTypeSpecSignLoc(), diag::ext_wchar_t_sign_spec)
 << DS.getSpecifierName(DS.getTypeSpecType(),
Context.getPrintingPolicy());
   Result = Context.getSignedWCharType();
 } else {
   assert(DS.getTypeSpecSign() == DeclSpec::TSS_unsigned &&
 "Unknown TSS value");
-  S.Diag(DS.getTypeSpecSignLoc(), diag::ext_invalid_sign_spec)
+  S.Diag(DS.getTypeSpecSignLoc(), diag::ext_wchar_t_sign_spec)
 << DS.getSpecifierName(DS.getTypeSpecType(),
Context.getPrintingPolicy());
   Result = Context.getUnsignedWCharType();

Modified: cfe/trunk/test/ASTMerge/exprs-cpp/test.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/ASTMerge/exprs-cpp/test.cpp?rev=367255&r1=367254&r2=367255&view=diff
==
--- cfe/trunk/test/ASTMerge/exprs-cpp/test.cpp (original)
+++ cfe/trunk/test/ASTMerge/exprs-cpp/test.cpp Mon Jul 29 13:00:46 2019
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++1z -fcxx-exceptions 
-emit-pch -o %t.1.ast %S/Inputs/exprs3.cpp
-// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++1z -fcxx-exceptions 
-ast-merge %t.1.ast -fsyntax-only -verify %s
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++1z -fcxx-exceptions 
-Wno-signed-unsigned-wchar -emit-pch -o %t.1.ast %S/Inputs/exprs3.cpp
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++1z -fcxx-exceptions 
-Wno-signed-unsigned-wchar -ast-merge %t.1.ast -fsyntax-only -verify %s
 // expected-no-diagnostics
 
 static_assert(Ch1 == 'a');

Modified: cfe/trunk/test/Misc/warning-flags.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Misc/warning-flags.c?rev=367255&r1=367254&r2=367255&view=diff
==
--- cfe/trunk/test/Misc/warning-flags.c (original)
+++ cfe/trunk/test/Misc/warning-flags.c Mon Jul 29 13:00:46 2019
@@ -96,4 +96,4 @@ CHECK-NEXT:   warn_weak_import
 
 The list of warnings in -Wpedantic should NEVER grow.
 
-CHECK: Number in -Wpedantic (not covered by other -W flags): 28
+CHECK: Number in -Wpedantic (not covered by other -W flags): 27

Modified: cfe/trunk/test/SemaCXX/wchar_t.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/wchar_t.cpp?rev=367255&r1=367254&r2=367255&view=diff
==
--- cfe/trunk/test/SemaCXX/wchar_t.cpp (original)
+++ cfe/trunk/test/SemaCXX/wchar_t.cpp Mon Jul 29 13:00:46 2019
@@ -1,10 +1,12 @@
-// RUN: %clang_cc1 -fsyntax-only -pedantic -verify %s 
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wno-signed-unsigne

[PATCH] D64146: [ConstExprPreter][WIP] Initial patch for the constexpr interpreter

[Nandor Licker via Phabricator via cfe-commits]

nand marked 10 inline comments as done.
nand added a comment.

We can add a separate integer type which tracks all the additional information 
required by `__builtin_constant_p` and compile all integers to it in this 
context. A later patch added an APInt fallback to the interpreter if an 
integral cannot be mapped to a type supported by the VM - this mechanism could 
be used to implement the fallback for contexts which cast pointers to integers.




Comment at: clang/lib/AST/ExprVM/Compiler.h:125
+/// Size of the local, in bytes.
+unsigned Size;
+  };

jfb wrote:
> `ByteSize` since it's the size in bytes :)
I've removed the size field since it's not going to be used, but left the 
structure since it will gain other fields in future patches.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64146



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

r367256 - [driver][test] Use /dev/null in as-options.s instead

[Jordan Rupprecht via cfe-commits]

Author: rupprecht
Date: Mon Jul 29 13:09:20 2019
New Revision: 367256

URL: http://llvm.org/viewvc/llvm-project?rev=367256&view=rev
Log:
[driver][test] Use /dev/null in as-options.s instead

Modified:
cfe/trunk/test/Driver/as-options.s

Modified: cfe/trunk/test/Driver/as-options.s
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/as-options.s?rev=367256&r1=367255&r2=367256&view=diff
==
--- cfe/trunk/test/Driver/as-options.s (original)
+++ cfe/trunk/test/Driver/as-options.s Mon Jul 29 13:09:20 2019
@@ -74,13 +74,13 @@
 // -Wa flags shouldn't cause warnings without an assembler stage with
 // -fno-integrated-as either.
 // RUN: %clang -Wa,-mno-warn-deprecated -fno-integrated-as -x c++ %s -S 2>&1 \
-// RUN:   -o %t.o \
+// RUN:   -o /dev/null \
 // RUN:   | FileCheck --check-prefix=NOWARN --allow-empty %s
 
 // But -m flags for the integrated assembler _should_ warn if the integrated
 // assembler is not in use.
-// RUN: %clang -mrelax-all -fintegrated-as -x c++ %s -S -o %t.o 2>&1 \
+// RUN: %clang -mrelax-all -fintegrated-as -x c++ %s -S -o /dev/null 2>&1 \
 // RUN:   | FileCheck --check-prefix=NOWARN --allow-empty %s
-// RUN: %clang -mrelax-all -fno-integrated-as -x c++ %s -S -o %t.o 2>&1 \
+// RUN: %clang -mrelax-all -fno-integrated-as -x c++ %s -S -o /dev/null 2>&1 \
 // RUN:   | FileCheck --check-prefix=WARN --allow-empty %s
 // WARN: unused


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

[PATCH] D63048: Update __VERSION__ to remove the hardcoded 4.2.1 version

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 added a comment.

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


Repository:
  rL LLVM

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

https://reviews.llvm.org/D63048



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

[PATCH] D59474: [OpenMP 5.0] Codegen support for user-defined mappers

[Lingda Li via Phabricator via cfe-commits]

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



Comment at: test/OpenMP/declare_mapper_codegen.cpp:44-48
+// CK0-DAG: store i8* %0, i8** [[HANDLEADDR:%[^,]+]]
+// CK0-DAG: store i8* %1, i8** [[BPTRADDR:%[^,]+]]
+// CK0-DAG: store i8* %2, i8** [[VPTRADDR:%[^,]+]]
+// CK0-DAG: store i64 %3, i{{64|32}}* [[SIZEADDR:%[^,]+]]
+// CK0-DAG: store i64 %4, i64* [[TYPEADDR:%[^,]+]]

ABataev wrote:
> lildmh wrote:
> > ABataev wrote:
> > > I would not rely on the predetermined indices here, better to use some 
> > > kind of patterns here just like in other places.
> > Could you give an example about what you suggest? For instance, some other 
> > tests I should look into.
> Just like in this test when you're using vars.
Sorry I was not clear before. What do you mean by "predetermined indices" here? 
If you are referring to, for example, `%0` in `store i8* %0, i8** 
[[HANDLEADDR:%[^,]+]]`, I guess there is no way to get rid of `%0` because it 
means the first argument of the function?


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

https://reviews.llvm.org/D59474



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

[PATCH] D59474: [OpenMP 5.0] Codegen support for user-defined mappers

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: test/OpenMP/declare_mapper_codegen.cpp:44-48
+// CK0-DAG: store i8* %0, i8** [[HANDLEADDR:%[^,]+]]
+// CK0-DAG: store i8* %1, i8** [[BPTRADDR:%[^,]+]]
+// CK0-DAG: store i8* %2, i8** [[VPTRADDR:%[^,]+]]
+// CK0-DAG: store i64 %3, i{{64|32}}* [[SIZEADDR:%[^,]+]]
+// CK0-DAG: store i64 %4, i64* [[TYPEADDR:%[^,]+]]

lildmh wrote:
> ABataev wrote:
> > lildmh wrote:
> > > ABataev wrote:
> > > > I would not rely on the predetermined indices here, better to use some 
> > > > kind of patterns here just like in other places.
> > > Could you give an example about what you suggest? For instance, some 
> > > other tests I should look into.
> > Just like in this test when you're using vars.
> Sorry I was not clear before. What do you mean by "predetermined indices" 
> here? If you are referring to, for example, `%0` in `store i8* %0, i8** 
> [[HANDLEADDR:%[^,]+]]`, I guess there is no way to get rid of `%0` because it 
> means the first argument of the function?
Yes, I meant those `%0` like registers. Better to mark them as variables in 
function declaration and use those names in the checks.


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

https://reviews.llvm.org/D59474



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

[PATCH] D65192: [Sema] Disable some enabled-by-default -Wparentheses diagnostics

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

I'm not sure the history behind why these were added as default-on 
warningsthey don't really seem appropriate as default warnings to me, 
either.

But I think someone else probably ought to approve the change.


Repository:
  rC Clang

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

https://reviews.llvm.org/D65192



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

[PATCH] D59474: [OpenMP 5.0] Codegen support for user-defined mappers

[Lingda Li via Phabricator via cfe-commits]

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



Comment at: test/OpenMP/declare_mapper_codegen.cpp:44-48
+// CK0-DAG: store i8* %0, i8** [[HANDLEADDR:%[^,]+]]
+// CK0-DAG: store i8* %1, i8** [[BPTRADDR:%[^,]+]]
+// CK0-DAG: store i8* %2, i8** [[VPTRADDR:%[^,]+]]
+// CK0-DAG: store i64 %3, i{{64|32}}* [[SIZEADDR:%[^,]+]]
+// CK0-DAG: store i64 %4, i64* [[TYPEADDR:%[^,]+]]

ABataev wrote:
> lildmh wrote:
> > ABataev wrote:
> > > lildmh wrote:
> > > > ABataev wrote:
> > > > > I would not rely on the predetermined indices here, better to use 
> > > > > some kind of patterns here just like in other places.
> > > > Could you give an example about what you suggest? For instance, some 
> > > > other tests I should look into.
> > > Just like in this test when you're using vars.
> > Sorry I was not clear before. What do you mean by "predetermined indices" 
> > here? If you are referring to, for example, `%0` in `store i8* %0, i8** 
> > [[HANDLEADDR:%[^,]+]]`, I guess there is no way to get rid of `%0` because 
> > it means the first argument of the function?
> Yes, I meant those `%0` like registers. Better to mark them as variables in 
> function declaration and use those names in the checks.
Now it's like `define {{.*}}void @.omp_mapper.{{.*}}C.id{{.*}}(i8*, i8*, i8*, 
i64, i64)`, I think you are suggesting something like `define {{.*}}void 
@.omp_mapper.{{.*}}C.id{{.*}}(i8* [[HANDLE:%[^,]+]], i8* [[BPTR:%[^,]+]], 
...)`, and later I can use `store i8* [[HANDLE]], i8** [[HANDLEADDR:%[^,]+]]`

I'm not sure how to add names for function arguments. They seems to be always 
nameless like `(i8*, i8*, i8*, i64, i64)`. Is there a way to do that?


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

https://reviews.llvm.org/D59474



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

[PATCH] D63048: Update __VERSION__ to remove the hardcoded 4.2.1 version

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

In D63048#1605050 , @xbolva00 wrote:

> Should we bump __GNUC__, __GNUC_MINOR__ too?


We discussed this two weeks ago:
http://lists.llvm.org/pipermail/cfe-dev/2019-July/062890.html
It's unlikely that we will make any policy change before 9.0.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D63048



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

[PATCH] D59474: [OpenMP 5.0] Codegen support for user-defined mappers

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: test/OpenMP/declare_mapper_codegen.cpp:44-48
+// CK0-DAG: store i8* %0, i8** [[HANDLEADDR:%[^,]+]]
+// CK0-DAG: store i8* %1, i8** [[BPTRADDR:%[^,]+]]
+// CK0-DAG: store i8* %2, i8** [[VPTRADDR:%[^,]+]]
+// CK0-DAG: store i64 %3, i{{64|32}}* [[SIZEADDR:%[^,]+]]
+// CK0-DAG: store i64 %4, i64* [[TYPEADDR:%[^,]+]]

lildmh wrote:
> ABataev wrote:
> > lildmh wrote:
> > > ABataev wrote:
> > > > lildmh wrote:
> > > > > ABataev wrote:
> > > > > > I would not rely on the predetermined indices here, better to use 
> > > > > > some kind of patterns here just like in other places.
> > > > > Could you give an example about what you suggest? For instance, some 
> > > > > other tests I should look into.
> > > > Just like in this test when you're using vars.
> > > Sorry I was not clear before. What do you mean by "predetermined indices" 
> > > here? If you are referring to, for example, `%0` in `store i8* %0, i8** 
> > > [[HANDLEADDR:%[^,]+]]`, I guess there is no way to get rid of `%0` 
> > > because it means the first argument of the function?
> > Yes, I meant those `%0` like registers. Better to mark them as variables in 
> > function declaration and use those names in the checks.
> Now it's like `define {{.*}}void @.omp_mapper.{{.*}}C.id{{.*}}(i8*, i8*, i8*, 
> i64, i64)`, I think you are suggesting something like `define {{.*}}void 
> @.omp_mapper.{{.*}}C.id{{.*}}(i8* [[HANDLE:%[^,]+]], i8* [[BPTR:%[^,]+]], 
> ...)`, and later I can use `store i8* [[HANDLE]], i8** [[HANDLEADDR:%[^,]+]]`
> 
> I'm not sure how to add names for function arguments. They seems to be always 
> nameless like `(i8*, i8*, i8*, i64, i64)`. Is there a way to do that?
If the clang parameters have names, the llvm params also will get the names. 
But it is not worth it to add the names to the function. Could just use regexp 
here to avoid using LLVM register names? Just `%{{·+}}`. And rely on the order, 
i.e. remove `-DAG` checks?


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

https://reviews.llvm.org/D59474



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

[PATCH] D64799: [Sema] Emit diagnostics for uncorrected delayed typos at the end of TU

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

@rsmith, emitting the typos on pop expression evaluation context resulted in a 
bunch of failures when trying to transform the resulting errors,  see a typical 
stacktrace below.
It seems we can actually pop expression evaluation contexts between producing 
and correcting a typo expression.

I don't see how to workaround this without digging further into typo correction 
and changing its design.
Would you be ok with landing the workaround as is? Alternatively, any ideas on 
how we can avoid this problem without extending the scope of the patch too much?

  FAIL: Clang :: CXX/temp/temp.decls/temp.class/temp.mem.enum/p1.cpp (1595 of 
15306)
   TEST 'Clang :: 
CXX/temp/temp.decls/temp.class/temp.mem.enum/p1.cpp' FAILED 
  Script:
  --
  : 'RUN: at line 1';   
/usr/local/google/home/ibiryukov/projects/llvm/build-rel/bin/clang -cc1 
-internal-isystem 
/usr/local/google/home/ibiryukov/projects/llvm/build-rel/lib/clang/10.0.0/include
 -nostdsysteminc -std=c++11 -verify 
/usr/local/google/home/ibiryukov/projects/llvm/clang/test/CXX/temp/temp.decls/temp.class/temp.mem.enum/p1.cpp
  --
  Exit Code: 134
  
  Command Output (stderr):
  --
  clang: 
/usr/local/google/home/ibiryukov/projects/llvm/clang/lib/Sema/SemaLookup.cpp:5378:
 const Sema::TypoExprState &clang::Sema::getTypoExprState(clang::TypoExpr *) 
const: Assertion `Entry != DelayedTypos.end() && "Failed to get the state for a 
TypoExpr!"' failed.
  Stack dump:
  0.  Program arguments: 
/usr/local/google/home/ibiryukov/projects/llvm/build-rel/bin/clang -cc1 
-internal-isystem 
/usr/local/google/home/ibiryukov/projects/llvm/build-rel/lib/clang/10.0.0/include
 -nostdsysteminc -std=c++11 -verify 
/usr/local/google/home/ibiryukov/projects/llvm/clang/test/CXX/temp/temp.decls/temp.class/temp.mem.enum/p1.cpp
  1.  
/usr/local/google/home/ibiryukov/projects/llvm/clang/test/CXX/temp/temp.decls/temp.class/temp.mem.enum/p1.cpp:23:26:
 current parser token ';'
   #0 0x043e1ea4 PrintStackTrace 
/usr/local/google/home/ibiryukov/projects/llvm/llvm/lib/Support/Unix/Signals.inc:533:13
   #1 0x043e1ea4 PrintStackTraceSignalHandler(void*) 
/usr/local/google/home/ibiryukov/projects/llvm/llvm/lib/Support/Unix/Signals.inc:593:0
   #2 0x043dfa9e llvm::sys::RunSignalHandlers() 
/usr/local/google/home/ibiryukov/projects/llvm/llvm/lib/Support/Signals.cpp:69:18
   #3 0x043e22b8 SignalHandler(int) 
/usr/local/google/home/ibiryukov/projects/llvm/llvm/lib/Support/Unix/Signals.inc:385:1
   #4 0x7fedabb7d3a0 __restore_rt 
(/lib/x86_64-linux-gnu/libpthread.so.0+0x123a0)
   #5 0x7fedaac0bcfb raise (/lib/x86_64-linux-gnu/libc.so.6+0x36cfb)
   #6 0x7fedaabf68ad abort (/lib/x86_64-linux-gnu/libc.so.6+0x218ad)
   #7 0x7fedaabf677f (/lib/x86_64-linux-gnu/libc.so.6+0x2177f)
   #8 0x7fedaac04542 (/lib/x86_64-linux-gnu/libc.so.6+0x2f542)
   #9 0x05f4ba2e LookupBucketFor 
/usr/local/google/home/ibiryukov/projects/llvm/llvm/include/llvm/ADT/DenseMap.h:618:5
  #10 0x05f4ba2e find 
/usr/local/google/home/ibiryukov/projects/llvm/llvm/include/llvm/ADT/DenseMap.h:184:0
  #11 0x05f4ba2e find 
/usr/local/google/home/ibiryukov/projects/llvm/llvm/include/llvm/ADT/MapVector.h:154:0
  #12 0x05f4ba2e clang::Sema::getTypoExprState(clang::TypoExpr*) const 
/usr/local/google/home/ibiryukov/projects/llvm/clang/lib/Sema/SemaLookup.cpp:5376:0
  #13 0x05e94695 TransformTypoExpr 
/usr/local/google/home/ibiryukov/projects/llvm/clang/lib/Sema/SemaExprCXX.cpp:7740:27
  #14 0x05e94695 clang::TreeTransform<(anonymous 
namespace)::TransformTypos>::TransformExpr(clang::Expr*) 
/usr/local/google/home/ibiryukov/projects/llvm/build-rel/tools/clang/include/clang/AST/StmtNodes.inc:1277:0
  #15 0x05e708ea hasErrorOccurred 
/usr/local/google/home/ibiryukov/projects/llvm/clang/include/clang/Sema/Sema.h:7864:38
  #16 0x05e708ea TryTransform 
/usr/local/google/home/ibiryukov/projects/llvm/clang/lib/Sema/SemaExprCXX.cpp:7651:0
  #17 0x05e708ea Transform 
/usr/local/google/home/ibiryukov/projects/llvm/clang/lib/Sema/SemaExprCXX.cpp:7686:0
  #18 0x05e708ea clang::Sema::CorrectDelayedTyposInExpr(clang::Expr*, 
clang::VarDecl*, llvm::function_ref 
(clang::Expr*)>) 
/usr/local/google/home/ibiryukov/projects/llvm/clang/lib/Sema/SemaExprCXX.cpp:7784:0
  #19 0x05c5deae isInvalid 
/usr/local/google/home/ibiryukov/projects/llvm/clang/include/clang/Sema/Ownership.h:208:37
  #20 0x05c5deae clang::Sema::AddInitializerToDecl(clang::Decl*, 
clang::Expr*, bool) 
/usr/local/google/home/ibiryukov/projects/llvm/clang/lib/Sema/SemaDecl.cpp:11236:0
  #21 0x05a22997 
clang::Parser::ParseDeclarationAfterDeclaratorAndAttributes(clang::Declarator&, 
clang::Parser::ParsedTemplateInfo const&, clang::Parser::ForRangeInit*) 
/usr/local/google/home/ibiryukov/projects/llvm/clang/lib/Parse/ParseDecl.cpp:2456:3
  #22 0x05a2093c clang

[PATCH] D65419: [clang-doc] Fix failing tests on Windows

[Diego Astiazarán via Phabricator via cfe-commits]

DiegoAstiazaran created this revision.
DiegoAstiazaran added a reviewer: juliehockett.
DiegoAstiazaran added a project: clang-tools-extra.

Tests on Windows were failing due to path separator differences.
Links in HTML should use posix-style paths.


https://reviews.llvm.org/D65419

Files:
  clang-tools-extra/clang-doc/HTMLGenerator.cpp


Index: clang-tools-extra/clang-doc/HTMLGenerator.cpp
===
--- clang-tools-extra/clang-doc/HTMLGenerator.cpp
+++ clang-tools-extra/clang-doc/HTMLGenerator.cpp
@@ -252,6 +252,8 @@
   llvm::SmallString<128> Path =
   computeRelativePath(Type.Path, CurrentDirectory);
   llvm::sys::path::append(Path, Type.Name + ".html");
+  // Paths in HTML must be in posix-style
+  llvm::sys::path::native(Path, llvm::sys::path::Style::posix);
   return genLink(Type.Name, Path);
 }
 


Index: clang-tools-extra/clang-doc/HTMLGenerator.cpp
===
--- clang-tools-extra/clang-doc/HTMLGenerator.cpp
+++ clang-tools-extra/clang-doc/HTMLGenerator.cpp
@@ -252,6 +252,8 @@
   llvm::SmallString<128> Path =
   computeRelativePath(Type.Path, CurrentDirectory);
   llvm::sys::path::append(Path, Type.Name + ".html");
+  // Paths in HTML must be in posix-style
+  llvm::sys::path::native(Path, llvm::sys::path::Style::posix);
   return genLink(Type.Name, Path);
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits