[PATCH] D63376: [clang] Small improvments after Adding APValue to ConstantExpr

2019-06-17 Thread Tyker via Phabricator via cfe-commits
Tyker updated this revision to Diff 204992.

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

https://reviews.llvm.org/D63376

Files:
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/Expr.h
  clang/include/clang/AST/Stmt.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/Expr.cpp
  clang/lib/AST/ExprConstant.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaOverload.cpp

Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -5514,7 +5514,12 @@
 
 if (Notes.empty()) {
   // It's a constant expression.
-  return ConstantExpr::Create(S.Context, Result.get(), Value);
+
+  // ExplicitBool wouldn't use the value stored in ConstantExpr.
+  if (CCE != Sema::CCEK_ExplicitBool)
+return ConstantExpr::Create(S.Context, Result.get(), Value);
+  else
+return ConstantExpr::Create(S.Context, Result.get());
 }
   }
 
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -14467,10 +14467,10 @@
   S.Diag(Loc, diag::ext_expr_not_ice) << SR << S.LangOpts.CPlusPlus;
 }
 
-ExprResult
-Sema::VerifyIntegerConstantExpression(Expr *E, llvm::APSInt *Result,
-  VerifyICEDiagnoser &Diagnoser,
-  bool AllowFold) {
+ExprResult Sema::VerifyIntegerConstantExpression(Expr *E, llvm::APSInt *Result,
+ VerifyICEDiagnoser &Diagnoser,
+ bool AllowFold,
+ bool StoreResult) {
   SourceLocation DiagLoc = E->getBeginLoc();
 
   if (getLangOpts().CPlusPlus11) {
@@ -14538,14 +14538,13 @@
 return ExprError();
   }
 
-  if (!isa(E))
-E = ConstantExpr::Create(Context, E);
-
   // Circumvent ICE checking in C++11 to avoid evaluating the expression twice
   // in the non-ICE case.
   if (!getLangOpts().CPlusPlus11 && E->isIntegerConstantExpr(Context)) {
 if (Result)
   *Result = E->EvaluateKnownConstIntCheckOverflow(Context);
+if (!isa(E))
+  E = ConstantExpr::Create(Context, E);
 return E;
   }
 
@@ -14555,8 +14554,13 @@
 
   // Try to evaluate the expression, and produce diagnostics explaining why it's
   // not a constant expression as a side-effect.
-  bool Folded = E->EvaluateAsRValue(EvalResult, Context) &&
-EvalResult.Val.isInt() && !EvalResult.HasSideEffects;
+  bool Folded =
+  E->EvaluateAsRValue(EvalResult, Context, /*isConstantContext*/ true) &&
+  EvalResult.Val.isInt() && !EvalResult.HasSideEffects;
+
+  if (!isa(E))
+if (StoreResult)
+  E = ConstantExpr::Create(Context, E, EvalResult.Val);
 
   // In C++11, we can rely on diagnostics being produced for any expression
   // which is not a constant expression. If no diagnostics were produced, then
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -14006,8 +14006,6 @@
 ExprResult Converted = PerformContextuallyConvertToBool(AssertExpr);
 if (Converted.isInvalid())
   Failed = true;
-else
-  Converted = ConstantExpr::Create(Context, Converted.get());
 
 llvm::APSInt Cond;
 if (!Failed && VerifyIntegerConstantExpression(Converted.get(), &Cond,
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -8963,6 +8963,8 @@
 
 bool IntExprEvaluator::VisitConstantExpr(const ConstantExpr *E) {
   llvm::SaveAndRestore InConstantContext(Info.InConstantContext, true);
+  if (E->getResultAPValueKind() != APValue::None)
+return Success(E->getAPValueResult(), E);
   return ExprEvaluatorBaseTy::VisitConstantExpr(E);
 }
 
Index: clang/lib/AST/Expr.cpp
===
--- clang/lib/AST/Expr.cpp
+++ clang/lib/AST/Expr.cpp
@@ -239,6 +239,7 @@
 ConstantExpr::getStorageKind(const APValue &Value) {
   switch (Value.getKind()) {
   case APValue::None:
+  case APValue::Indeterminate:
 return ConstantExpr::RSK_None;
   case APValue::Int:
 if (!Value.getInt().needsCleanup())
@@ -249,9 +250,18 @@
   }
 }
 
+ConstantExpr::ResultStorageKind
+ConstantExpr::getStorageKind(const Type *T, const ASTContext &Context) {
+  if (T->isIntegralOrEnumerationType() && Context.getTypeInfo(T).Width <= 64)
+return ConstantExpr::RSK_Int64;
+  return ConstantExpr::RSK_APValue;
+}
+
 void ConstantExpr::DefaultInit(ResultStorageKind StorageKind) {
   ConstantExprBits.ResultKind = StorageKind;
-  if (StorageKind == RSK_APValue)
+  ConstantExprBits.APV

r363529 - Re-commit r357452 (take 3): "SimplifyCFG SinkCommonCodeFromPredecessors: Also sink function calls without used results (PR41259)"

2019-06-17 Thread Hans Wennborg via cfe-commits
Author: hans
Date: Mon Jun 17 00:47:28 2019
New Revision: 363529

URL: http://llvm.org/viewvc/llvm-project?rev=363529&view=rev
Log:
Re-commit r357452 (take 3): "SimplifyCFG SinkCommonCodeFromPredecessors: Also 
sink function calls without used results (PR41259)"

Third time's the charm.

This was reverted in r363220 due to being suspected of an internal benchmark
regression and a test failure, none of which turned out to be caused by this.

Modified:
cfe/trunk/test/CodeGenCXX/nrvo.cpp
cfe/trunk/test/CodeGenCXX/stack-reuse-exceptions.cpp
cfe/trunk/test/CodeGenObjC/exceptions.m

Modified: cfe/trunk/test/CodeGenCXX/nrvo.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/nrvo.cpp?rev=363529&r1=363528&r2=363529&view=diff
==
--- cfe/trunk/test/CodeGenCXX/nrvo.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/nrvo.cpp Mon Jun 17 00:47:28 2019
@@ -60,7 +60,6 @@ X test2(bool B) {
   // CHECK-NEXT: call void @llvm.lifetime.start
   // CHECK-NEXT: call {{.*}} @_ZN1XC1Ev
   // CHECK: call {{.*}} @_ZN1XC1ERKS_
-  // CHECK: call {{.*}} @_ZN1XC1ERKS_
   // CHECK: call {{.*}} @_ZN1XD1Ev
   // CHECK-NEXT: call void @llvm.lifetime.end
   // CHECK: call {{.*}} @_ZN1XD1Ev

Modified: cfe/trunk/test/CodeGenCXX/stack-reuse-exceptions.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/stack-reuse-exceptions.cpp?rev=363529&r1=363528&r2=363529&view=diff
==
--- cfe/trunk/test/CodeGenCXX/stack-reuse-exceptions.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/stack-reuse-exceptions.cpp Mon Jun 17 00:47:28 
2019
@@ -1,5 +1,5 @@
 // RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu %s -o - -emit-llvm -O1 \
-// RUN: -fexceptions -fcxx-exceptions | FileCheck %s
+// RUN: -fexceptions -fcxx-exceptions -mllvm 
-simplifycfg-sink-common=false | FileCheck %s
 //
 // We should emit lifetime.ends for these temporaries in both the 'exception'
 // and 'normal' paths in functions.

Modified: cfe/trunk/test/CodeGenObjC/exceptions.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjC/exceptions.m?rev=363529&r1=363528&r2=363529&view=diff
==
--- cfe/trunk/test/CodeGenObjC/exceptions.m (original)
+++ cfe/trunk/test/CodeGenObjC/exceptions.m Mon Jun 17 00:47:28 2019
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple x86_64-apple-darwin10 
-fobjc-runtime=macosx-fragile-10.5 -emit-llvm -fobjc-exceptions -O2 -o - %s | 
FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 
-fobjc-runtime=macosx-fragile-10.5 -emit-llvm -fobjc-exceptions -mllvm 
-simplifycfg-sink-common=false -O2 -o - %s | FileCheck %s
 //
 //  [irgen] [eh] Exception code built with clang 
(x86_64) crashes
 


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


[PATCH] D63397: [clangd] Detect C++ for extension-less source files in vscode extension

2019-06-17 Thread Haojian Wu via Phabricator via cfe-commits
hokein created this revision.
hokein added a reviewer: kadircet.
Herald added subscribers: arphaman, jkorous, MaskRay, ilya-biryukov.
Herald added a project: clang.

Extend our extension to support detecting these files as C++ files based on the 
first
line (`-*- C++ -*-`), it will make clangd work on C++ standard headers
(e.g. iostream).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D63397

Files:
  clang-tools-extra/clangd/clients/clangd-vscode/package.json


Index: clang-tools-extra/clangd/clients/clangd-vscode/package.json
===
--- clang-tools-extra/clangd/clients/clangd-vscode/package.json
+++ clang-tools-extra/clangd/clients/clangd-vscode/package.json
@@ -50,6 +50,10 @@
 "url": 
"http://llvm.org/svn/llvm-project/clang-tools-extra/trunk/clangd/clients/clangd-vscode/";
 },
 "contributes": {
+"languages": [{
+"id": "cpp",
+"firstLine": ".*-\\*-\\s*C\\+\\+\\s*-\\*-.*"
+}],
 "configuration": {
 "type": "object",
 "title": "clangd configuration",


Index: clang-tools-extra/clangd/clients/clangd-vscode/package.json
===
--- clang-tools-extra/clangd/clients/clangd-vscode/package.json
+++ clang-tools-extra/clangd/clients/clangd-vscode/package.json
@@ -50,6 +50,10 @@
 "url": "http://llvm.org/svn/llvm-project/clang-tools-extra/trunk/clangd/clients/clangd-vscode/";
 },
 "contributes": {
+"languages": [{
+"id": "cpp",
+"firstLine": ".*-\\*-\\s*C\\+\\+\\s*-\\*-.*"
+}],
 "configuration": {
 "type": "object",
 "title": "clangd configuration",
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D63397: [clangd] Detect C++ for extension-less source files in vscode extension

2019-06-17 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment.

Could you also update summary to include a link to documentation regarding 
`contributes.languages` ?




Comment at: clang-tools-extra/clangd/clients/clangd-vscode/package.json:55
+"id": "cpp",
+"firstLine": ".*-\\*-\\s*C\\+\\+\\s*-\\*-.*"
+}],

could you also make sure it starts with "^//"


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63397



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


[PATCH] D63316: [clangd] Include the diagnostics's code when comparing diagnostics

2019-06-17 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision.
kadircet added a comment.
This revision is now accepted and ready to land.

LGTM with one more test case request. Thanks!




Comment at: clang-tools-extra/clangd/test/fixits-duplication.test:44
+---
+{"jsonrpc":"2.0","id":2,"method":"textDocument/codeAction","params":{"textDocument":{"uri":"test:///foo.cpp"},"range":{"start":{"line":0,"character":23},"end":{"line":0,"character":24}},"context":{"diagnostics":[{"range":{"start":
 {"line": 0, "character": 23}, "end": {"line": 0, "character": 
24}},"severity":2,"message":"Use nullptr (fix available)", "code": 
"hicpp-use-nullptr", "source": "clang-tidy"},{"range":{"start": {"line": 0, 
"character": 23}, "end": {"line": 0, "character": 
24}},"severity":2,"message":"Use nullptr (fix available)", "code": 
"modernize-use-nullptr", "source": "clang-tidy"}]}}}
+#  CHECK:  "id": 2,

could you also add another codeAction request with one of the codes missing?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63316



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


[PATCH] D63397: [clangd] Detect C++ for extension-less source files in vscode extension

2019-06-17 Thread Haojian Wu via Phabricator via cfe-commits
hokein updated this revision to Diff 204999.
hokein marked 2 inline comments as done.
hokein added a comment.

Address comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63397

Files:
  clang-tools-extra/clangd/clients/clangd-vscode/package.json


Index: clang-tools-extra/clangd/clients/clangd-vscode/package.json
===
--- clang-tools-extra/clangd/clients/clangd-vscode/package.json
+++ clang-tools-extra/clangd/clients/clangd-vscode/package.json
@@ -50,6 +50,10 @@
 "url": 
"http://llvm.org/svn/llvm-project/clang-tools-extra/trunk/clangd/clients/clangd-vscode/";
 },
 "contributes": {
+"languages": [{
+"id": "cpp",
+"firstLine": "^\/[/*].*-\\*-\\s*C\\+\\+\\s*-\\*-.*"
+}],
 "configuration": {
 "type": "object",
 "title": "clangd configuration",


Index: clang-tools-extra/clangd/clients/clangd-vscode/package.json
===
--- clang-tools-extra/clangd/clients/clangd-vscode/package.json
+++ clang-tools-extra/clangd/clients/clangd-vscode/package.json
@@ -50,6 +50,10 @@
 "url": "http://llvm.org/svn/llvm-project/clang-tools-extra/trunk/clangd/clients/clangd-vscode/";
 },
 "contributes": {
+"languages": [{
+"id": "cpp",
+"firstLine": "^\/[/*].*-\\*-\\s*C\\+\\+\\s*-\\*-.*"
+}],
 "configuration": {
 "type": "object",
 "title": "clangd configuration",
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D62115: fix a issue that clang is incompatible with gcc with -H option.

2019-06-17 Thread Kan Shengchen via Phabricator via cfe-commits
skan updated this revision to Diff 205001.
skan added a comment.

move test case to print-header-includes.c


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

https://reviews.llvm.org/D62115

Files:
  lib/Frontend/HeaderIncludeGen.cpp
  test/Frontend/print-header-includes.c


Index: test/Frontend/print-header-includes.c
===
--- test/Frontend/print-header-includes.c
+++ test/Frontend/print-header-includes.c
@@ -29,4 +29,15 @@
 // MS-BLACKLIST: Note: including file:  {{[^ ]*test2.h}}
 // MS-BLACKLIST-NOT: Note
 
+// RUN: %clang_cc1 -H -fsyntax-only %s 2>&1 | FileCheck %s \
+// RUN: --check-prefix=GCC-STDOUT
+// GCC-STDOUT: .
+// GCC-STDOUT-SAME: Inputs{{\/|}}test.h
+// GCC-STDOUT: ..
+// GCC-STDOUT-SAME: Inputs{{\/|}}test2.h
+
+#if defined(_WIN32) || defined(_WIN64)
+#include "Inputs\test.h"
+#else
 #include "Inputs/test.h"
+#endif
Index: lib/Frontend/HeaderIncludeGen.cpp
===
--- lib/Frontend/HeaderIncludeGen.cpp
+++ lib/Frontend/HeaderIncludeGen.cpp
@@ -51,6 +51,10 @@
 static void PrintHeaderInfo(raw_ostream *OutputFile, StringRef Filename,
 bool ShowDepth, unsigned CurrentIncludeDepth,
 bool MSStyle) {
+  // Simplify Filename that starts with "./"
+  if (Filename.startswith("./"))
+Filename = Filename.substr(2);
+
   // Write to a temporary string to avoid unnecessary flushing on errs().
   SmallString<512> Pathname(Filename);
   if (!MSStyle)


Index: test/Frontend/print-header-includes.c
===
--- test/Frontend/print-header-includes.c
+++ test/Frontend/print-header-includes.c
@@ -29,4 +29,15 @@
 // MS-BLACKLIST: Note: including file:  {{[^ ]*test2.h}}
 // MS-BLACKLIST-NOT: Note
 
+// RUN: %clang_cc1 -H -fsyntax-only %s 2>&1 | FileCheck %s \
+// RUN: --check-prefix=GCC-STDOUT
+// GCC-STDOUT: .
+// GCC-STDOUT-SAME: Inputs{{\/|}}test.h
+// GCC-STDOUT: ..
+// GCC-STDOUT-SAME: Inputs{{\/|}}test2.h
+
+#if defined(_WIN32) || defined(_WIN64)
+#include "Inputs\test.h"
+#else
 #include "Inputs/test.h"
+#endif
Index: lib/Frontend/HeaderIncludeGen.cpp
===
--- lib/Frontend/HeaderIncludeGen.cpp
+++ lib/Frontend/HeaderIncludeGen.cpp
@@ -51,6 +51,10 @@
 static void PrintHeaderInfo(raw_ostream *OutputFile, StringRef Filename,
 bool ShowDepth, unsigned CurrentIncludeDepth,
 bool MSStyle) {
+  // Simplify Filename that starts with "./"
+  if (Filename.startswith("./"))
+Filename = Filename.substr(2);
+
   // Write to a temporary string to avoid unnecessary flushing on errs().
   SmallString<512> Pathname(Filename);
   if (!MSStyle)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D62115: fix a issue that clang is incompatible with gcc with -H option.

2019-06-17 Thread Kan Shengchen via Phabricator via cfe-commits
skan added a comment.

In D62115#1538631 , @kimgr wrote:

> I think the test needs a bit more work. It's essentially checking the same 
> thing twice to exercise the Windows path separators.
>
> It looks like there's already a test for `-H` in 
> `FrontEnd/print-header-includes.c`. That also demonstrates the use of 
> `--check-prefix` to handle target-specific stuff. Maybe you could fold this 
> into there?


Done


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

https://reviews.llvm.org/D62115



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


r363541 - Recommit [OpenCL] Move OpenCLBuiltins.td and remove unused include

2019-06-17 Thread Sven van Haastregt via cfe-commits
Author: svenvh
Date: Mon Jun 17 03:06:34 2019
New Revision: 363541

URL: http://llvm.org/viewvc/llvm-project?rev=363541&view=rev
Log:
Recommit [OpenCL] Move OpenCLBuiltins.td and remove unused include

Reland r363242 after fixing an issue with the tablegen dependence.

Patch by Pierre Gondois and Sven van Haastregt.

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

Added:
cfe/trunk/lib/Sema/OpenCLBuiltins.td
  - copied, changed from r363529, 
cfe/trunk/include/clang/Basic/OpenCLBuiltins.td
Removed:
cfe/trunk/include/clang/Basic/OpenCLBuiltins.td
Modified:
cfe/trunk/include/clang/Basic/CMakeLists.txt
cfe/trunk/lib/Sema/CMakeLists.txt
cfe/trunk/lib/Sema/SemaLookup.cpp

Modified: cfe/trunk/include/clang/Basic/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/CMakeLists.txt?rev=363541&r1=363540&r2=363541&view=diff
==
--- cfe/trunk/include/clang/Basic/CMakeLists.txt (original)
+++ cfe/trunk/include/clang/Basic/CMakeLists.txt Mon Jun 17 03:06:34 2019
@@ -41,12 +41,6 @@ clang_tablegen(AttrHasAttributeImpl.inc
   TARGET ClangAttrHasAttributeImpl
   )
 
-clang_tablegen(OpenCLBuiltins.inc
-  -I ${CMAKE_CURRENT_SOURCE_DIR}/../../ -gen-clang-opencl-builtins
-  SOURCE OpenCLBuiltins.td
-  TARGET ClangOpenCLBuiltinsImpl
-  )
-
 # ARM NEON
 clang_tablegen(arm_neon.inc -gen-arm-neon-sema
   SOURCE arm_neon.td

Removed: cfe/trunk/include/clang/Basic/OpenCLBuiltins.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/OpenCLBuiltins.td?rev=363540&view=auto
==
--- cfe/trunk/include/clang/Basic/OpenCLBuiltins.td (original)
+++ cfe/trunk/include/clang/Basic/OpenCLBuiltins.td (removed)
@@ -1,296 +0,0 @@
-//==--- OpenCLBuiltins.td - OpenCL builtin declarations 
---===//
-//
-// The LLVM Compiler Infrastructure
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===--===//
-//
-// This file contains TableGen definitions for OpenCL builtin function
-// declarations.  In case of an unresolved function name in OpenCL, Clang will
-// check for a function described in this file when -fdeclare-opencl-builtins
-// is specified.
-//
-//===--===//
-
-//===--===//
-//  Definitions of miscellaneous basic entities.
-//===--===//
-// Versions of OpenCL
-class Version {
-  int Version = _Version;
-}
-def CL10: Version<100>;
-def CL11: Version<110>;
-def CL12: Version<120>;
-def CL20: Version<200>;
-
-// Address spaces
-// Pointer types need to be assigned an address space.
-class AddressSpace {
-  string AddrSpace = _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">;
-
-
-// Qualified Type. Allow to retrieve one ASTContext QualType.
-class QualType {
-  // Name of the field or function in a clang::ASTContext
-  // E.g. Name="IntTy" for the int type, and "getIntPtrType()" for an intptr_t
-  string Name = _Name;
-}
-
-// Helper class to store type access qualifiers (volatile, const, ...).
-class Qualifier {
-  string QualName = _QualName;
-}
-
-//===--===//
-//  OpenCL C classes for types
-//===--===//
-// OpenCL types (int, float, ...)
-class Type {
-  // Name of the Type
-  string Name = _Name;
-  // QualType associated with this type
-  QualType QTName = _QTName;
-  // Size of the vector (if applicable)
-  int VecWidth = 0;
-  // Is pointer
-  bit IsPointer = 0;
-  // List of qualifiers associated with the type (volatile, ...)
-  list QualList = [];
-  // Address space
-  string AddrSpace = "clang::LangAS::Default";
-  // Access qualifier. Must be one of ("RO", "WO", "RW").
-  string AccessQualifier = "";
-}
-
-// OpenCL vector types (e.g. int2, int3, int16, float8, ...)
-class VectorType : Type<_Ty.Name, _Ty.QTName> {
-  int VecWidth = _VecWidth;
-}
-
-// OpenCL pointer types (e.g. int*, float*, ...)
-class PointerType :
-  Type<_Ty.Name, _Ty.QTName> {
-  bit IsPointer = 1;
-  string AddrSpace 

[PATCH] D62944: [Driver] Fix wchar_t and wint_t definitions on Solaris

2019-06-17 Thread Rainer Orth via Phabricator via cfe-commits
ro updated this revision to Diff 205012.
ro added a comment.
Herald added a subscriber: krytarowski.

Adapt `Sema/format-strings.c`.


Repository:
  rC Clang

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

https://reviews.llvm.org/D62944

Files:
  lib/Basic/Targets/OSTargets.h
  test/Preprocessor/wchar_t.c
  test/Sema/format-strings.c
  test/Sema/wchar.c


Index: test/Sema/wchar.c
===
--- test/Sema/wchar.c
+++ test/Sema/wchar.c
@@ -9,7 +9,11 @@
 #elif defined(__arm) || defined(__aarch64__)
   #define WCHAR_T_TYPE unsigned int
 #elif defined(__sun)
-  #define WCHAR_T_TYPE long
+  #if defined(__LP64__)
+#define WCHAR_T_TYPE int
+  #else
+#define WCHAR_T_TYPE long
+  #endif
 #else /* Solaris. */
   #define WCHAR_T_TYPE int
 #endif
Index: test/Sema/format-strings.c
===
--- test/Sema/format-strings.c
+++ test/Sema/format-strings.c
@@ -329,7 +329,11 @@
   printf("%S", s); // no-warning
   printf("%s", s); // expected-warning{{format specifies type 'char *' but the 
argument has type 'wchar_t *'}}
   printf("%C", s[0]); // no-warning
+#if defined(__sun) && !defined(__LP64__)
+  printf("%c", s[0]); // expected-warning{{format specifies type 'int' but the 
argument has type 'wchar_t' (aka 'long')}}
+#else
   printf("%c", s[0]);
+#endif
   // FIXME: This test reports inconsistent results. On Windows, '%C' expects
   // 'unsigned short'.
   // printf("%C", 10);
@@ -401,7 +405,7 @@
 void pr7981(wint_t c, wchar_t c2) {
   printf("%lc", c); // no-warning
   printf("%lc", 1.0); // expected-warning{{the argument has type 'double'}}
-#if __WINT_WIDTH__ == 32
+#if __WINT_WIDTH__ == 32 && !(defined(__sun) && !defined(__LP64__))
   printf("%lc", (char) 1); // no-warning
 #else
   printf("%lc", (char) 1); // expected-warning{{the argument has type 'char'}}
Index: test/Preprocessor/wchar_t.c
===
--- test/Preprocessor/wchar_t.c
+++ test/Preprocessor/wchar_t.c
@@ -1,8 +1,13 @@
 // RUN: %clang_cc1 -triple i386-pc-solaris -dM -E %s -o - | FileCheck %s 
-check-prefix CHECK-SOLARIS
 // CHECK-SOLARIS-DAG: #define __WCHAR_MAX__ 2147483647
-// CHECK-SOLARIS-DAG: #define __WCHAR_TYPE__ int
+// CHECK-SOLARIS-DAG: #define __WCHAR_TYPE__ long int
 // CHECK-SOLARIS-NOT: #define __WCHAR_UNSIGNED__ 0
 
+// RUN: %clang_cc1 -triple x86_64-pc-solaris -dM -E %s -o - | FileCheck %s 
-check-prefix CHECK-SOLARIS64
+// CHECK-SOLARIS64-DAG: #define __WCHAR_MAX__ 2147483647
+// CHECK-SOLARIS64-DAG: #define __WCHAR_TYPE__ int
+// CHECK-SOLARIS64-NOT: #define __WCHAR_UNSIGNED__ 0
+
 // RUN: %clang_cc1 -triple avr-unknown-unknown -fwchar-type=int -fsigned-wchar 
-dM -E %s -o - | FileCheck %s -check-prefix CHECK-AVR
 // CHECK-AVR-DAG: #define __WCHAR_MAX__ 32767
 // CHECK-AVR-DAG: #define __WCHAR_TYPE__ int
Index: lib/Basic/Targets/OSTargets.h
===
--- lib/Basic/Targets/OSTargets.h
+++ lib/Basic/Targets/OSTargets.h
@@ -632,7 +632,11 @@
 public:
   SolarisTargetInfo(const llvm::Triple &Triple, const TargetOptions &Opts)
   : OSTargetInfo(Triple, Opts) {
-// FIXME: WIntType should be SignedLong
+if (this->PointerWidth == 64) {
+  this->WCharType = this->WIntType = this->SignedInt;
+} else {
+  this->WCharType = this->WIntType = this->SignedLong;
+}
 switch (Triple.getArch()) {
 default:
   break;


Index: test/Sema/wchar.c
===
--- test/Sema/wchar.c
+++ test/Sema/wchar.c
@@ -9,7 +9,11 @@
 #elif defined(__arm) || defined(__aarch64__)
   #define WCHAR_T_TYPE unsigned int
 #elif defined(__sun)
-  #define WCHAR_T_TYPE long
+  #if defined(__LP64__)
+#define WCHAR_T_TYPE int
+  #else
+#define WCHAR_T_TYPE long
+  #endif
 #else /* Solaris. */
   #define WCHAR_T_TYPE int
 #endif
Index: test/Sema/format-strings.c
===
--- test/Sema/format-strings.c
+++ test/Sema/format-strings.c
@@ -329,7 +329,11 @@
   printf("%S", s); // no-warning
   printf("%s", s); // expected-warning{{format specifies type 'char *' but the argument has type 'wchar_t *'}}
   printf("%C", s[0]); // no-warning
+#if defined(__sun) && !defined(__LP64__)
+  printf("%c", s[0]); // expected-warning{{format specifies type 'int' but the argument has type 'wchar_t' (aka 'long')}}
+#else
   printf("%c", s[0]);
+#endif
   // FIXME: This test reports inconsistent results. On Windows, '%C' expects
   // 'unsigned short'.
   // printf("%C", 10);
@@ -401,7 +405,7 @@
 void pr7981(wint_t c, wchar_t c2) {
   printf("%lc", c); // no-warning
   printf("%lc", 1.0); // expected-warning{{the argument has type 'double'}}
-#if __WINT_WIDTH__ == 32
+#if __WINT_WIDTH__ == 32 && !(defined(__sun) && !defined(__LP64__))
   printf("%lc", (char) 1); // no-warning
 #else
   printf("%

[libunwind] r363545 - [libunwind][AArch64] Fix libunwind::Registers_arm64::jumpto

2019-06-17 Thread Mikhail Maltsev via cfe-commits
Author: miyuki
Date: Mon Jun 17 04:00:21 2019
New Revision: 363545

URL: http://llvm.org/viewvc/llvm-project?rev=363545&view=rev
Log:
[libunwind][AArch64] Fix libunwind::Registers_arm64::jumpto

Summary:
The AArch64 version of the libunwind function which restores the
CPU state and resumes execution is not interrupt-safe. It restores
the target value of SP before loading the floating-point registers
from the context struct, but that struct is allocated on the stack
which is being deallocated. This means that if an interrupt occurs
during this function, and uses a lot of stack space, it could
overwrite the values about to be loaded into the floating-point
registers.

This patch fixes the issue.

Patch by Oliver Stannard.

Reviewers: phosek, chill

Reviewed By: chill

Subscribers: chill, javed.absar, kristof.beyls, christof, LukeCheeseman, 
pbarrio, olista01, libcxx-commits

Tags: #libc

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

Modified:
libunwind/trunk/src/UnwindRegistersRestore.S

Modified: libunwind/trunk/src/UnwindRegistersRestore.S
URL: 
http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/UnwindRegistersRestore.S?rev=363545&r1=363544&r2=363545&view=diff
==
--- libunwind/trunk/src/UnwindRegistersRestore.S (original)
+++ libunwind/trunk/src/UnwindRegistersRestore.S Mon Jun 17 04:00:21 2019
@@ -575,7 +575,8 @@ DEFINE_LIBUNWIND_FUNCTION(_ZN9libunwind1
   ldpx10,x11, [x0, #0x050]
   ldpx12,x13, [x0, #0x060]
   ldpx14,x15, [x0, #0x070]
-  ldpx16,x17, [x0, #0x080]
+  // x16 and x17 were clobbered by the call into the unwinder, so no point in
+  // restoring them.
   ldpx18,x19, [x0, #0x090]
   ldpx20,x21, [x0, #0x0A0]
   ldpx22,x23, [x0, #0x0B0]
@@ -583,8 +584,6 @@ DEFINE_LIBUNWIND_FUNCTION(_ZN9libunwind1
   ldpx26,x27, [x0, #0x0D0]
   ldpx28,x29, [x0, #0x0E0]
   ldrx30, [x0, #0x100]  // restore pc into lr
-  ldrx1,  [x0, #0x0F8]
-  movsp,x1  // restore sp
 
   ldpd0, d1,  [x0, #0x110]
   ldpd2, d3,  [x0, #0x120]
@@ -604,7 +603,13 @@ DEFINE_LIBUNWIND_FUNCTION(_ZN9libunwind1
   ldrd30, [x0, #0x200]
   ldrd31, [x0, #0x208]
 
+  // Finally, restore sp. This must be done after the the last read from the
+  // context struct, because it is allocated on the stack, and an exception
+  // could clobber the de-allocated portion of the stack after sp has been
+  // restored.
+  ldrx16, [x0, #0x0F8]
   ldpx0, x1,  [x0, #0x000]  // restore x0,x1
+  movsp,x16 // restore sp
   retx30// jump to pc
 
 #elif defined(__arm__) && !defined(__APPLE__)


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


[PATCH] D63387: clang: Promote -fdebug-compilation-dir from cc1 flag to driver-level flag

2019-06-17 Thread Hans Wennborg via Phabricator via cfe-commits
hans accepted this revision.
hans added a comment.
This revision is now accepted and ready to land.

lgtm




Comment at: clang/include/clang/Driver/Options.td:717
+def fdebug_compilation_dir : Separate<["-"], "fdebug-compilation-dir">,
+Group, Flags<[CC1Option, CC1AsOption, CoreOption]>,
+HelpText<"The compilation directory to embed in the debug info.">;

Maybe mention in the commit message that it's not just a driver-level option, 
but also available in clang-cl (which makes sense I think).


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

https://reviews.llvm.org/D63387



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


[PATCH] D63335: [HIP] Add the interface deriving the stub name of device kernels.

2019-06-17 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl accepted this revision.
yaxunl added a comment.

LGTM. Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63335



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


[PATCH] D62944: [Driver] Fix wchar_t and wint_t definitions on Solaris

2019-06-17 Thread Rainer Orth via Phabricator via cfe-commits
ro added a comment.

In D62944#1542217 , @efriedma wrote:

> For format-strings.c, I'm not really happy suggesting `#if defined(__sun) && 
> !defined(__LP64__)`, but I don't think the alternative is better.  We could 
> restrict the test so it doesn't run using a Solaris target triple, but we 
> actually want coverage here: the difference in wint_t affects the semantics 
> of "%lc", so we want some coverage of that path.


Agreed.  While gcc supports quite a number of targets with long int for 
wchar_t/wint_t, Solaris is the only one in clang.  I've now adapted
the test accordingly.  Should there be more in the future, we can look for a 
more generic solution.

Tested on `i386-pc-solaris2.11`, `x86_64-pc-solaris2.11`, and 
`x86_64-pc-linux-gnu`.  Ok for trunk now?


Repository:
  rC Clang

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

https://reviews.llvm.org/D62944



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


[PATCH] D63417: [WIP][RISCV] Specify registers used for exception handling

2019-06-17 Thread Edward Jones via Phabricator via cfe-commits
edward-jones created this revision.
edward-jones added reviewers: asb, lewis-revill.
Herald added subscribers: cfe-commits, Jim, benna, psnobl, jocewei, PkmX, 
rkruppe, the_o, brucehoult, MartinMosbeck, rogfer01, zzheng, jrtc27, shiva0217, 
kito-cheng, niosHD, sabuasal, apazos, simoncook, johnrusso, rbar.
Herald added a project: clang.

Implements the handling of __builtin_eh_return_regno()

Work in progress as this is missing tests


Repository:
  rC Clang

https://reviews.llvm.org/D63417

Files:
  lib/Basic/Targets/RISCV.h


Index: lib/Basic/Targets/RISCV.h
===
--- lib/Basic/Targets/RISCV.h
+++ lib/Basic/Targets/RISCV.h
@@ -58,6 +58,15 @@
 
   ArrayRef getGCCRegNames() const override;
 
+  int getEHDataRegisterNumber(unsigned RegNo) const override {
+if (RegNo == 0)
+  return 10;
+else if (RegNo == 1)
+  return 11;
+else
+  return -1;
+  }
+
   ArrayRef getGCCRegAliases() const override;
 
   bool validateAsmConstraint(const char *&Name,


Index: lib/Basic/Targets/RISCV.h
===
--- lib/Basic/Targets/RISCV.h
+++ lib/Basic/Targets/RISCV.h
@@ -58,6 +58,15 @@
 
   ArrayRef getGCCRegNames() const override;
 
+  int getEHDataRegisterNumber(unsigned RegNo) const override {
+if (RegNo == 0)
+  return 10;
+else if (RegNo == 1)
+  return 11;
+else
+  return -1;
+  }
+
   ArrayRef getGCCRegAliases() const override;
 
   bool validateAsmConstraint(const char *&Name,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r363548 - Promote -fdebug-compilation-dir from a cc1 flag to clang and clang-cl driver flags

2019-06-17 Thread Nico Weber via cfe-commits
Author: nico
Date: Mon Jun 17 05:10:40 2019
New Revision: 363548

URL: http://llvm.org/viewvc/llvm-project?rev=363548&view=rev
Log:
Promote -fdebug-compilation-dir from a cc1 flag to clang and clang-cl driver 
flags

The flag is useful when wanting to create .o files that are independent
from the absolute path to the build directory. -fdebug-prefix-map= can
be used to the same effect, but it requires putting the absolute path
to the build directory on the build command line, so it still requires
the build command line to be dependent on the absolute path of the build
directory. With this flag, "-fdebug-compilation-dir ." makes it so that
both debug info and the compile command itself are independent of the
absolute path of the build directory, which is good for build
determinism (in the sense that the build is independent of which
directory it happens in) and for caching compile results.
(The tradeoff is that the debugger needs explicit configuration to know
the build directory. See also http://dwarfstd.org/ShowIssue.php?issue=171130.2)

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

Modified:
cfe/trunk/include/clang/Driver/CC1Options.td
cfe/trunk/include/clang/Driver/Options.td
cfe/trunk/lib/Driver/ToolChains/Clang.cpp
cfe/trunk/test/Driver/cl-options.c
cfe/trunk/test/Driver/clang_f_opts.c

Modified: cfe/trunk/include/clang/Driver/CC1Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/CC1Options.td?rev=363548&r1=363547&r2=363548&view=diff
==
--- cfe/trunk/include/clang/Driver/CC1Options.td (original)
+++ cfe/trunk/include/clang/Driver/CC1Options.td Mon Jun 17 05:10:40 2019
@@ -190,8 +190,6 @@ def default_function_attr : Separate<["-
   HelpText<"Apply given attribute to all functions">;
 def dwarf_version_EQ : Joined<["-"], "dwarf-version=">;
 def debugger_tuning_EQ : Joined<["-"], "debugger-tuning=">;
-def fdebug_compilation_dir : Separate<["-"], "fdebug-compilation-dir">,
-  HelpText<"The compilation directory to embed in the debug info.">;
 def dwarf_debug_flags : Separate<["-"], "dwarf-debug-flags">,
   HelpText<"The string to embed in the Dwarf debug flags record.">;
 def record_command_line : Separate<["-"], "record-command-line">,

Modified: cfe/trunk/include/clang/Driver/Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=363548&r1=363547&r2=363548&view=diff
==
--- cfe/trunk/include/clang/Driver/Options.td (original)
+++ cfe/trunk/include/clang/Driver/Options.td Mon Jun 17 05:10:40 2019
@@ -713,11 +713,14 @@ def fauto_profile_accurate : Flag<["-"],
 Group, Alias;
 def fno_auto_profile_accurate : Flag<["-"], "fno-auto-profile-accurate">,
 Group, Alias;
-def fdebug_info_for_profiling : Flag<["-"], "fdebug-info-for-profiling">, 
Group,
-Flags<[CC1Option]>,
+def fdebug_compilation_dir : Separate<["-"], "fdebug-compilation-dir">,
+Group, Flags<[CC1Option, CC1AsOption, CoreOption]>,
+HelpText<"The compilation directory to embed in the debug info.">;
+def fdebug_info_for_profiling : Flag<["-"], "fdebug-info-for-profiling">,
+Group, Flags<[CC1Option]>,
 HelpText<"Emit extra debug info to make sample profile more accurate.">;
-def fno_debug_info_for_profiling : Flag<["-"], 
"fno-debug-info-for-profiling">, Group,
-Flags<[DriverOption]>,
+def fno_debug_info_for_profiling : Flag<["-"], "fno-debug-info-for-profiling">,
+Group, Flags<[DriverOption]>,
 HelpText<"Do not emit extra debug info for sample profiler.">;
 def fprofile_instr_generate : Flag<["-"], "fprofile-instr-generate">,
 Group, Flags<[CoreOption]>,

Modified: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Clang.cpp?rev=363548&r1=363547&r2=363548&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp Mon Jun 17 05:10:40 2019
@@ -618,7 +618,11 @@ static bool shouldUseLeafFramePointer(co
 /// Add a CC1 option to specify the debug compilation directory.
 static void addDebugCompDirArg(const ArgList &Args, ArgStringList &CmdArgs,
const llvm::vfs::FileSystem &VFS) {
-  if (llvm::ErrorOr CWD = VFS.getCurrentWorkingDirectory()) {
+  if (Arg *A = Args.getLastArg(options::OPT_fdebug_compilation_dir)) {
+CmdArgs.push_back("-fdebug-compilation-dir");
+CmdArgs.push_back(A->getValue());
+  } else if (llvm::ErrorOr CWD =
+ VFS.getCurrentWorkingDirectory()) {
 CmdArgs.push_back("-fdebug-compilation-dir");
 CmdArgs.push_back(Args.MakeArgString(*CWD));
   }
@@ -637,7 +641,8 @@ static void addDebugPrefixMapArg(const D
 }
 
 /// Vectorize at all optimization levels greater than 1 except for -Oz.
-/// For

[PATCH] D63387: clang: Promote -fdebug-compilation-dir from cc1 flag to driver-level flag

2019-06-17 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL363548: Promote -fdebug-compilation-dir from a cc1 flag to 
clang and clang-cl driver… (authored by nico, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D63387?vs=204956&id=205041#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D63387

Files:
  cfe/trunk/include/clang/Driver/CC1Options.td
  cfe/trunk/include/clang/Driver/Options.td
  cfe/trunk/lib/Driver/ToolChains/Clang.cpp
  cfe/trunk/test/Driver/cl-options.c
  cfe/trunk/test/Driver/clang_f_opts.c


Index: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp
@@ -618,7 +618,11 @@
 /// Add a CC1 option to specify the debug compilation directory.
 static void addDebugCompDirArg(const ArgList &Args, ArgStringList &CmdArgs,
const llvm::vfs::FileSystem &VFS) {
-  if (llvm::ErrorOr CWD = VFS.getCurrentWorkingDirectory()) {
+  if (Arg *A = Args.getLastArg(options::OPT_fdebug_compilation_dir)) {
+CmdArgs.push_back("-fdebug-compilation-dir");
+CmdArgs.push_back(A->getValue());
+  } else if (llvm::ErrorOr CWD =
+ VFS.getCurrentWorkingDirectory()) {
 CmdArgs.push_back("-fdebug-compilation-dir");
 CmdArgs.push_back(Args.MakeArgString(*CWD));
   }
@@ -637,7 +641,8 @@
 }
 
 /// Vectorize at all optimization levels greater than 1 except for -Oz.
-/// For -Oz the loop vectorizer is disable, while the slp vectorizer is 
enabled.
+/// For -Oz the loop vectorizer is disabled, while the slp vectorizer is
+/// enabled.
 static bool shouldEnableVectorizerAtOLevel(const ArgList &Args, bool isSlpVec) 
{
   if (Arg *A = Args.getLastArg(options::OPT_O_Group)) {
 if (A->getOption().matches(options::OPT_O4) ||
Index: cfe/trunk/include/clang/Driver/Options.td
===
--- cfe/trunk/include/clang/Driver/Options.td
+++ cfe/trunk/include/clang/Driver/Options.td
@@ -713,11 +713,14 @@
 Group, Alias;
 def fno_auto_profile_accurate : Flag<["-"], "fno-auto-profile-accurate">,
 Group, Alias;
-def fdebug_info_for_profiling : Flag<["-"], "fdebug-info-for-profiling">, 
Group,
-Flags<[CC1Option]>,
+def fdebug_compilation_dir : Separate<["-"], "fdebug-compilation-dir">,
+Group, Flags<[CC1Option, CC1AsOption, CoreOption]>,
+HelpText<"The compilation directory to embed in the debug info.">;
+def fdebug_info_for_profiling : Flag<["-"], "fdebug-info-for-profiling">,
+Group, Flags<[CC1Option]>,
 HelpText<"Emit extra debug info to make sample profile more accurate.">;
-def fno_debug_info_for_profiling : Flag<["-"], 
"fno-debug-info-for-profiling">, Group,
-Flags<[DriverOption]>,
+def fno_debug_info_for_profiling : Flag<["-"], "fno-debug-info-for-profiling">,
+Group, Flags<[DriverOption]>,
 HelpText<"Do not emit extra debug info for sample profiler.">;
 def fprofile_instr_generate : Flag<["-"], "fprofile-instr-generate">,
 Group, Flags<[CoreOption]>,
Index: cfe/trunk/include/clang/Driver/CC1Options.td
===
--- cfe/trunk/include/clang/Driver/CC1Options.td
+++ cfe/trunk/include/clang/Driver/CC1Options.td
@@ -190,8 +190,6 @@
   HelpText<"Apply given attribute to all functions">;
 def dwarf_version_EQ : Joined<["-"], "dwarf-version=">;
 def debugger_tuning_EQ : Joined<["-"], "debugger-tuning=">;
-def fdebug_compilation_dir : Separate<["-"], "fdebug-compilation-dir">,
-  HelpText<"The compilation directory to embed in the debug info.">;
 def dwarf_debug_flags : Separate<["-"], "dwarf-debug-flags">,
   HelpText<"The string to embed in the Dwarf debug flags record.">;
 def record_command_line : Separate<["-"], "record-command-line">,
Index: cfe/trunk/test/Driver/cl-options.c
===
--- cfe/trunk/test/Driver/cl-options.c
+++ cfe/trunk/test/Driver/cl-options.c
@@ -619,6 +619,7 @@
 // RUN: -fno-coverage-mapping \
 // RUN: -fdiagnostics-color \
 // RUN: -fno-diagnostics-color \
+// RUN: -fdebug-compilation-dir . \
 // RUN: -fdiagnostics-parseable-fixits \
 // RUN: -fdiagnostics-absolute-paths \
 // RUN: -ferror-limit=10 \
Index: cfe/trunk/test/Driver/clang_f_opts.c
===
--- cfe/trunk/test/Driver/clang_f_opts.c
+++ cfe/trunk/test/Driver/clang_f_opts.c
@@ -525,11 +525,15 @@
 // CHECK-CF-PROTECTION-BRANCH: -fcf-protection=branch
 // CHECK-NO-CF-PROTECTION-BRANCH-NOT: -fcf-protection=branch
 
+// RUN: %clang -### -S -fdebug-compilation-dir . %s 2>&1 | FileCheck 
-check-prefix=CHECK-DEBUG-COMPILATION-DIR %s
+// RUN: %clang -### -fdebug-compila

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-06-17 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 created this revision.
xbolva00 added reviewers: jfb, rsmith.
Herald added subscribers: cfe-commits, dexonsmith.
Herald added a project: clang.

Motivation:
https://twitter.com/jfbastien/status/1139298419988549632
https://twitter.com/mikemx7f/status/1139335901790625793
https://codesearch.isocpp.org/cgi-bin/cgi_ppsearch?q=10+%5E&search=Search


Repository:
  rC Clang

https://reviews.llvm.org/D63423

Files:
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaExpr.cpp
  test/SemaCXX/warn-xor-as-pow.cpp

Index: test/SemaCXX/warn-xor-as-pow.cpp
===
--- test/SemaCXX/warn-xor-as-pow.cpp
+++ test/SemaCXX/warn-xor-as-pow.cpp
@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 -x c++ -fsyntax-only -verify -Wxor-as-pow %s
+// RUN: %clang_cc1 -x c++ -fsyntax-only -verify %s
+// RUN: %clang_cc1 -x c++ -fsyntax-only -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+void test(int a, int b) {
+int res;
+res = a ^ 5;
+res = 2 ^ b;
+res = a ^ b;
+res = 2 ^ 0;
+res = 2 ^ 1; // expected-warning {{result of '2 ^ 1' is 3, maybe you mean '1<<1' (2)?}}
+// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:16}:"1<<1"
+res = 2 ^ 8; // expected-warning {{result of '2 ^ 8' is 10, maybe you mean '1<<8' (256)?}}
+// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:16}:"1<<8"
+res = 2 ^ 32; // expected-warning {{result of '2 ^ 32' is 34, maybe you mean '1<<32' (0)?}}
+// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:17}:"1<<32"
+res = 0b10 ^ 32;
+res = 2 ^ 0b100;
+res = 2 xor 32;
+
+res = 10 ^ 0;
+res = 10 ^ 1; // expected-warning {{result of '10 ^ 1' is 11, maybe you mean '10'?}}
+// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:17}:"10"
+res = 10 ^ 10; // expected-warning {{result of '10 ^ 10' is 0, maybe you mean '100'?}}
+// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:18}:"100"
+}
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -10890,6 +10890,47 @@
   return GetSignedVectorType(vType);
 }
 
+static void diagnoseXorMisusedAsPow(Sema &S, ExprResult &LHS, ExprResult &RHS,
+ SourceLocation Loc) {
+  auto *LHSInt = dyn_cast(LHS.get());
+  auto *RHSInt = dyn_cast(RHS.get());
+  if (!LHSInt || !RHSInt)
+return;
+
+  CharSourceRange OpRange = CharSourceRange::getCharRange(
+  LHSInt->getBeginLoc(), S.getLocForEndOfToken(RHSInt->getLocation()));
+  llvm::StringRef ExprStr =
+  Lexer::getSourceText(OpRange, S.getSourceManager(), S.getLangOpts());
+
+  if (S.getLangOpts().CPlusPlus) {
+// Do not diagnose binary literals
+if (ExprStr.find("0b") != llvm::StringRef::npos)
+  return;
+// Do not diagnose if xor keyword is used
+if (ExprStr.find("xor") != llvm::StringRef::npos)
+  return;
+  }
+
+  int64_t RightSideValue = RHSInt->getValue().getSExtValue();
+  if (RightSideValue < 1)
+return;
+
+  llvm::APInt XorValue = LHSInt->getValue() ^ RHSInt->getValue();
+  if (LHSInt->getValue() == 2) {
+llvm::APInt PowValue = (LHSInt->getValue() - 1) << RHSInt->getValue();
+std::string SuggestedExpr = "1<<" + RHSInt->getValue().toString(10, true);
+S.Diag(Loc, diag::warn_xor_used_as_pow_base_two)
+<< ExprStr << XorValue.toString(10, true) << SuggestedExpr
+<< PowValue.toString(10, true)
+<< FixItHint::CreateReplacement(OpRange, SuggestedExpr);
+  } else if (LHSInt->getValue() == 10) {
+std::string SuggestedValue = "1" + std::string(RightSideValue, '0');
+S.Diag(Loc, diag::warn_xor_used_as_pow_base_ten)
+<< ExprStr << XorValue.toString(10, true) << SuggestedValue
+<< FixItHint::CreateReplacement(OpRange, SuggestedValue);
+  }
+}
+
 QualType Sema::CheckVectorLogicalOperands(ExprResult &LHS, ExprResult &RHS,
   SourceLocation Loc) {
   // Ensure that either both operands are of the same vector type, or
@@ -10933,6 +10974,9 @@
   if (Opc == BO_And)
 diagnoseLogicalNotOnLHSofCheck(*this, LHS, RHS, Loc, Opc);
 
+  if (Opc == BO_Xor)
+diagnoseXorMisusedAsPow(*this, LHS, RHS, Loc);
+
   ExprResult LHSResult = LHS, RHSResult = RHS;
   QualType compType = UsualArithmeticConversions(LHSResult, RHSResult,
  IsCompAssign);
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -3302,6 +3302,14 @@
   "code; pointer may be assumed to always convert to true">,
   InGroup;
 
+def warn_xor_used_as_pow_base_two : Warning<
+  "result of '%0' is %1, maybe you mean '%2' (%3)?">,
+  InGroup;
+
+def warn_xor_used_as_pow_base_ten : Warning<
+  "result of '%0' is %1, maybe you m

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-06-17 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 marked an inline comment as done.
xbolva00 added inline comments.



Comment at: include/clang/Basic/DiagnosticGroups.td:508
 def Varargs : DiagGroup<"varargs">;
+def XorAsPow : DiagGroup<"xor-as-pow">;
 

GCC folks will do this diagnostic soon too, so I will wait and use their name.


Repository:
  rC Clang

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

https://reviews.llvm.org/D63423



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


r363553 - [HIP] Add the interface deriving the stub name of device kernels.

2019-06-17 Thread Michael Liao via cfe-commits
Author: hliao
Date: Mon Jun 17 05:51:36 2019
New Revision: 363553

URL: http://llvm.org/viewvc/llvm-project?rev=363553&view=rev
Log:
[HIP] Add the interface deriving the stub name of device kernels.

Summary:
- Revise the interface to derive the stub name and simplify the
  assertion of it.

Reviewers: yaxunl, tra

Subscribers: cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/lib/CodeGen/CGCUDANV.cpp
cfe/trunk/lib/CodeGen/CGCUDARuntime.h
cfe/trunk/lib/CodeGen/CodeGenModule.cpp

Modified: cfe/trunk/lib/CodeGen/CGCUDANV.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCUDANV.cpp?rev=363553&r1=363552&r2=363553&view=diff
==
--- cfe/trunk/lib/CodeGen/CGCUDANV.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGCUDANV.cpp Mon Jun 17 05:51:36 2019
@@ -132,6 +132,8 @@ public:
   llvm::Function *makeModuleCtorFunction() override;
   /// Creates module destructor function
   llvm::Function *makeModuleDtorFunction() override;
+  /// Construct and return the stub name of a kernel.
+  std::string getDeviceStubName(llvm::StringRef Name) const override;
 };
 
 }
@@ -217,10 +219,20 @@ std::string CGNVCUDARuntime::getDeviceSi
 
 void CGNVCUDARuntime::emitDeviceStub(CodeGenFunction &CGF,
  FunctionArgList &Args) {
-  assert(getDeviceSideName(CGF.CurFuncDecl) == CGF.CurFn->getName() ||
- getDeviceSideName(CGF.CurFuncDecl) + ".stub" == CGF.CurFn->getName() 
||
- CGF.CGM.getContext().getTargetInfo().getCXXABI() !=
- CGF.CGM.getContext().getAuxTargetInfo()->getCXXABI());
+  // Ensure either we have different ABIs between host and device compilations,
+  // says host compilation following MSVC ABI but device compilation follows
+  // Itanium C++ ABI or, if they follow the same ABI, kernel names after
+  // mangling should be the same after name stubbing. The later checking is
+  // very important as the device kernel name being mangled in host-compilation
+  // is used to resolve the device binaries to be executed. Inconsistent naming
+  // result in undefined behavior. Even though we cannot check that naming
+  // directly between host- and device-compilations, the host- and
+  // device-mangling in host compilation could help catching certain ones.
+  assert((CGF.CGM.getContext().getAuxTargetInfo() &&
+  (CGF.CGM.getContext().getAuxTargetInfo()->getCXXABI() !=
+   CGF.CGM.getContext().getTargetInfo().getCXXABI())) ||
+ getDeviceStubName(getDeviceSideName(CGF.CurFuncDecl)) ==
+ CGF.CurFn->getName());
 
   EmittedKernels.push_back({CGF.CurFn, CGF.CurFuncDecl});
   if (CudaFeatureEnabled(CGM.getTarget().getSDKVersion(),
@@ -780,6 +792,12 @@ llvm::Function *CGNVCUDARuntime::makeMod
   return ModuleDtorFunc;
 }
 
+std::string CGNVCUDARuntime::getDeviceStubName(llvm::StringRef Name) const {
+  if (!CGM.getLangOpts().HIP)
+return Name;
+  return std::move((Name + ".stub").str());
+}
+
 CGCUDARuntime *CodeGen::CreateNVCUDARuntime(CodeGenModule &CGM) {
   return new CGNVCUDARuntime(CGM);
 }

Modified: cfe/trunk/lib/CodeGen/CGCUDARuntime.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCUDARuntime.h?rev=363553&r1=363552&r2=363553&view=diff
==
--- cfe/trunk/lib/CodeGen/CGCUDARuntime.h (original)
+++ cfe/trunk/lib/CodeGen/CGCUDARuntime.h Mon Jun 17 05:51:36 2019
@@ -15,6 +15,8 @@
 #ifndef LLVM_CLANG_LIB_CODEGEN_CGCUDARUNTIME_H
 #define LLVM_CLANG_LIB_CODEGEN_CGCUDARUNTIME_H
 
+#include "llvm/ADT/StringRef.h"
+
 namespace llvm {
 class Function;
 class GlobalVariable;
@@ -63,6 +65,9 @@ public:
   /// Returns a module cleanup function or nullptr if it's not needed.
   /// Must be called after ModuleCtorFunction
   virtual llvm::Function *makeModuleDtorFunction() = 0;
+
+  /// Construct and return the stub name of a kernel.
+  virtual std::string getDeviceStubName(llvm::StringRef Name) const = 0;
 };
 
 /// Creates an instance of a CUDA runtime class.

Modified: cfe/trunk/lib/CodeGen/CodeGenModule.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.cpp?rev=363553&r1=363552&r2=363553&view=diff
==
--- cfe/trunk/lib/CodeGen/CodeGenModule.cpp (original)
+++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp Mon Jun 17 05:51:36 2019
@@ -1088,13 +1088,11 @@ StringRef CodeGenModule::getMangledName(
   const auto *ND = cast(GD.getDecl());
   std::string MangledName = getMangledNameImpl(*this, GD, ND);
 
-  // Postfix kernel stub names with .stub to differentiate them from kernel
-  // names in device binaries. This is to facilitate the debugger to find
-  // the correct symbols for kernels in the device binary.
+  // Adjust kernel stub mangling as we may need to be able to differentiate
+  // them from the ker

[PATCH] D63335: [HIP] Add the interface deriving the stub name of device kernels.

2019-06-17 Thread Michael Liao via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL363553: [HIP] Add the interface deriving the stub name of 
device kernels. (authored by hliao, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D63335?vs=204856&id=205051#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D63335

Files:
  cfe/trunk/lib/CodeGen/CGCUDANV.cpp
  cfe/trunk/lib/CodeGen/CGCUDARuntime.h
  cfe/trunk/lib/CodeGen/CodeGenModule.cpp


Index: cfe/trunk/lib/CodeGen/CGCUDARuntime.h
===
--- cfe/trunk/lib/CodeGen/CGCUDARuntime.h
+++ cfe/trunk/lib/CodeGen/CGCUDARuntime.h
@@ -15,6 +15,8 @@
 #ifndef LLVM_CLANG_LIB_CODEGEN_CGCUDARUNTIME_H
 #define LLVM_CLANG_LIB_CODEGEN_CGCUDARUNTIME_H
 
+#include "llvm/ADT/StringRef.h"
+
 namespace llvm {
 class Function;
 class GlobalVariable;
@@ -63,6 +65,9 @@
   /// Returns a module cleanup function or nullptr if it's not needed.
   /// Must be called after ModuleCtorFunction
   virtual llvm::Function *makeModuleDtorFunction() = 0;
+
+  /// Construct and return the stub name of a kernel.
+  virtual std::string getDeviceStubName(llvm::StringRef Name) const = 0;
 };
 
 /// Creates an instance of a CUDA runtime class.
Index: cfe/trunk/lib/CodeGen/CodeGenModule.cpp
===
--- cfe/trunk/lib/CodeGen/CodeGenModule.cpp
+++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp
@@ -1088,13 +1088,11 @@
   const auto *ND = cast(GD.getDecl());
   std::string MangledName = getMangledNameImpl(*this, GD, ND);
 
-  // Postfix kernel stub names with .stub to differentiate them from kernel
-  // names in device binaries. This is to facilitate the debugger to find
-  // the correct symbols for kernels in the device binary.
+  // Adjust kernel stub mangling as we may need to be able to differentiate
+  // them from the kernel itself (e.g., for HIP).
   if (auto *FD = dyn_cast(GD.getDecl()))
-if (getLangOpts().HIP && !getLangOpts().CUDAIsDevice &&
-FD->hasAttr())
-  MangledName = MangledName + ".stub";
+if (!getLangOpts().CUDAIsDevice && FD->hasAttr())
+  MangledName = getCUDARuntime().getDeviceStubName(MangledName);
 
   auto Result = Manglings.insert(std::make_pair(MangledName, GD));
   return MangledDeclNames[CanonicalGD] = Result.first->first();
Index: cfe/trunk/lib/CodeGen/CGCUDANV.cpp
===
--- cfe/trunk/lib/CodeGen/CGCUDANV.cpp
+++ cfe/trunk/lib/CodeGen/CGCUDANV.cpp
@@ -132,6 +132,8 @@
   llvm::Function *makeModuleCtorFunction() override;
   /// Creates module destructor function
   llvm::Function *makeModuleDtorFunction() override;
+  /// Construct and return the stub name of a kernel.
+  std::string getDeviceStubName(llvm::StringRef Name) const override;
 };
 
 }
@@ -217,10 +219,20 @@
 
 void CGNVCUDARuntime::emitDeviceStub(CodeGenFunction &CGF,
  FunctionArgList &Args) {
-  assert(getDeviceSideName(CGF.CurFuncDecl) == CGF.CurFn->getName() ||
- getDeviceSideName(CGF.CurFuncDecl) + ".stub" == CGF.CurFn->getName() 
||
- CGF.CGM.getContext().getTargetInfo().getCXXABI() !=
- CGF.CGM.getContext().getAuxTargetInfo()->getCXXABI());
+  // Ensure either we have different ABIs between host and device compilations,
+  // says host compilation following MSVC ABI but device compilation follows
+  // Itanium C++ ABI or, if they follow the same ABI, kernel names after
+  // mangling should be the same after name stubbing. The later checking is
+  // very important as the device kernel name being mangled in host-compilation
+  // is used to resolve the device binaries to be executed. Inconsistent naming
+  // result in undefined behavior. Even though we cannot check that naming
+  // directly between host- and device-compilations, the host- and
+  // device-mangling in host compilation could help catching certain ones.
+  assert((CGF.CGM.getContext().getAuxTargetInfo() &&
+  (CGF.CGM.getContext().getAuxTargetInfo()->getCXXABI() !=
+   CGF.CGM.getContext().getTargetInfo().getCXXABI())) ||
+ getDeviceStubName(getDeviceSideName(CGF.CurFuncDecl)) ==
+ CGF.CurFn->getName());
 
   EmittedKernels.push_back({CGF.CurFn, CGF.CurFuncDecl});
   if (CudaFeatureEnabled(CGM.getTarget().getSDKVersion(),
@@ -780,6 +792,12 @@
   return ModuleDtorFunc;
 }
 
+std::string CGNVCUDARuntime::getDeviceStubName(llvm::StringRef Name) const {
+  if (!CGM.getLangOpts().HIP)
+return Name;
+  return std::move((Name + ".stub").str());
+}
+
 CGCUDARuntime *CodeGen::CreateNVCUDARuntime(CodeGenModule &CGM) {
   return new CGNVCUDARuntime(CGM);
 }


Index: cfe/trunk/lib/CodeGen/CGCUDARuntime.h
===
--- cfe/trunk/lib/

[PATCH] D63425: [clangd] Perform merge for main file symbols.

2019-06-17 Thread Haojian Wu via Phabricator via cfe-commits
hokein created this revision.
hokein added a reviewer: kadircet.
Herald added subscribers: arphaman, jkorous, MaskRay, ilya-biryukov.
Herald added a project: clang.

Previously, we randomly pick one main file symbol in dynamic index, we
may loose the ideal symbol (with definition location) in the index.

It fixes the issue where sometimes we fail to go to the symbol definition, see:

1. call go-to-decl on Foo in Foo.cpp
2. jump to Foo.h, call go-to-def on Foo in Foo.h

we can't go back to Foo.cpp -- because we open Foo.cpp, Foo.h in clangd, both
files have Foo symbol (one with def&decl, one with decl only), we randomely
choose one.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D63425

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


Index: clang-tools-extra/clangd/unittests/FileIndexTests.cpp
===
--- clang-tools-extra/clangd/unittests/FileIndexTests.cpp
+++ clang-tools-extra/clangd/unittests/FileIndexTests.cpp
@@ -46,6 +46,7 @@
 }
 MATCHER_P(QName, N, "") { return (arg.Scope + arg.Name).str() == N; }
 MATCHER_P(NumReferences, N, "") { return arg.References == N; }
+MATCHER_P(hasOrign, O, "") { return bool(arg.Origin & O); }
 
 namespace clang {
 namespace clangd {
@@ -367,6 +368,27 @@
   RefsAre({RefRange(Main.range())}));
 }
 
+TEST(FileIndexTest, MergeMainFileSymbols) {
+  const char* CommonHeader = "void foo();";
+  TestTU Header = TestTU::withCode(CommonHeader);
+  TestTU Cpp = TestTU::withCode("void foo() {}");
+  Cpp.Filename = "foo.cpp";
+  Cpp.HeaderFilename = "foo.h";
+  Cpp.HeaderCode = CommonHeader;
+
+  FileIndex Index;
+  auto HeaderAST = Header.build();
+  auto CppAST = Cpp.build();
+  Index.updateMain(testPath("foo.h"), HeaderAST);
+  Index.updateMain(testPath("foo.cpp"), CppAST);
+
+  auto Symbols = runFuzzyFind(Index, "");
+  // Check foo is merged, foo in Cpp wins (as we see the definition there).
+  EXPECT_THAT(Symbols, ElementsAre(AllOf(DeclURI("unittest:///foo.h"),
+ DefURI("unittest:///foo.cpp"),
+ hasOrign(SymbolOrigin::Merge;
+}
+
 TEST(FileSymbolsTest, CountReferencesNoRefSlabs) {
   FileSymbols FS;
   FS.update("f1", numSlab(1, 3), nullptr, true);
Index: clang-tools-extra/clangd/index/FileIndex.cpp
===
--- clang-tools-extra/clangd/index/FileIndex.cpp
+++ clang-tools-extra/clangd/index/FileIndex.cpp
@@ -236,7 +236,7 @@
   llvm::make_unique(std::move(Contents.second)),
   /*CountReferences=*/true);
   MainFileIndex.reset(
-  MainFileSymbols.buildIndex(IndexType::Light, 
DuplicateHandling::PickOne));
+  MainFileSymbols.buildIndex(IndexType::Light, DuplicateHandling::Merge));
 }
 
 } // namespace clangd


Index: clang-tools-extra/clangd/unittests/FileIndexTests.cpp
===
--- clang-tools-extra/clangd/unittests/FileIndexTests.cpp
+++ clang-tools-extra/clangd/unittests/FileIndexTests.cpp
@@ -46,6 +46,7 @@
 }
 MATCHER_P(QName, N, "") { return (arg.Scope + arg.Name).str() == N; }
 MATCHER_P(NumReferences, N, "") { return arg.References == N; }
+MATCHER_P(hasOrign, O, "") { return bool(arg.Origin & O); }
 
 namespace clang {
 namespace clangd {
@@ -367,6 +368,27 @@
   RefsAre({RefRange(Main.range())}));
 }
 
+TEST(FileIndexTest, MergeMainFileSymbols) {
+  const char* CommonHeader = "void foo();";
+  TestTU Header = TestTU::withCode(CommonHeader);
+  TestTU Cpp = TestTU::withCode("void foo() {}");
+  Cpp.Filename = "foo.cpp";
+  Cpp.HeaderFilename = "foo.h";
+  Cpp.HeaderCode = CommonHeader;
+
+  FileIndex Index;
+  auto HeaderAST = Header.build();
+  auto CppAST = Cpp.build();
+  Index.updateMain(testPath("foo.h"), HeaderAST);
+  Index.updateMain(testPath("foo.cpp"), CppAST);
+
+  auto Symbols = runFuzzyFind(Index, "");
+  // Check foo is merged, foo in Cpp wins (as we see the definition there).
+  EXPECT_THAT(Symbols, ElementsAre(AllOf(DeclURI("unittest:///foo.h"),
+ DefURI("unittest:///foo.cpp"),
+ hasOrign(SymbolOrigin::Merge;
+}
+
 TEST(FileSymbolsTest, CountReferencesNoRefSlabs) {
   FileSymbols FS;
   FS.update("f1", numSlab(1, 3), nullptr, true);
Index: clang-tools-extra/clangd/index/FileIndex.cpp
===
--- clang-tools-extra/clangd/index/FileIndex.cpp
+++ clang-tools-extra/clangd/index/FileIndex.cpp
@@ -236,7 +236,7 @@
   llvm::make_unique(std::move(Contents.second)),
   /*CountReferences=*/true);
   MainFileIndex.reset(
-  MainFileSymbols.buildIndex(IndexType::Light, DuplicateHandling::PickOne));
+  MainFileSymbols.buildIndex(IndexType::Light, DuplicateHandling::Merge));
 }
 
 } // namespace clangd

[clang-tools-extra] r363554 - [clangd] Detect C++ for extension-less source files in vscode extension

2019-06-17 Thread Haojian Wu via cfe-commits
Author: hokein
Date: Mon Jun 17 05:59:14 2019
New Revision: 363554

URL: http://llvm.org/viewvc/llvm-project?rev=363554&view=rev
Log:
[clangd] Detect C++ for extension-less source files in vscode extension

Summary:
Extend our extension to support detecting these files as C++ files based on the 
first
line (`-*- C++ -*-`), it will make clangd work on C++ standard headers
(e.g. iostream).

We use the contributes.languages[1] to enrich the builtin VScode C++
support.

[1]: 
https://code.visualstudio.com/api/references/contribution-points#contributes.languages

Reviewers: kadircet

Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, cfe-commits

Tags: #clang

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

Modified:
clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json

Modified: clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json?rev=363554&r1=363553&r2=363554&view=diff
==
--- clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json (original)
+++ clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json Mon Jun 
17 05:59:14 2019
@@ -50,6 +50,10 @@
 "url": 
"http://llvm.org/svn/llvm-project/clang-tools-extra/trunk/clangd/clients/clangd-vscode/";
 },
 "contributes": {
+"languages": [{
+"id": "cpp",
+"firstLine": "^\/[/*].*-\\*-\\s*C\\+\\+\\s*-\\*-.*"
+}],
 "configuration": {
 "type": "object",
 "title": "clangd configuration",


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


[PATCH] D63397: [clangd] Detect C++ for extension-less source files in vscode extension

2019-06-17 Thread Haojian Wu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL363554: [clangd] Detect C++ for extension-less source files 
in vscode extension (authored by hokein, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D63397?vs=204999&id=205055#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D63397

Files:
  clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json


Index: clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json
===
--- clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json
+++ clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json
@@ -50,6 +50,10 @@
 "url": 
"http://llvm.org/svn/llvm-project/clang-tools-extra/trunk/clangd/clients/clangd-vscode/";
 },
 "contributes": {
+"languages": [{
+"id": "cpp",
+"firstLine": "^\/[/*].*-\\*-\\s*C\\+\\+\\s*-\\*-.*"
+}],
 "configuration": {
 "type": "object",
 "title": "clangd configuration",


Index: clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json
===
--- clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json
+++ clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json
@@ -50,6 +50,10 @@
 "url": "http://llvm.org/svn/llvm-project/clang-tools-extra/trunk/clangd/clients/clangd-vscode/";
 },
 "contributes": {
+"languages": [{
+"id": "cpp",
+"firstLine": "^\/[/*].*-\\*-\\s*C\\+\\+\\s*-\\*-.*"
+}],
 "configuration": {
 "type": "object",
 "title": "clangd configuration",
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D63426: [clangd] Narrow rename to local symbols.

2019-06-17 Thread 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.

Previously, we perform rename for all kinds of symbols (local, global).

This patch narrows the scope by only renaming symbols not being used
outside of the main file (with index asisitance). Renaming global
symbols is not supported at the moment (return an error).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D63426

Files:
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/refactor/Rename.cpp
  clang-tools-extra/clangd/refactor/Rename.h
  clang-tools-extra/clangd/unittests/RenameTests.cpp

Index: clang-tools-extra/clangd/unittests/RenameTests.cpp
===
--- clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -18,6 +18,10 @@
 namespace clangd {
 namespace {
 
+MATCHER_P2(RenameRange, Code, Range, "") {
+  return replacementToEdit(Code, arg).range == Range;
+}
+
 TEST(RenameTest, SingleFile) {
   struct Test {
 const char* Before;
@@ -87,6 +91,69 @@
   }
 }
 
+TEST(RenameTest, WithIndex) {
+  const char *Tests[] = {
+  R"cpp(
+class [[Foo]] {};
+void f() {
+  [[Fo^o]] b;
+}
+ )cpp",
+
+  R"cpp(
+void f(int [[Lo^cal]]) {
+  [[Local]] = 2;
+}
+  )cpp",
+
+  // Cases where we reject to do the rename.
+  R"cpp( // no symbol at the cursor
+// This is in comme^nt.
+   )cpp",
+
+  R"cpp(
+void f() { // Outside main file.
+  Out^side s;
+}
+  )cpp",
+  };
+  const char *Header = "class Outside {};";
+
+  TestTU OtherFile = TestTU::withCode("Outside s;");
+  OtherFile.HeaderCode = Header;
+  OtherFile.Filename = "other.cc";
+
+  for (const char *Test : Tests) {
+Annotations T(Test);
+TestTU MainFile;
+MainFile.Code = T.code();
+MainFile.HeaderCode = Header;
+auto AST = MainFile.build();
+// Build a fake index containing refs of MainFile and OtherFile
+auto OtherFileIndex = OtherFile.index();
+auto MainFileIndex = MainFile.index();
+auto Index = MergedIndex(MainFileIndex.get(), OtherFileIndex.get());
+
+auto Results = renameWithinFile(AST, testPath(MainFile.Filename), T.point(),
+"dummyNewName", &Index);
+bool WantRename = true;
+if (T.ranges().empty())
+  WantRename = false;
+if (!WantRename) {
+  EXPECT_FALSE(Results) << "expected renameWithinFile returned an error: "
+<< T.code();
+  llvm::consumeError(Results.takeError());
+} else {
+  EXPECT_TRUE(bool(Results)) << "renameWithinFile returned an error: "
+ << llvm::toString(Results.takeError());
+  std::vector> Expected;
+  for (const auto &R : T.ranges())
+Expected.push_back(RenameRange(MainFile.Code, R));
+  EXPECT_THAT(*Results, UnorderedElementsAreArray(Expected));
+}
+  }
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/refactor/Rename.h
===
--- clang-tools-extra/clangd/refactor/Rename.h
+++ clang-tools-extra/clangd/refactor/Rename.h
@@ -18,10 +18,10 @@
 
 /// Renames all occurrences of the symbol at \p Pos to \p NewName.
 /// Occurrences outside the current file are not modified.
-llvm::Expected renameWithinFile(ParsedAST &AST,
-   llvm::StringRef File,
-   Position Pos,
-   llvm::StringRef NewName);
+/// Only support renaming symbols not being used outside the file.
+llvm::Expected
+renameWithinFile(ParsedAST &AST, llvm::StringRef File, Position Pos,
+ llvm::StringRef NewName, const SymbolIndex *Index = nullptr);
 
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/refactor/Rename.cpp
===
--- clang-tools-extra/clangd/refactor/Rename.cpp
+++ clang-tools-extra/clangd/refactor/Rename.cpp
@@ -7,8 +7,11 @@
 //===--===//
 
 #include "refactor/Rename.h"
+#include "AST.h"
+#include "Logger.h"
 #include "clang/Tooling/Refactoring/RefactoringResultConsumer.h"
 #include "clang/Tooling/Refactoring/Rename/RenamingAction.h"
+#include "clang/Tooling/Refactoring/Rename/USRFinder.h"
 
 namespace clang {
 namespace clangd {
@@ -46,15 +49,70 @@
   return Err;
 }
 
+static llvm::Optional filePath(const SymbolLocation &Loc,
+llvm::StringRef HintFilePath) {
+  if (!Loc)
+return None;
+  auto Uri = URI::parse(Loc.FileURI);
+  if (!Uri) {
+elog("Could 

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-06-17 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 updated this revision to Diff 205058.
xbolva00 added a comment.

More tests


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

https://reviews.llvm.org/D63423

Files:
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaExpr.cpp
  test/SemaCXX/warn-xor-as-pow.cpp

Index: test/SemaCXX/warn-xor-as-pow.cpp
===
--- test/SemaCXX/warn-xor-as-pow.cpp
+++ test/SemaCXX/warn-xor-as-pow.cpp
@@ -0,0 +1,37 @@
+// RUN: %clang_cc1 -x c++ -fsyntax-only -verify -Wxor-as-pow %s
+// RUN: %clang_cc1 -x c++ -fsyntax-only -verify %s
+// RUN: %clang_cc1 -x c++ -fsyntax-only -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+
+#define TWO 2
+#define TEN 10
+#define TWO_ULL 2ULL
+void test(unsigned a, unsigned b) {
+unsigned res;
+res = a ^ 5;
+res = 2 ^ b;
+res = a ^ b;
+res = 2 ^ 0;
+res = 2 ^ 1; // expected-warning {{result of '2 ^ 1' is 3, maybe you mean '1<<1' (2)?}}
+// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:16}:"1<<1"
+res = 2 ^ 8; // expected-warning {{result of '2 ^ 8' is 10, maybe you mean '1<<8' (256)?}}
+// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:16}:"1<<8"
+res = TWO ^ 8; // expected-warning {{result of 'TWO ^ 8' is 10, maybe you mean '1<<8' (256)?}}
+// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:18}:"1<<8"
+res = 2 ^ 16; // expected-warning {{result of '2 ^ 16' is 18, maybe you mean '1<<16' (65536)?}}
+// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:17}:"1<<16"
+res = 0b10 ^ 16;
+res = 2 ^ 0b100;
+res = 2 xor 16;
+unsigned char two = 2;
+res = two ^ 16;
+res = TWO_ULL ^ 16;
+
+res = 10 ^ 0;
+res = 10 ^ 1; // expected-warning {{result of '10 ^ 1' is 11, maybe you mean '10'?}}
+// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:17}:"10"
+res = 10 ^ 10; // expected-warning {{result of '10 ^ 10' is 0, maybe you mean '100'?}}
+// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:18}:"100"
+res = TEN ^ 10; // expected-warning {{result of 'TEN ^ 10' is 0, maybe you mean '100'?}}
+// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:19}:"100"
+res = 10 xor 10;
+}
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -10890,6 +10890,50 @@
   return GetSignedVectorType(vType);
 }
 
+static void diagnoseXorMisusedAsPow(Sema &S, ExprResult &LHS, ExprResult &RHS,
+SourceLocation Loc) {
+  auto *LHSInt = dyn_cast(LHS.get());
+  auto *RHSInt = dyn_cast(RHS.get());
+  if (!LHSInt || !RHSInt)
+return;
+
+  if (LHSInt->getValue().getBitWidth() != RHSInt->getValue().getBitWidth())
+return;
+
+  CharSourceRange OpRange = CharSourceRange::getCharRange(
+  LHSInt->getBeginLoc(), S.getLocForEndOfToken(RHSInt->getLocation()));
+  llvm::StringRef ExprStr =
+  Lexer::getSourceText(OpRange, S.getSourceManager(), S.getLangOpts());
+
+  if (S.getLangOpts().CPlusPlus) {
+// Do not diagnose binary literals
+if (ExprStr.find("0b") != llvm::StringRef::npos)
+  return;
+// Do not diagnose if xor keyword is used
+if (ExprStr.find("xor") != llvm::StringRef::npos)
+  return;
+  }
+
+  int64_t RightSideValue = RHSInt->getValue().getSExtValue();
+  if (RightSideValue < 1)
+return;
+
+  llvm::APInt XorValue = LHSInt->getValue() ^ RHSInt->getValue();
+  if (LHSInt->getValue() == 2) {
+llvm::APInt PowValue = (LHSInt->getValue() - 1) << RHSInt->getValue();
+std::string SuggestedExpr = "1<<" + RHSInt->getValue().toString(10, true);
+S.Diag(Loc, diag::warn_xor_used_as_pow_base_two)
+<< ExprStr << XorValue.toString(10, true) << SuggestedExpr
+<< PowValue.toString(10, true)
+<< FixItHint::CreateReplacement(OpRange, SuggestedExpr);
+  } else if (LHSInt->getValue() == 10) {
+std::string SuggestedValue = "1" + std::string(RightSideValue, '0');
+S.Diag(Loc, diag::warn_xor_used_as_pow_base_ten)
+<< ExprStr << XorValue.toString(10, true) << SuggestedValue
+<< FixItHint::CreateReplacement(OpRange, SuggestedValue);
+  }
+}
+
 QualType Sema::CheckVectorLogicalOperands(ExprResult &LHS, ExprResult &RHS,
   SourceLocation Loc) {
   // Ensure that either both operands are of the same vector type, or
@@ -10933,6 +10977,9 @@
   if (Opc == BO_And)
 diagnoseLogicalNotOnLHSofCheck(*this, LHS, RHS, Loc, Opc);
 
+  if (Opc == BO_Xor)
+diagnoseXorMisusedAsPow(*this, LHS, RHS, Loc);
+
   ExprResult LHSResult = LHS, RHSResult = RHS;
   QualType compType = UsualArithmeticConversions(LHSResult, RHSResult,
  IsCompAssign);
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
---

[clang-tools-extra] r363555 - [clangd] Bump vscode-clangd v0.0.15.

2019-06-17 Thread Haojian Wu via cfe-commits
Author: hokein
Date: Mon Jun 17 06:18:24 2019
New Revision: 363555

URL: http://llvm.org/viewvc/llvm-project?rev=363555&view=rev
Log:
[clangd] Bump vscode-clangd v0.0.15.

CHANGELOG:
- support detecting C++ language from first line (`-*- C++ -*-`) of the file.

Modified:
clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json

Modified: clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json?rev=363555&r1=363554&r2=363555&view=diff
==
--- clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json (original)
+++ clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json Mon Jun 
17 06:18:24 2019
@@ -2,7 +2,7 @@
 "name": "vscode-clangd",
 "displayName": "vscode-clangd",
 "description": "Clang Language Server",
-"version": "0.0.14",
+"version": "0.0.15",
 "publisher": "llvm-vs-code-extensions",
 "homepage": "https://clang.llvm.org/extra/clangd.html";,
 "engines": {


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


[PATCH] D63425: [clangd] Perform merge for main file symbols.

2019-06-17 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment.

We also make use of `PickOne` in `updatePreamble` shouldn't that also cause 
similar troubles?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63425



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


[PATCH] D53157: Teach the IRBuilder about constrained fadd and friends

2019-06-17 Thread Kevin P. Neal via Phabricator via cfe-commits
kpn added a comment.

Ping.


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

https://reviews.llvm.org/D53157



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


[PATCH] D63425: [clangd] Perform merge for main file symbols.

2019-06-17 Thread Haojian Wu via Phabricator via cfe-commits
hokein added a comment.

In D63425#1546010 , @kadircet wrote:

> We also make use of `PickOne` in `updatePreamble` shouldn't that also cause 
> similar troubles?


Yeah, we have similar problems in `updatePreamble`, but this is a 
correctness/performance tradeoff -- we have a large number of symbols in 
preamble, the cost of merging is too high.

However the number of main file symbols is relatively small (e.g. Sema.cpp only 
has 99 symbols), the cost is not too high.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63425



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


Re: r363116 - [X86] [ABI] Fix i386 ABI "__m64" type bug

2019-06-17 Thread Hans Wennborg via cfe-commits
This broke Chromium for 32-bit Linux:
https://bugs.chromium.org/p/chromium/issues/detail?id=974542#c5

It's not clear what's happening yet, bet just to give a heads up.

Since this changes ABI behaviour, it would probably we worth
mentioning in docs/ReleaseNotes.rst too.

On Wed, Jun 12, 2019 at 3:49 AM Pengfei Wang via cfe-commits
 wrote:
>
> Author: pengfei
> Date: Tue Jun 11 18:52:23 2019
> New Revision: 363116
>
> URL: http://llvm.org/viewvc/llvm-project?rev=363116&view=rev
> Log:
> [X86] [ABI] Fix i386 ABI "__m64" type bug
>
> According to System V i386 ABI: the  __m64 type paramater and return
> value are passed by MMX registers. But current implementation treats
> __m64 as i64 which results in parameter passing by stack and returning
> by EDX and EAX.
>
> This patch fixes the bug (https://bugs.llvm.org/show_bug.cgi?id=41029)
> for Linux and NetBSD.
>
> Patch by Wei Xiao (wxiao3)
>
> Differential Revision: https://reviews.llvm.org/D59744
>
> Added:
> cfe/trunk/test/CodeGen/x86_32-m64.c
> Modified:
> cfe/trunk/lib/CodeGen/TargetInfo.cpp
> cfe/trunk/test/CodeGen/x86_32-arguments-linux.c
>
> Modified: cfe/trunk/lib/CodeGen/TargetInfo.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/TargetInfo.cpp?rev=363116&r1=363115&r2=363116&view=diff
> ==
> --- cfe/trunk/lib/CodeGen/TargetInfo.cpp (original)
> +++ cfe/trunk/lib/CodeGen/TargetInfo.cpp Tue Jun 11 18:52:23 2019
> @@ -915,14 +915,6 @@ ABIArgInfo PNaClABIInfo::classifyReturnT
> : ABIArgInfo::getDirect());
>  }
>
> -/// IsX86_MMXType - Return true if this is an MMX type.
> -bool IsX86_MMXType(llvm::Type *IRType) {
> -  // Return true if the type is an MMX type <2 x i32>, <4 x i16>, or <8 x 
> i8>.
> -  return IRType->isVectorTy() && IRType->getPrimitiveSizeInBits() == 64 &&
> -cast(IRType)->getElementType()->isIntegerTy() &&
> -IRType->getScalarSizeInBits() != 64;
> -}
> -
>  static llvm::Type* X86AdjustInlineAsmType(CodeGen::CodeGenFunction &CGF,
>StringRef Constraint,
>llvm::Type* Ty) {
> @@ -1011,6 +1003,7 @@ class X86_32ABIInfo : public SwiftABIInf
>bool IsSoftFloatABI;
>bool IsMCUABI;
>unsigned DefaultNumRegisterParameters;
> +  bool IsMMXEnabled;
>
>static bool isRegisterSize(unsigned Size) {
>  return (Size == 8 || Size == 16 || Size == 32 || Size == 64);
> @@ -1070,13 +1063,15 @@ public:
>
>X86_32ABIInfo(CodeGen::CodeGenTypes &CGT, bool DarwinVectorABI,
>  bool RetSmallStructInRegABI, bool Win32StructABI,
> -unsigned NumRegisterParameters, bool SoftFloatABI)
> +unsigned NumRegisterParameters, bool SoftFloatABI,
> +bool MMXEnabled)
>  : SwiftABIInfo(CGT), IsDarwinVectorABI(DarwinVectorABI),
>IsRetSmallStructInRegABI(RetSmallStructInRegABI),
>IsWin32StructABI(Win32StructABI),
>IsSoftFloatABI(SoftFloatABI),
>IsMCUABI(CGT.getTarget().getTriple().isOSIAMCU()),
> -  DefaultNumRegisterParameters(NumRegisterParameters) {}
> +  DefaultNumRegisterParameters(NumRegisterParameters),
> +  IsMMXEnabled(MMXEnabled) {}
>
>bool shouldPassIndirectlyForSwift(ArrayRef scalars,
>  bool asReturnValue) const override {
> @@ -1091,16 +1086,30 @@ public:
>  // x86-32 lowering does not support passing swifterror in a register.
>  return false;
>}
> +
> +  bool isPassInMMXRegABI() const {
> +// The System V i386 psABI requires __m64 to be passed in MMX registers.
> +// Clang historically had a bug where it failed to apply this rule, and
> +// some platforms (e.g. Darwin, PS4, and FreeBSD) have opted to maintain
> +// compatibility with the old Clang behavior, so we only apply it on
> +// platforms that have specifically requested it (currently just Linux 
> and
> +// NetBSD).
> +const llvm::Triple &T = getTarget().getTriple();
> +if (IsMMXEnabled && (T.isOSLinux() || T.isOSNetBSD()))
> +  return true;
> +return false;
> +  }
>  };
>
>  class X86_32TargetCodeGenInfo : public TargetCodeGenInfo {
>  public:
>X86_32TargetCodeGenInfo(CodeGen::CodeGenTypes &CGT, bool DarwinVectorABI,
>bool RetSmallStructInRegABI, bool Win32StructABI,
> -  unsigned NumRegisterParameters, bool SoftFloatABI)
> +  unsigned NumRegisterParameters, bool SoftFloatABI,
> +  bool MMXEnabled = false)
>: TargetCodeGenInfo(new X86_32ABIInfo(
>  CGT, DarwinVectorABI, RetSmallStructInRegABI, Win32StructABI,
> -NumRegisterParameters, SoftFloatABI)) {}
> +NumRegisterParameters, SoftFloatABI, MMXEnabled)) {}
>
>static bool isStructReturnInRegABI(
>const llvm::Triple &Triple, const CodeGen

Re: r363116 - [X86] [ABI] Fix i386 ABI "__m64" type bug

2019-06-17 Thread Hans Wennborg via cfe-commits
pengfei's email bounced. Trying Wei Xiao instead.

On Mon, Jun 17, 2019 at 3:44 PM Hans Wennborg  wrote:
>
> This broke Chromium for 32-bit Linux:
> https://bugs.chromium.org/p/chromium/issues/detail?id=974542#c5
>
> It's not clear what's happening yet, bet just to give a heads up.
>
> Since this changes ABI behaviour, it would probably we worth
> mentioning in docs/ReleaseNotes.rst too.
>
> On Wed, Jun 12, 2019 at 3:49 AM Pengfei Wang via cfe-commits
>  wrote:
> >
> > Author: pengfei
> > Date: Tue Jun 11 18:52:23 2019
> > New Revision: 363116
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=363116&view=rev
> > Log:
> > [X86] [ABI] Fix i386 ABI "__m64" type bug
> >
> > According to System V i386 ABI: the  __m64 type paramater and return
> > value are passed by MMX registers. But current implementation treats
> > __m64 as i64 which results in parameter passing by stack and returning
> > by EDX and EAX.
> >
> > This patch fixes the bug (https://bugs.llvm.org/show_bug.cgi?id=41029)
> > for Linux and NetBSD.
> >
> > Patch by Wei Xiao (wxiao3)
> >
> > Differential Revision: https://reviews.llvm.org/D59744
> >
> > Added:
> > cfe/trunk/test/CodeGen/x86_32-m64.c
> > Modified:
> > cfe/trunk/lib/CodeGen/TargetInfo.cpp
> > cfe/trunk/test/CodeGen/x86_32-arguments-linux.c
> >
> > Modified: cfe/trunk/lib/CodeGen/TargetInfo.cpp
> > URL: 
> > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/TargetInfo.cpp?rev=363116&r1=363115&r2=363116&view=diff
> > ==
> > --- cfe/trunk/lib/CodeGen/TargetInfo.cpp (original)
> > +++ cfe/trunk/lib/CodeGen/TargetInfo.cpp Tue Jun 11 18:52:23 2019
> > @@ -915,14 +915,6 @@ ABIArgInfo PNaClABIInfo::classifyReturnT
> > : ABIArgInfo::getDirect());
> >  }
> >
> > -/// IsX86_MMXType - Return true if this is an MMX type.
> > -bool IsX86_MMXType(llvm::Type *IRType) {
> > -  // Return true if the type is an MMX type <2 x i32>, <4 x i16>, or <8 x 
> > i8>.
> > -  return IRType->isVectorTy() && IRType->getPrimitiveSizeInBits() == 64 &&
> > -cast(IRType)->getElementType()->isIntegerTy() &&
> > -IRType->getScalarSizeInBits() != 64;
> > -}
> > -
> >  static llvm::Type* X86AdjustInlineAsmType(CodeGen::CodeGenFunction &CGF,
> >StringRef Constraint,
> >llvm::Type* Ty) {
> > @@ -1011,6 +1003,7 @@ class X86_32ABIInfo : public SwiftABIInf
> >bool IsSoftFloatABI;
> >bool IsMCUABI;
> >unsigned DefaultNumRegisterParameters;
> > +  bool IsMMXEnabled;
> >
> >static bool isRegisterSize(unsigned Size) {
> >  return (Size == 8 || Size == 16 || Size == 32 || Size == 64);
> > @@ -1070,13 +1063,15 @@ public:
> >
> >X86_32ABIInfo(CodeGen::CodeGenTypes &CGT, bool DarwinVectorABI,
> >  bool RetSmallStructInRegABI, bool Win32StructABI,
> > -unsigned NumRegisterParameters, bool SoftFloatABI)
> > +unsigned NumRegisterParameters, bool SoftFloatABI,
> > +bool MMXEnabled)
> >  : SwiftABIInfo(CGT), IsDarwinVectorABI(DarwinVectorABI),
> >IsRetSmallStructInRegABI(RetSmallStructInRegABI),
> >IsWin32StructABI(Win32StructABI),
> >IsSoftFloatABI(SoftFloatABI),
> >IsMCUABI(CGT.getTarget().getTriple().isOSIAMCU()),
> > -  DefaultNumRegisterParameters(NumRegisterParameters) {}
> > +  DefaultNumRegisterParameters(NumRegisterParameters),
> > +  IsMMXEnabled(MMXEnabled) {}
> >
> >bool shouldPassIndirectlyForSwift(ArrayRef scalars,
> >  bool asReturnValue) const override {
> > @@ -1091,16 +1086,30 @@ public:
> >  // x86-32 lowering does not support passing swifterror in a register.
> >  return false;
> >}
> > +
> > +  bool isPassInMMXRegABI() const {
> > +// The System V i386 psABI requires __m64 to be passed in MMX 
> > registers.
> > +// Clang historically had a bug where it failed to apply this rule, and
> > +// some platforms (e.g. Darwin, PS4, and FreeBSD) have opted to 
> > maintain
> > +// compatibility with the old Clang behavior, so we only apply it on
> > +// platforms that have specifically requested it (currently just Linux 
> > and
> > +// NetBSD).
> > +const llvm::Triple &T = getTarget().getTriple();
> > +if (IsMMXEnabled && (T.isOSLinux() || T.isOSNetBSD()))
> > +  return true;
> > +return false;
> > +  }
> >  };
> >
> >  class X86_32TargetCodeGenInfo : public TargetCodeGenInfo {
> >  public:
> >X86_32TargetCodeGenInfo(CodeGen::CodeGenTypes &CGT, bool DarwinVectorABI,
> >bool RetSmallStructInRegABI, bool Win32StructABI,
> > -  unsigned NumRegisterParameters, bool 
> > SoftFloatABI)
> > +  unsigned NumRegisterParameters, bool 
> > SoftFloatABI,
> > +  bool MMXEnabled = f

[PATCH] D59744: Fix i386 ABI "__m64" type bug

2019-06-17 Thread Hans Wennborg via Phabricator via cfe-commits
hans added a comment.

I tried replying on the cfe-commits email, but both Pengfei and Wei's email 
addresses seem to bounce, so replying here instead:

This broke Chromium for 32-bit Linux:
https://bugs.chromium.org/p/chromium/issues/detail?id=974542#c5

It's not clear what's happening yet, bet just to give a heads up.

Since this changes ABI behaviour, it would probably we worth
mentioning in docs/ReleaseNotes.rst too.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D59744



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


[PATCH] D43500: [clang-tidy]: modernize-use-default-member-init: Remove trailing comma and colon.

2019-06-17 Thread Jeremy Demeule via Phabricator via cfe-commits
jdemeule added a comment.

In D43500#1244518 , @JonasToth wrote:

> What is the status here? Will you continue to work on this patch @jdemeule 
> (which would be great!) ?


Hi, now, this patch contains only tests which are already covered with lit 
tests.
The cleanup is now applied in clang-apply-replacements.

In D43500#1491369 , @chgans wrote:

> Hi, Using llvm-8 (llvm ubuntu repo), clang-replacement has improved, but 
> there still a case when a comma is left beyond: if there was only one member 
> init.


I do not reproduce at master (9.0.0 trunk), do you have an example in mind?


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

https://reviews.llvm.org/D43500



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


[PATCH] D43500: [clang-tidy]: modernize-use-default-member-init: Remove trailing comma and colon.

2019-06-17 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment.

In D43500#1546077 , @jdemeule wrote:

> In D43500#1244518 , @JonasToth wrote:
>
> > What is the status here? Will you continue to work on this patch @jdemeule 
> > (which would be great!) ?
>
>
> Hi, now, this patch contains only tests which are already covered with lit 
> tests.
>  The cleanup is now applied in clang-apply-replacements.


How does that work with `clang-tidy -fix` ?

> 
> 
> In D43500#1491369 , @chgans wrote:
> 
>> Hi, Using llvm-8 (llvm ubuntu repo), clang-replacement has improved, but 
>> there still a case when a comma is left beyond: if there was only one member 
>> init.
> 
> 
> I do not reproduce at master (9.0.0 trunk), do you have an example in mind?




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

https://reviews.llvm.org/D43500



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


[PATCH] D53157: Teach the IRBuilder about constrained fadd and friends

2019-06-17 Thread Melanie Blower via Phabricator via cfe-commits
mibintc added a comment.

This works for me, I redid my patches adding fp-model options to work with this 
newest changeset, (but I haven't yet uploaded them.)


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

https://reviews.llvm.org/D53157



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


[PATCH] D62697: AMDGPU: Disable errno by default

2019-06-17 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl accepted this revision.
yaxunl added a comment.
This revision is now accepted and ready to land.

LGTM. Sorry for the delay.


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

https://reviews.llvm.org/D62697



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


[PATCH] D43500: [clang-tidy]: modernize-use-default-member-init: Remove trailing comma and colon.

2019-06-17 Thread Jeremy Demeule via Phabricator via cfe-commits
jdemeule added a comment.

In D43500#1546089 , @lebedev.ri wrote:

> In D43500#1546077 , @jdemeule wrote:
>
> > In D43500#1244518 , @JonasToth 
> > wrote:
> >
> > > What is the status here? Will you continue to work on this patch 
> > > @jdemeule (which would be great!) ?
> >
> >
> > Hi, now, this patch contains only tests which are already covered with lit 
> > tests.
> >  The cleanup is now applied in clang-apply-replacements.
>
>
> How does that work with `clang-tidy -fix` ?


If I remember by calling 'format::cleanupAroundReplacements' when applying the 
fix in memory.


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

https://reviews.llvm.org/D43500



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


r363563 - [clang][CodeGen] Remove std::move on temporary

2019-06-17 Thread Kadir Cetinkaya via cfe-commits
Author: kadircet
Date: Mon Jun 17 07:23:06 2019
New Revision: 363563

URL: http://llvm.org/viewvc/llvm-project?rev=363563&view=rev
Log:
[clang][CodeGen] Remove std::move on temporary

Modified:
cfe/trunk/lib/CodeGen/CGCUDANV.cpp

Modified: cfe/trunk/lib/CodeGen/CGCUDANV.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCUDANV.cpp?rev=363563&r1=363562&r2=363563&view=diff
==
--- cfe/trunk/lib/CodeGen/CGCUDANV.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGCUDANV.cpp Mon Jun 17 07:23:06 2019
@@ -795,7 +795,7 @@ llvm::Function *CGNVCUDARuntime::makeMod
 std::string CGNVCUDARuntime::getDeviceStubName(llvm::StringRef Name) const {
   if (!CGM.getLangOpts().HIP)
 return Name;
-  return std::move((Name + ".stub").str());
+  return (Name + ".stub").str();
 }
 
 CGCUDARuntime *CodeGen::CreateNVCUDARuntime(CodeGenModule &CGM) {


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


[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-06-17 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment.

Should we also emit a note how to silence it, eg. swap xor operands or add 
parentheses around 2/10 ?


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

https://reviews.llvm.org/D63423



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


[clang-tools-extra] r363568 - [clangd] Perform merge for main file symbols.

2019-06-17 Thread Haojian Wu via cfe-commits
Author: hokein
Date: Mon Jun 17 07:49:18 2019
New Revision: 363568

URL: http://llvm.org/viewvc/llvm-project?rev=363568&view=rev
Log:
[clangd] Perform merge for main file symbols.

Summary:
Previously, we randomly pick one main file symbol in dynamic index, we
may loose the ideal symbol (with definition location) in the index.

It fixes the issue where sometimes we fail to go to the symbol definition, see:

1. call go-to-decl on Foo in Foo.cpp
2. jump to Foo.h, call go-to-def on Foo in Foo.h

we can't go back to Foo.cpp -- because we open Foo.cpp, Foo.h in clangd, both
files have Foo symbol (one with def&decl, one with decl only), we randomely
choose one.

Reviewers: kadircet

Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, cfe-commits

Tags: #clang

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

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

Modified: clang-tools-extra/trunk/clangd/index/FileIndex.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/FileIndex.cpp?rev=363568&r1=363567&r2=363568&view=diff
==
--- clang-tools-extra/trunk/clangd/index/FileIndex.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/FileIndex.cpp Mon Jun 17 07:49:18 2019
@@ -260,7 +260,7 @@ void FileIndex::updateMain(PathRef Path,
   llvm::make_unique(std::move(std::get<2>(Contents))),
   /*CountReferences=*/true);
   MainFileIndex.reset(
-  MainFileSymbols.buildIndex(IndexType::Light, 
DuplicateHandling::PickOne));
+  MainFileSymbols.buildIndex(IndexType::Light, DuplicateHandling::Merge));
 }
 
 } // namespace clangd

Modified: clang-tools-extra/trunk/clangd/unittests/FileIndexTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/unittests/FileIndexTests.cpp?rev=363568&r1=363567&r2=363568&view=diff
==
--- clang-tools-extra/trunk/clangd/unittests/FileIndexTests.cpp (original)
+++ clang-tools-extra/trunk/clangd/unittests/FileIndexTests.cpp Mon Jun 17 
07:49:18 2019
@@ -46,6 +46,7 @@ MATCHER_P(DefURI, U, "") {
 }
 MATCHER_P(QName, N, "") { return (arg.Scope + arg.Name).str() == N; }
 MATCHER_P(NumReferences, N, "") { return arg.References == N; }
+MATCHER_P(hasOrign, O, "") { return bool(arg.Origin & O); }
 
 namespace clang {
 namespace clangd {
@@ -386,6 +387,27 @@ TEST(FileIndexTest, ReferencesInMainFile
   RefsAre({RefRange(Main.range())}));
 }
 
+TEST(FileIndexTest, MergeMainFileSymbols) {
+  const char* CommonHeader = "void foo();";
+  TestTU Header = TestTU::withCode(CommonHeader);
+  TestTU Cpp = TestTU::withCode("void foo() {}");
+  Cpp.Filename = "foo.cpp";
+  Cpp.HeaderFilename = "foo.h";
+  Cpp.HeaderCode = CommonHeader;
+
+  FileIndex Index;
+  auto HeaderAST = Header.build();
+  auto CppAST = Cpp.build();
+  Index.updateMain(testPath("foo.h"), HeaderAST);
+  Index.updateMain(testPath("foo.cpp"), CppAST);
+
+  auto Symbols = runFuzzyFind(Index, "");
+  // Check foo is merged, foo in Cpp wins (as we see the definition there).
+  EXPECT_THAT(Symbols, ElementsAre(AllOf(DeclURI("unittest:///foo.h"),
+ DefURI("unittest:///foo.cpp"),
+ hasOrign(SymbolOrigin::Merge;
+}
+
 TEST(FileSymbolsTest, CountReferencesNoRefSlabs) {
   FileSymbols FS;
   FS.update("f1", numSlab(1, 3), nullptr, nullptr, true);


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


[PATCH] D63425: [clangd] Perform merge for main file symbols.

2019-06-17 Thread Haojian Wu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL363568: [clangd] Perform merge for main file symbols. 
(authored by hokein, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D63425?vs=205053&id=205072#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D63425

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


Index: clang-tools-extra/trunk/clangd/unittests/FileIndexTests.cpp
===
--- clang-tools-extra/trunk/clangd/unittests/FileIndexTests.cpp
+++ clang-tools-extra/trunk/clangd/unittests/FileIndexTests.cpp
@@ -46,6 +46,7 @@
 }
 MATCHER_P(QName, N, "") { return (arg.Scope + arg.Name).str() == N; }
 MATCHER_P(NumReferences, N, "") { return arg.References == N; }
+MATCHER_P(hasOrign, O, "") { return bool(arg.Origin & O); }
 
 namespace clang {
 namespace clangd {
@@ -386,6 +387,27 @@
   RefsAre({RefRange(Main.range())}));
 }
 
+TEST(FileIndexTest, MergeMainFileSymbols) {
+  const char* CommonHeader = "void foo();";
+  TestTU Header = TestTU::withCode(CommonHeader);
+  TestTU Cpp = TestTU::withCode("void foo() {}");
+  Cpp.Filename = "foo.cpp";
+  Cpp.HeaderFilename = "foo.h";
+  Cpp.HeaderCode = CommonHeader;
+
+  FileIndex Index;
+  auto HeaderAST = Header.build();
+  auto CppAST = Cpp.build();
+  Index.updateMain(testPath("foo.h"), HeaderAST);
+  Index.updateMain(testPath("foo.cpp"), CppAST);
+
+  auto Symbols = runFuzzyFind(Index, "");
+  // Check foo is merged, foo in Cpp wins (as we see the definition there).
+  EXPECT_THAT(Symbols, ElementsAre(AllOf(DeclURI("unittest:///foo.h"),
+ DefURI("unittest:///foo.cpp"),
+ hasOrign(SymbolOrigin::Merge;
+}
+
 TEST(FileSymbolsTest, CountReferencesNoRefSlabs) {
   FileSymbols FS;
   FS.update("f1", numSlab(1, 3), nullptr, nullptr, true);
Index: clang-tools-extra/trunk/clangd/index/FileIndex.cpp
===
--- clang-tools-extra/trunk/clangd/index/FileIndex.cpp
+++ clang-tools-extra/trunk/clangd/index/FileIndex.cpp
@@ -260,7 +260,7 @@
   llvm::make_unique(std::move(std::get<2>(Contents))),
   /*CountReferences=*/true);
   MainFileIndex.reset(
-  MainFileSymbols.buildIndex(IndexType::Light, 
DuplicateHandling::PickOne));
+  MainFileSymbols.buildIndex(IndexType::Light, DuplicateHandling::Merge));
 }
 
 } // namespace clangd


Index: clang-tools-extra/trunk/clangd/unittests/FileIndexTests.cpp
===
--- clang-tools-extra/trunk/clangd/unittests/FileIndexTests.cpp
+++ clang-tools-extra/trunk/clangd/unittests/FileIndexTests.cpp
@@ -46,6 +46,7 @@
 }
 MATCHER_P(QName, N, "") { return (arg.Scope + arg.Name).str() == N; }
 MATCHER_P(NumReferences, N, "") { return arg.References == N; }
+MATCHER_P(hasOrign, O, "") { return bool(arg.Origin & O); }
 
 namespace clang {
 namespace clangd {
@@ -386,6 +387,27 @@
   RefsAre({RefRange(Main.range())}));
 }
 
+TEST(FileIndexTest, MergeMainFileSymbols) {
+  const char* CommonHeader = "void foo();";
+  TestTU Header = TestTU::withCode(CommonHeader);
+  TestTU Cpp = TestTU::withCode("void foo() {}");
+  Cpp.Filename = "foo.cpp";
+  Cpp.HeaderFilename = "foo.h";
+  Cpp.HeaderCode = CommonHeader;
+
+  FileIndex Index;
+  auto HeaderAST = Header.build();
+  auto CppAST = Cpp.build();
+  Index.updateMain(testPath("foo.h"), HeaderAST);
+  Index.updateMain(testPath("foo.cpp"), CppAST);
+
+  auto Symbols = runFuzzyFind(Index, "");
+  // Check foo is merged, foo in Cpp wins (as we see the definition there).
+  EXPECT_THAT(Symbols, ElementsAre(AllOf(DeclURI("unittest:///foo.h"),
+ DefURI("unittest:///foo.cpp"),
+ hasOrign(SymbolOrigin::Merge;
+}
+
 TEST(FileSymbolsTest, CountReferencesNoRefSlabs) {
   FileSymbols FS;
   FS.update("f1", numSlab(1, 3), nullptr, nullptr, true);
Index: clang-tools-extra/trunk/clangd/index/FileIndex.cpp
===
--- clang-tools-extra/trunk/clangd/index/FileIndex.cpp
+++ clang-tools-extra/trunk/clangd/index/FileIndex.cpp
@@ -260,7 +260,7 @@
   llvm::make_unique(std::move(std::get<2>(Contents))),
   /*CountReferences=*/true);
   MainFileIndex.reset(
-  MainFileSymbols.buildIndex(IndexType::Light, DuplicateHandling::PickOne));
+  MainFileSymbols.buildIndex(IndexType::Light, DuplicateHandling::Merge));
 }
 
 } // namespace clangd
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D62883: [analyzer] Track conditions of terminator statements on which the reported node depends on

2019-06-17 Thread Csaba Dabis via Phabricator via cfe-commits
Charusso accepted this revision.
Charusso added a comment.

My opinion based on the Summary, which is the kind of the opposite what we 
have. No invalidation happens, so no swap happens, so it is kind of misleading 
like writing out "Returning something" where we cannot be sure what we return 
("resulting in new notes being placed at for the call to foo() on line 14 and a 
note about flag being invalidated on line 5.").
Please update the Summary according to the changes in the patch.

Also please note that, we are very pessimistic and everything is uninteresting 
(like returning unknown values), you only could `markInteresting()`.

In D62883#1545347 , @Szelethus wrote:

> I like to think that most of the unimportant notes can be easily categorized, 
> as seen above. With some, admittedly crude heuristics, we could cut these 
> down greatly, and leave some breathing room for fine-tuning it.


I am not against that direction, just you throw oil on fire, and we already 
have a huge fire (D62978 ). I believe with 
that we could see more bad-patterns and we could answer "In which range do we 
track?".
I hope you find the answer, and thanks for working on that to solve the problem!


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

https://reviews.llvm.org/D62883



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


[PATCH] D63164: [HIP] Add option to force lambda nameing following ODR in HIP/CUDA.

2019-06-17 Thread Michael Liao via Phabricator via cfe-commits
hliao added a comment.

ping again. not sure my explanation gives more details on why this patch is 
created.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63164



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


[PATCH] D59402: Suggestions to fix -Wmissing-{prototypes,variable-declarations}

2019-06-17 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: lib/Sema/SemaDecl.cpp:13340-13342
   }
+  else
+Diag(FD->getTypeSpecStartLoc(), diag::note_static_for_internal_linkage)

Formatting here is a bit off -- you should run through clang-format. Also, I'd 
appreciate curly braces even though the `else` clause is technically one line.



Comment at: lib/Sema/SemaDecl.cpp:13345-13346
+<< (FD->getStorageClass() == SC_None
+? FixItHint::CreateInsertion(FD->getTypeSpecStartLoc(),
+ "static ")
+: FixItHint{});

We may not want to produce the fixit if there's a macro involved in the 
declaration. Consider:
```
#ifdef SOMETHING
#define FROBBLE static
#else
#define FROBBLE
#endif

FROBBLE void foo(void);
```
We probably don't want the fixit in the case `SOMETHING` is not defined.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59402



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


[PATCH] D62619: [analyzer][IDF] Add a control dependency calculator + a new debug checker

2019-06-17 Thread Jakub Kuderski via Phabricator via cfe-commits
kuhar added inline comments.



Comment at: clang/include/clang/Analysis/Analyses/Dominators.h:51
+  CFGDominatorTreeImpl(CFG *cfg) {
+buildDominatorTree(cfg);
+  }

DomTree has a constructor that runs the builder -- why not use it directly?



Comment at: clang/include/clang/Analysis/Analyses/Dominators.h:160
   /// changes.
   void changeImmediateDominator(CFGBlock *N, CFGBlock *NewIDom) {
 DT.changeImmediateDominator(N, NewIDom);

Do you need it at all? I understand it's a wrapper around dominators, but this 
API is virtually impossible to use safely.



Comment at: clang/include/clang/Analysis/Analyses/Dominators.h:225
+
+  /// Whether \p A is control dependent of \p B.
+  bool isControlDependent(CFGBlock *A, CFGBlock *B) {

I thinks it should be `A is control dependent on B`


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

https://reviews.llvm.org/D62619



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


[PATCH] D63436: [analyzer] Fix JSON dumps for ExplodedNodes

2019-06-17 Thread Csaba Dabis via Phabricator via cfe-commits
Charusso created this revision.
Charusso added a reviewer: NoQ.
Charusso added a project: clang.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, 
mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun.

- Now we apply the `has_report` on the proper `ProgramPoint`.
- This patch also removes the trailing comma after each node.


Repository:
  rC Clang

https://reviews.llvm.org/D63436

Files:
  clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
  clang/test/Analysis/dump_egraph.c


Index: clang/test/Analysis/dump_egraph.c
===
--- clang/test/Analysis/dump_egraph.c
+++ clang/test/Analysis/dump_egraph.c
@@ -1,6 +1,6 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-dump-egraph=%t.dot 
%s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core \
+// RUN:  -analyzer-dump-egraph=%t.dot %s
 // RUN: cat %t.dot | FileCheck %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-dump-egraph=%t.dot 
-trim-egraph %s
 // REQUIRES: asserts
 
 int getJ();
@@ -12,7 +12,7 @@
 
 // CHECK: digraph "Exploded Graph" {
 
-// CHECK: \"program_points\": [\l\{ \"kind\": 
\"Edge\", \"src_id\": 2, \"dst_id\": 1, \"terminator\": null, \"term_kind\": 
null, \"tag\": null \}\l  ],\l  \"program_state\": null
+// CHECK: \"program_points\": [\l\{ \"kind\": 
\"Edge\", \"src_id\": 2, \"dst_id\": 1, \"terminator\": null, \"term_kind\": 
null, \"has_report\": false, \"tag\": null 
\}\l  ],\l  \"program_state\": null
 
 // CHECK: \"program_points\": [\l\{ \"kind\": 
\"BlockEntrance\", \"block_id\": 1
 
Index: clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -3076,9 +3076,7 @@
 ProgramStateRef State = N->getState();
 
 Out << "{ \"node_id\": " << N->getID(G) << ", \"pointer\": \""
-<< (const void *)N << "\", \"state_id\": " << State->getID()
-<< ", \"has_report\": " << (nodeHasBugReport(N) ? "true" : "false")
-<< ",\\l";
+<< (const void *)N << "\", \"state_id\": " << State->getID() << ",\\l";
 
 Indent(Out, Space, IsDot) << "\"program_points\": [\\l";
 
@@ -3088,7 +3086,9 @@
 [&](const ExplodedNode *OtherNode) {
   Indent(Out, Space + 1, IsDot) << "{ ";
   OtherNode->getLocation().printJson(Out, /*NL=*/"\\l");
-  Out << ", \"tag\": ";
+  Out << ", \"has_report\": "
+  << (nodeHasBugReport(OtherNode) ? "true" : "false")
+  << ", \"tag\": ";
   if (const ProgramPointTag *Tag = OtherNode->getLocation().getTag())
 Out << '\"' << Tag->getTagDescription() << "\" }";
   else
@@ -3112,11 +3112,7 @@
   Indent(Out, Space, IsDot) << "\"program_state\": null";
 }
 
-Out << "\\l}";
-if (!N->succ_empty())
-  Out << ',';
-Out << "\\l";
-
+Out << "\\l}\\l";
 return Out.str();
   }
 };


Index: clang/test/Analysis/dump_egraph.c
===
--- clang/test/Analysis/dump_egraph.c
+++ clang/test/Analysis/dump_egraph.c
@@ -1,6 +1,6 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-dump-egraph=%t.dot %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core \
+// RUN:  -analyzer-dump-egraph=%t.dot %s
 // RUN: cat %t.dot | FileCheck %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-dump-egraph=%t.dot -trim-egraph %s
 // REQUIRES: asserts
 
 int getJ();
@@ -12,7 +12,7 @@
 
 // CHECK: digraph "Exploded Graph" {
 
-// CHECK: \"program_points\": [\l\{ \"kind\": \"Edge\", \"src_id\": 2, \"dst_id\": 1, \"terminator\": null, \"term_kind\": null, \"tag\": null \}\l  ],\l  \"program_state\": null
+// CHECK: \"program_points\": [\l\{ \"kind\": \"Edge\", \"src_id\": 2, \"dst_id\": 1, \"terminator\": null, \"term_kind\": null, \"has_report\": false, \"tag\": null \}\l  ],\l  \"program_state\": null
 
 // CHECK: \"program_points\": [\l\{ \"kind\": \"BlockEntrance\", \"block_id\": 1
 
Index: clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -3076,9 +3076,7 @@
 ProgramStateRef State = N->getState();
 
 Out << "{ \"node_id\": " << N->getID(G) << ", \"pointer\": \""
-<< (const void *)N << "\", \"state_id\": " << State->getID()
-<< ", \"has_report\": " << (nodeHasBugReport(N) ? "true" : "false")
-<< ",\\l";
+<< (const void *)N << "\", \"state_id\": " << State->getID() << ",\\l";
 
 Indent(Out, Space, IsDot) << "\"program_points\": [\\l";
 
@@ -3088,7 +3086,9 @@
 [&](const ExplodedNode *OtherNode) {
   Indent(Out, Space + 1, IsDot) << "{ ";
   OtherNode->getLocation().printJson(Out, /*NL=*/"\\l");
-  Out << ", \"tag\": 

[PATCH] D61552: [clang] Adapt ASTMatcher to explicit(bool) specifier

2019-06-17 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D61552#1530730 , @Tyker wrote:

> i didn't manage to get "clang/docs/tools/dump_ast_matchers.py" to work. it 
> keep failing to parse and generating empty files as a result.


Are you getting errors from running it, or just incorrect output?




Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:6190
+/// cxxDeductionGuideDecl(isExplicit()) will match #6, but not #5.
+/// cxxConstructorDecl(isExplicit()) will match #8, but not #7 or #9.
+AST_POLYMORPHIC_MATCHER(isExplicit, AST_POLYMORPHIC_SUPPORTED_TYPES(

Why won't it match #2?



Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:6220
+/// #6.
+AST_MATCHER_P(FunctionDecl, hasExplicitExpr, internal::Matcher,
+  InnerMatcher) {

Not keen on the name `hasExplicitExpr` -- I think `hasExplicitSpecifier()` 
would be a more intuitive name.



Comment at: clang/lib/ASTMatchers/Dynamic/Registry.cpp:171
   REGISTER_MATCHER(cxxConversionDecl);
+  REGISTER_MATCHER(cxxDeductionGuideDecl);
   REGISTER_MATCHER(cxxCtorInitializer);

Please sort alphabetically.



Comment at: clang/lib/ASTMatchers/Dynamic/Registry.cpp:263
   REGISTER_MATCHER(hasDeducedType);
+  REGISTER_MATCHER(hasExplicitExpr);
   REGISTER_MATCHER(hasDefaultArgument);

Please sort alphabetically.


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

https://reviews.llvm.org/D61552



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


[PATCH] D63436: [analyzer] Fix JSON dumps for ExplodedNodes

2019-06-17 Thread Csaba Dabis via Phabricator via cfe-commits
Charusso marked an inline comment as done.
Charusso added inline comments.



Comment at: clang/test/Analysis/dump_egraph.c:3
 // RUN: cat %t.dot | FileCheck %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-dump-egraph=%t.dot 
-trim-egraph %s
 // REQUIRES: asserts

This did nothing.


Repository:
  rC Clang

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

https://reviews.llvm.org/D63436



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


[PATCH] D62571: Implement codegen for MSVC unions with reference members

2019-06-17 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added reviewers: rnk, majnemer.
aaron.ballman added subscribers: majnemer, rnk.
aaron.ballman added a comment.

This looks reasonable to me, but @rnk and @majnemer may have opinions as well.




Comment at: clang/test/CodeGenCXX/ms-union-member-ref.cpp:3-6
+union A {
+int *&ref;
+int **ptr;
+};

Can you clang-format the patch?


Repository:
  rC Clang

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

https://reviews.llvm.org/D62571



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


[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-06-17 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 updated this revision to Diff 205089.
xbolva00 added a comment.

Silence note added.


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

https://reviews.llvm.org/D63423

Files:
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaExpr.cpp
  test/SemaCXX/warn-xor-as-pow.cpp

Index: test/SemaCXX/warn-xor-as-pow.cpp
===
--- test/SemaCXX/warn-xor-as-pow.cpp
+++ test/SemaCXX/warn-xor-as-pow.cpp
@@ -0,0 +1,52 @@
+// RUN: %clang_cc1 -x c++ -fsyntax-only -verify -Wxor-as-pow %s
+// RUN: %clang_cc1 -x c++ -fsyntax-only -verify %s
+// RUN: %clang_cc1 -x c++ -fsyntax-only -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+
+#define TWO 2
+#define TEN 10
+#define TWO_ULL 2ULL
+void test(unsigned a, unsigned b) {
+unsigned res;
+res = a ^ 5;
+res = 2 ^ b;
+res = a ^ b;
+res = 2 ^ 0;
+res = 2 ^ 1; // expected-warning {{result of '2 ^ 1' is 3, maybe you mean '1<<1' (2)?}}
+// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:16}:"1<<1"
+// expected-note@-2 {{replace expression with '(2) ^ 1' to silence this warning}}
+res = 2 ^ 8; // expected-warning {{result of '2 ^ 8' is 10, maybe you mean '1<<8' (256)?}}
+// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:16}:"1<<8"
+// expected-note@-2 {{replace expression with '(2) ^ 8' to silence this warning}}
+res = TWO ^ 8; // expected-warning {{result of 'TWO ^ 8' is 10, maybe you mean '1<<8' (256)?}}
+// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:18}:"1<<8"
+// expected-note@-2 {{replace expression with '(TWO) ^ 8' to silence this warning}}
+res = 2 ^ 16; // expected-warning {{result of '2 ^ 16' is 18, maybe you mean '1<<16' (65536)?}}
+// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:17}:"1<<16"
+// expected-note@-2 {{replace expression with '(2) ^ 16' to silence this warning}}
+res = 2 ^ TEN; // expected-warning {{result of '2 ^ TEN' is 8, maybe you mean '1<(LHS.get());
+  auto *RHSInt = dyn_cast(RHS.get());
+  if (!LHSInt || !RHSInt)
+return;
+
+  if (LHSInt->getValue().getBitWidth() != RHSInt->getValue().getBitWidth())
+return;
+
+  CharSourceRange OpRange = CharSourceRange::getCharRange(
+  LHSInt->getBeginLoc(), S.getLocForEndOfToken(RHSInt->getLocation()));
+  llvm::StringRef ExprStr =
+  Lexer::getSourceText(OpRange, S.getSourceManager(), S.getLangOpts());
+
+  if (S.getLangOpts().CPlusPlus) {
+// Do not diagnose binary literals
+if (ExprStr.find("0b") != llvm::StringRef::npos)
+  return;
+// Do not diagnose if xor keyword is used
+if (ExprStr.find("xor") != llvm::StringRef::npos)
+  return;
+  }
+
+  const llvm::APInt &LeftSideValue = LHSInt->getValue();
+  const llvm::APInt &RightSideValue = RHSInt->getValue();
+  const int64_t RightSideIntValue = RightSideValue.getSExtValue();
+  if (RightSideIntValue < 1)
+return;
+
+  if (LeftSideValue == 2 || LeftSideValue == 10) {
+llvm::APInt XorValue = LeftSideValue ^ RightSideValue;
+std::string LHSStr = Lexer::getSourceText(
+CharSourceRange::getTokenRange(LHSInt->getSourceRange()),
+S.getSourceManager(), S.getLangOpts());
+std::string RHSStr = Lexer::getSourceText(
+CharSourceRange::getTokenRange(RHSInt->getSourceRange()),
+S.getSourceManager(), S.getLangOpts());
+if (LeftSideValue == 2) {
+  llvm::APInt PowValue = (LeftSideValue - 1) << RightSideValue;
+  std::string SuggestedExpr = "1<<" + RHSStr;
+  S.Diag(Loc, diag::warn_xor_used_as_pow_base_two)
+  << ExprStr << XorValue.toString(10, true) << SuggestedExpr
+  << PowValue.toString(10, true)
+  << FixItHint::CreateReplacement(OpRange, SuggestedExpr);
+} else if (LeftSideValue == 10) {
+  std::string SuggestedValue = "1" + std::string(RightSideIntValue, '0');
+  S.Diag(Loc, diag::warn_xor_used_as_pow_base_ten)
+  << ExprStr << XorValue.toString(10, true) << SuggestedValue
+  << FixItHint::CreateReplacement(OpRange, SuggestedValue);
+}
+
+S.Diag(Loc, diag::note_xor_used_as_pow_silence)
+<< ("(" + LHSStr + ") ^ " + RHSStr);
+  }
+}
+
 QualType Sema::CheckVectorLogicalOperands(ExprResult &LHS, ExprResult &RHS,
   SourceLocation Loc) {
   // Ensure that either both operands are of the same vector type, or
@@ -10933,6 +10990,9 @@
   if (Opc == BO_And)
 diagnoseLogicalNotOnLHSofCheck(*this, LHS, RHS, Loc, Opc);
 
+  if (Opc == BO_Xor)
+diagnoseXorMisusedAsPow(*this, LHS, RHS, Loc);
+
   ExprResult LHSResult = LHS, RHSResult = RHS;
   QualType compType = UsualArithmeticConversions(LHSResult, RHSResult,
  IsCompAssign);
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ incl

[PATCH] D63437: [CodeGen][ARM] Fix FP16 vector coercion

2019-06-17 Thread Mikhail Maltsev via Phabricator via cfe-commits
miyuki created this revision.
miyuki added reviewers: eli.friedman, olista01.
Herald added subscribers: kristof.beyls, javed.absar.
Herald added a project: clang.

When a function argument or return type is a homogeneous aggregate
which contains an FP16 vector but the target does not support FP16
operations natively, the type must be converted into an array of
integer vectors by then front end (otherwise LLVM will handle FP16
vectors incorrectly by scalarizing them and promoting FP16 to float,
see https://reviews.llvm.org/D50507).

Currently the logic for checking whether or not a given homogeneous
aggregate contains FP16 vectors is incorrect: it only looks at the
type of the first vector.

This patch fixes the issue by adding a new method
ARMABIInfo::containsAnyFP16Vectors and using it. The traversal logic
of this method is largely the same as in
ABIInfo::isHomogeneousAggregate.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D63437

Files:
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGen/arm-vfp16-arguments2.cpp

Index: clang/test/CodeGen/arm-vfp16-arguments2.cpp
===
--- /dev/null
+++ clang/test/CodeGen/arm-vfp16-arguments2.cpp
@@ -0,0 +1,63 @@
+// RUN: %clang_cc1 -triple armv7a--none-eabi -target-abi aapcs \
+// RUN:   -mfloat-abi soft -target-feature +neon -emit-llvm -o - -O1 %s \
+// RUN:   | FileCheck %s --check-prefix=CHECK-SOFT
+// RUN: %clang_cc1 -triple armv7a--none-eabi -target-abi aapcs \
+// RUN:   -mfloat-abi hard -target-feature +neon -emit-llvm -o - -O1 %s \
+// RUN:   | FileCheck %s --check-prefix=CHECK-HARD
+// RUN: %clang_cc1 -triple armv7a--none-eabi -target-abi aapcs \
+// RUN:   -mfloat-abi hard -target-feature +neon -target-feature +fullfp16 \
+// RUN:   -emit-llvm -o - -O1 %s \
+// RUN:   | FileCheck %s --check-prefix=CHECK-FULL
+
+typedef float float32_t;
+typedef __fp16 float16_t;
+typedef __attribute__((neon_vector_type(2))) float32_t float32x2_t;
+typedef __attribute__((neon_vector_type(4))) float16_t float16x4_t;
+
+struct S1 {
+  float32x2_t M1;
+  float16x4_t M2;
+};
+
+struct B1 { float32x2_t M; };
+struct B2 { float16x4_t M; };
+
+struct S2 : B1, B2 {};
+
+struct S3 : B1 {
+  float16x4_t M;
+};
+
+struct S4 : B1 {
+  B2 M[1];
+};
+
+// S5 does not contain any FP16 vectors
+struct S5 : B1 {
+  B1 M[1];
+};
+
+// CHECK-SOFT: define void @_Z2f12S1(%struct.S1* noalias nocapture sret %agg.result, [2 x i64] %s1.coerce)
+// CHECK-HARD: define arm_aapcs_vfpcc [2 x <2 x i32>] @_Z2f12S1([2 x <2 x i32>] returned %s1.coerce)
+// CHECK-FULL: define arm_aapcs_vfpcc %struct.S1 @_Z2f12S1(%struct.S1 returned %s1.coerce)
+struct S1 f1(struct S1 s1) { return s1; }
+
+// CHECK-SOFT: define void @_Z2f22S2(%struct.S2* noalias nocapture sret %agg.result, [4 x i32] %s2.coerce)
+// CHECK-HARD: define arm_aapcs_vfpcc [2 x <2 x i32>] @_Z2f22S2([2 x <2 x i32>] returned %s2.coerce)
+// CHECK-FULL: define arm_aapcs_vfpcc %struct.S2 @_Z2f22S2(%struct.S2 returned %s2.coerce)
+struct S2 f2(struct S2 s2) { return s2; }
+
+// CHECK-SOFT: define void @_Z2f32S3(%struct.S3* noalias nocapture sret %agg.result, [2 x i64] %s3.coerce)
+// CHECK-HARD: define arm_aapcs_vfpcc [2 x <2 x i32>] @_Z2f32S3([2 x <2 x i32>] returned %s3.coerce)
+// CHECK-FULL: define arm_aapcs_vfpcc %struct.S3 @_Z2f32S3(%struct.S3 returned %s3.coerce)
+struct S3 f3(struct S3 s3) { return s3; }
+
+// CHECK-SOFT: define void @_Z2f42S4(%struct.S4* noalias nocapture sret %agg.result, [2 x i64] %s4.coerce)
+// CHECK-HARD: define arm_aapcs_vfpcc [2 x <2 x i32>] @_Z2f42S4([2 x <2 x i32>] returned %s4.coerce)
+// CHECK-FULL: define arm_aapcs_vfpcc %struct.S4 @_Z2f42S4(%struct.S4 returned %s4.coerce)
+struct S4 f4(struct S4 s4) { return s4; }
+
+// CHECK-SOFT: define void @_Z2f52S5(%struct.S5* noalias nocapture sret %agg.result, [2 x i64] %s5.coerce)
+// CHECK-HARD: define arm_aapcs_vfpcc %struct.S5 @_Z2f52S5(%struct.S5 returned %s5.coerce)
+// CHECK-FULL: define arm_aapcs_vfpcc %struct.S5 @_Z2f52S5(%struct.S5 returned %s5.coerce)
+struct S5 f5(struct S5 s5) { return s5; }
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -5607,6 +5607,7 @@
   uint64_t Members) const;
   ABIArgInfo coerceIllegalVector(QualType Ty) const;
   bool isIllegalVectorType(QualType Ty) const;
+  bool containsAnyFP16Vectors(QualType Ty) const;
 
   bool isHomogeneousAggregateBaseType(QualType Ty) const override;
   bool isHomogeneousAggregateSmallEnough(const Type *Ty,
@@ -5806,9 +5807,7 @@
   // Base can be a floating-point or a vector.
   if (const VectorType *VT = Base->getAs()) {
 // FP16 vectors should be converted to integer vectors
-if (!getTarget().hasLegalHalfType() &&
-(VT->getElementType()->isFloat16Type() ||
-  VT->getElementType()->isHalfType())) {
+if (!getTarget().hasLegalHa

[PATCH] D63039: Various improvements to Clang MSVC Visualizers

2019-06-17 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM! Thank you for the fixes, Mike!


Repository:
  rC Clang

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

https://reviews.llvm.org/D63039



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


[PATCH] D63438: [analyzer] print() JSONify: ProgramPoint revision

2019-06-17 Thread Csaba Dabis via Phabricator via cfe-commits
Charusso created this revision.
Charusso added a reviewer: NoQ.
Charusso added a project: clang.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, 
mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun.

Now we also print out the filename with its path.


Repository:
  rC Clang

https://reviews.llvm.org/D63438

Files:
  clang/lib/Analysis/ProgramPoint.cpp


Index: clang/lib/Analysis/ProgramPoint.cpp
===
--- clang/lib/Analysis/ProgramPoint.cpp
+++ clang/lib/Analysis/ProgramPoint.cpp
@@ -55,7 +55,8 @@
   }
 
   Out << "{ \"line\": " << SM.getExpansionLineNumber(Loc)
-  << ", \"column\": " << SM.getExpansionColumnNumber(Loc) << " }";
+  << ", \"column\": " << SM.getExpansionColumnNumber(Loc)
+  << ", \"file\": \"" << SM.getFilename(Loc) << "\" }";
 }
 
 void ProgramPoint::printJson(llvm::raw_ostream &Out, const char *NL) const {


Index: clang/lib/Analysis/ProgramPoint.cpp
===
--- clang/lib/Analysis/ProgramPoint.cpp
+++ clang/lib/Analysis/ProgramPoint.cpp
@@ -55,7 +55,8 @@
   }
 
   Out << "{ \"line\": " << SM.getExpansionLineNumber(Loc)
-  << ", \"column\": " << SM.getExpansionColumnNumber(Loc) << " }";
+  << ", \"column\": " << SM.getExpansionColumnNumber(Loc)
+  << ", \"file\": \"" << SM.getFilename(Loc) << "\" }";
 }
 
 void ProgramPoint::printJson(llvm::raw_ostream &Out, const char *NL) const {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D63437: [CodeGen][ARM] Fix FP16 vector coercion

2019-06-17 Thread Oliver Stannard (Linaro) via Phabricator via cfe-commits
ostannard added a comment.

This doesn't look like the right pace to fix this - the backend can handle 
vectors of i8 and i16, which are also not legal types, so why can't it 
correctly handle vectors of f16?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63437



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


[PATCH] D61333: [ASTImporter] Fix LLDB lookup in transparent ctx and with ext src

2019-06-17 Thread Gabor Marton via Phabricator via cfe-commits
martong updated this revision to Diff 205093.
martong added a comment.

- Fix regression of TestFormatters.py


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61333

Files:
  clang/lib/AST/ASTImporter.cpp
  clang/unittests/AST/ASTImporterTest.cpp
  lldb/packages/Python/lldbsuite/test/lang/c/ast/Makefile
  lldb/packages/Python/lldbsuite/test/lang/c/ast/TestAST.py
  lldb/packages/Python/lldbsuite/test/lang/c/ast/main.c
  lldb/packages/Python/lldbsuite/test/lang/c/modules/main.c
  lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp

Index: lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
@@ -609,6 +609,8 @@
   if (!original_decl_context)
 return;
 
+  // Indicates whether we skipped any Decls of the original DeclContext.
+  bool SkippedDecls = false;
   for (TagDecl::decl_iterator iter = original_decl_context->decls_begin();
iter != original_decl_context->decls_end(); ++iter) {
 Decl *decl = *iter;
@@ -637,21 +639,22 @@
 
 m_ast_importer_sp->RequireCompleteType(copied_field_type);
   }
-
-  DeclContext *decl_context_non_const =
-  const_cast(decl_context);
-
-  if (copied_decl->getDeclContext() != decl_context) {
-if (copied_decl->getDeclContext()->containsDecl(copied_decl))
-  copied_decl->getDeclContext()->removeDecl(copied_decl);
-copied_decl->setDeclContext(decl_context_non_const);
-  }
-
-  if (!decl_context_non_const->containsDecl(copied_decl))
-decl_context_non_const->addDeclInternal(copied_decl);
+} else {
+  SkippedDecls = true;
 }
   }
 
+  // CopyDecl may build a lookup table which may set up ExternalLexicalStorage
+  // to false.  However, since we skipped some of the external Decls we must
+  // set it back!
+  if (SkippedDecls) {
+decl_context->setHasExternalLexicalStorage(true);
+// This sets HasLazyExternalLexicalLookups to true.  By setting this bit we
+// ensure that the lookup table is rebuilt, which means the external source
+// is consulted again when a clang::DeclContext::lookup is called.
+const_cast(decl_context)->setMustBuildLookupTable();
+  }
+
   return;
 }
 
Index: lldb/packages/Python/lldbsuite/test/lang/c/modules/main.c
===
--- lldb/packages/Python/lldbsuite/test/lang/c/modules/main.c
+++ lldb/packages/Python/lldbsuite/test/lang/c/modules/main.c
@@ -5,11 +5,11 @@
 typedef struct {
 int a;
 int b;
-} FILE;
+} MYFILE;
 
 int main()
 {
-FILE *myFile = malloc(sizeof(FILE));
+MYFILE *myFile = malloc(sizeof(MYFILE));
 
 myFile->a = 5;
 myFile->b = 9;
Index: lldb/packages/Python/lldbsuite/test/lang/c/ast/main.c
===
--- /dev/null
+++ lldb/packages/Python/lldbsuite/test/lang/c/ast/main.c
@@ -0,0 +1,5 @@
+int main()
+{
+int a = 0; // Set breakpoint 0 here.
+return 0;
+}
Index: lldb/packages/Python/lldbsuite/test/lang/c/ast/TestAST.py
===
--- /dev/null
+++ lldb/packages/Python/lldbsuite/test/lang/c/ast/TestAST.py
@@ -0,0 +1,77 @@
+"""Test that importing modules in C works as expected."""
+
+from __future__ import print_function
+
+
+from distutils.version import StrictVersion
+import os
+import time
+import platform
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class CModulesTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+@skipIfFreeBSD
+@skipIfLinux
+@skipIfWindows
+@skipIfNetBSD
+@skipIf(macos_version=["<", "10.12"])
+def test_expr(self):
+self.build()
+exe = self.getBuildArtifact("a.out")
+self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
+
+lldbutil.run_break_set_by_file_and_line(
+self, "main.c", self.line, num_expected_locations=1, loc_exact=True)
+
+self.runCmd("run", RUN_SUCCEEDED)
+
+# The stop reason of the thread should be breakpoint.
+self.expect("thread list", STOPPED_DUE_TO_BREAKPOINT,
+substrs=['stopped',
+ 'stop reason = breakpoint'])
+
+# The breakpoint should have a hit count of 1.
+self.expect("breakpoint list -f", BREAKPOINT_HIT_ONCE,
+substrs=[' resolved, hit count = 1'])
+
+# Enable logging of the imported AST.
+log_file = os.path.join(self.getBuildDir(), "lldb-ast-log.txt")
+self.runCmd("log enable lldb ast -f '%s'" % log_file)
+
+self.expect(
+"expr -l objc++ -- @import Darwin; 3",
+VARIABLE

r363573 - [Remarks] Extend -fsave-optimization-record to specify the format

2019-06-17 Thread Francis Visoiu Mistrih via cfe-commits
Author: thegameg
Date: Mon Jun 17 09:06:00 2019
New Revision: 363573

URL: http://llvm.org/viewvc/llvm-project?rev=363573&view=rev
Log:
[Remarks] Extend -fsave-optimization-record to specify the format

Use -fsave-optimization-record= to specify a different format
than the default, which is YAML.

For now, only YAML is supported.

Modified:
cfe/trunk/docs/UsersManual.rst
cfe/trunk/include/clang/Basic/CodeGenOptions.h
cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
cfe/trunk/include/clang/Driver/CC1Options.td
cfe/trunk/include/clang/Driver/Options.td
cfe/trunk/lib/CodeGen/BackendUtil.cpp
cfe/trunk/lib/CodeGen/CodeGenAction.cpp
cfe/trunk/lib/Driver/ToolChains/Clang.cpp
cfe/trunk/lib/Driver/ToolChains/Darwin.cpp
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/test/CodeGen/opt-record-MIR.c
cfe/trunk/test/CodeGen/opt-record.c
cfe/trunk/test/Driver/darwin-ld.c
cfe/trunk/test/Driver/opt-record.c

Modified: cfe/trunk/docs/UsersManual.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/UsersManual.rst?rev=363573&r1=363572&r2=363573&view=diff
==
--- cfe/trunk/docs/UsersManual.rst (original)
+++ cfe/trunk/docs/UsersManual.rst Mon Jun 17 09:06:00 2019
@@ -324,13 +324,21 @@ output format of the diagnostics that it
 
 .. _opt_fsave-optimization-record:
 
-**-fsave-optimization-record**
-   Write optimization remarks to a YAML file.
+.. option:: -fsave-optimization-record[=]
+
+   Write optimization remarks to a separate file.
 
This option, which defaults to off, controls whether Clang writes
-   optimization reports to a YAML file. By recording diagnostics in a file,
-   using a structured YAML format, users can parse or sort the remarks in a
-   convenient way.
+   optimization reports to a separate file. By recording diagnostics in a file,
+   users can parse or sort the remarks in a convenient way.
+
+   By default, the serialization format is YAML.
+
+   The supported serialization formats are:
+
+   -  .. _opt_fsave_optimization_record_yaml:
+
+  ``-fsave-optimization-record=yaml``: A structured YAML format.
 
 .. _opt_foptimization-record-file:
 

Modified: cfe/trunk/include/clang/Basic/CodeGenOptions.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/CodeGenOptions.h?rev=363573&r1=363572&r2=363573&view=diff
==
--- cfe/trunk/include/clang/Basic/CodeGenOptions.h (original)
+++ cfe/trunk/include/clang/Basic/CodeGenOptions.h Mon Jun 17 09:06:00 2019
@@ -246,6 +246,9 @@ public:
   /// records.
   std::string OptRecordPasses;
 
+  /// The format used for serializing remarks (default: YAML)
+  std::string OptRecordFormat;
+
   /// The name of the partition that symbols are assigned to, specified with
   /// -fsymbol-partition (see https://lld.llvm.org/Partitions.html).
   std::string SymbolPartition;

Modified: cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td?rev=363573&r1=363572&r2=363573&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td Mon Jun 17 09:06:00 
2019
@@ -230,6 +230,8 @@ def err_drv_emit_llvm_link : Error<
"-emit-llvm cannot be used when linking">;
 def err_drv_optimization_remark_pattern : Error<
   "in pattern '%1': %0">;
+def err_drv_optimization_remark_format : Error<
+  "unknown remark serializer format: '%0'">;
 def err_drv_no_neon_modifier : Error<"[no]neon is not accepted as modifier, 
please use [no]simd instead">;
 def err_drv_invalid_omp_target : Error<"OpenMP target is invalid: '%0'">;
 def err_drv_omp_host_ir_file_not_found : Error<

Modified: cfe/trunk/include/clang/Driver/CC1Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/CC1Options.td?rev=363573&r1=363572&r2=363573&view=diff
==
--- cfe/trunk/include/clang/Driver/CC1Options.td (original)
+++ cfe/trunk/include/clang/Driver/CC1Options.td Mon Jun 17 09:06:00 2019
@@ -636,6 +636,8 @@ def opt_record_file : Separate<["-"], "o
   HelpText<"File name to use for YAML optimization record output">;
 def opt_record_passes : Separate<["-"], "opt-record-passes">,
   HelpText<"Only record remark information for passes whose names match the 
given regular expression">;
+def opt_record_format : Separate<["-"], "opt-record-format">,
+  HelpText<"The format used for serializing remarks (default: YAML)">;
 
 def print_stats : Flag<["-"], "print-stats">,
   HelpText<"Print performance metrics and statistics">;

Modified: cfe/trunk/include/clang/Driver/Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/cla

r363574 - Various improvements to Clang MSVC Visualizer

2019-06-17 Thread Mike Spertus via cfe-commits
Author: mps
Date: Mon Jun 17 09:12:45 2019
New Revision: 363574

URL: http://llvm.org/viewvc/llvm-project?rev=363574&view=rev
Log:
Various improvements to Clang MSVC Visualizer

This change adds/improves MSVC visualizers for many Clang types, including 
array types, trailing return types in function, deduction guides, a fix for 
OpaquePtr, etc. It also replaces all of the view(deref) with the "na" 
formatter, which is a better built-in natvis technique for doing the same 
thing. 

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


Modified:
cfe/trunk/utils/ClangVisualizers/clang.natvis

Modified: cfe/trunk/utils/ClangVisualizers/clang.natvis
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/ClangVisualizers/clang.natvis?rev=363574&r1=363573&r2=363574&view=diff
==
--- cfe/trunk/utils/ClangVisualizers/clang.natvis (original)
+++ cfe/trunk/utils/ClangVisualizers/clang.natvis Mon Jun 17 09:12:45 2019
@@ -27,18 +27,32 @@ For later versions of Visual Studio, no
 {*(clang::PointerType *)this}
 {*(clang::LValueReferenceType *)this}
 {*(clang::RValueReferenceType *)this}
+{(clang::ConstantArrayType *)this,na}
+{(clang::ConstantArrayType 
*)this,view(left)na}
+{(clang::ConstantArrayType 
*)this,view(right)na}
+{(clang::IncompleteArrayType *)this,na}
+{(clang::IncompleteArrayType 
*)this,view(left)na}
+{(clang::IncompleteArrayType 
*)this,view(right)na}
 {*(clang::AttributedType *)this}
+{(clang::DecayedType *)this,na}
+{(clang::DecayedType *)this,view(left)na}
+{(clang::DecayedType *)this,view(right)na}
 {*(clang::TemplateTypeParmType *)this}
+{*(clang::TemplateTypeParmType 
*)this,view(cpp)}
 {*(clang::SubstTemplateTypeParmType *)this}
 {*(clang::RecordType *)this}
 {*(clang::RecordType *)this,view(cpp)}
-{*(clang::FunctionProtoType *)this}
+{(clang::FunctionProtoType *)this,na}
+{(clang::FunctionProtoType 
*)this,view(left)na}
+{(clang::FunctionProtoType 
*)this,view(right)na}
 {*(clang::TemplateSpecializationType *)this}
 {*(clang::DeducedTemplateSpecializationType 
*)this}
 {*(clang::InjectedClassNameType *)this}
 {*(clang::PackExpansionType *)this}
 {*(clang::LocInfoType *)this}
 {*this,view(poly)}
+{*this,view(cpp)}
+
 No visualizer yet for 
{(clang::Type::TypeClass)TypeBits.TC,en}Type 
 Dependent{" ",sb}
 
@@ -62,13 +76,16 @@ For later versions of Visual Studio, no
 {*this,view(cmn)}  {{{*this,view(poly)}}}
 
   (clang::Type::TypeClass)TypeBits.TC
-  *this,view(flags)
+  this,view(flags)na
   CanonicalType
   *(clang::BuiltinType 
*)this
   *(clang::PointerType 
*)this
   *(clang::LValueReferenceType
 *)this
   *(clang::RValueReferenceType
 *)this
+  (clang::ConstantArrayType
 *)this
+  (clang::IncompleteArrayType
 *)this
   *(clang::AttributedType
 *)this
+  (clang::DecayedType 
*)this
   (clang::TemplateTypeParmType
 *)this
   (clang::SubstTemplateTypeParmType
 *)this
   (clang::RecordType 
*)this
@@ -80,6 +97,28 @@ For later versions of Visual Studio, no
   (clang::LocInfoType 
*)this
 
   
+  
+
+  ElementType
+
+  
+  
+{ElementType,view(cpp)}
+[{Size}]
+{ElementType,view(cpp)}[{Size}]
+
+  Size
+  (clang::ArrayType *)this
+
+  
+  
+{ElementType,view(cpp)}
+[]
+{ElementType,view(cpp)}[]
+
+  (clang::ArrayType *)this
+
+  
   
 {PointeeType, view(poly)} *
 
@@ -110,9 +149,9 @@ For later versions of Visual Studio, no
   
   
   
-{(clang::Decl::Kind)DeclKind,en}Decl
+
{(clang::Decl::Kind)DeclContextBits.DeclKind,en}Decl
 
-  (clang::Decl::Kind)DeclKind,en
+  (clang::Decl::Kind)DeclContextBits.DeclKind,en
   
 
 
@@ -147,10 +186,14 @@ For later versions of Visual Studio, no
 {*this,view(TorC)} 
{*this,view(MaybeEllipses)}{Name,view(cpp)} 
   
   
-template{TemplateParams,view(deref)} 
{*TemplatedDecl};
+template{TemplateParams,na} 
{*TemplatedDecl};
+
+  TemplateParams,na
+  TemplatedDecl,na
+
   
   
-{Storage,view(deref)}
+{Storage,na}
 
   Storage
 
@@ -174,7 +217,7 @@ For later versions of Visual Studio, no
 
   
   
-{*decl,view(cpp)}
+{decl,view(cpp)na}
 {*decl}
 
   *(clang::Type *)this, view(cmn)
@@ -182,8 +225,8 @@ For later versions of Visual Studio, no
 
   
   
-{*(clang::TagType 
*)this,view(cpp)}
-{*(clang::TagType *)this}
+{(clang::TagType 
*)this,view(cpp)na}
+{(clang::TagType *)this,na}
 
   *(clang::TagType *)this
 
@@ -198,7 +241,8 @@ For later versions of Visual Studio, no
   
   
-{ResultType,view(cpp)}
+
+{ResultType,view(cpp)}
 
 {*(clang::QualType 
*)(this+1),view(cpp)}{*this,view(parm1)}
 
@@ -211,7 +255,9 @@ For later versions of Visual Studio, no
 

[PATCH] D61333: [ASTImporter] Fix LLDB lookup in transparent ctx and with ext src

2019-06-17 Thread Gabor Marton via Phabricator via cfe-commits
martong added a comment.

About the regression of TestFormatters.py: I realized that the problem is about 
the wrong implementation of the ExternalASTSource interface.
In the implementation of FindExternalLexicalDecls of this interface, we simply 
ignored those cases when the given predicate (passed as a param) is false. When 
that happens, that means we still have some more external decls which should be 
dug up by the upcoming calls of DeclContext::lookup. The fix is about to 
indicate this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61333



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


[PATCH] D63039: Various improvements to Clang MSVC Visualizers

2019-06-17 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL363574: Various improvements to Clang MSVC Visualizer 
(authored by mps, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D63039?vs=203649&id=205097#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D63039

Files:
  cfe/trunk/utils/ClangVisualizers/clang.natvis

Index: cfe/trunk/utils/ClangVisualizers/clang.natvis
===
--- cfe/trunk/utils/ClangVisualizers/clang.natvis
+++ cfe/trunk/utils/ClangVisualizers/clang.natvis
@@ -27,18 +27,32 @@
 {*(clang::PointerType *)this}
 {*(clang::LValueReferenceType *)this}
 {*(clang::RValueReferenceType *)this}
+{(clang::ConstantArrayType *)this,na}
+{(clang::ConstantArrayType *)this,view(left)na}
+{(clang::ConstantArrayType *)this,view(right)na}
+{(clang::IncompleteArrayType *)this,na}
+{(clang::IncompleteArrayType *)this,view(left)na}
+{(clang::IncompleteArrayType *)this,view(right)na}
 {*(clang::AttributedType *)this}
+{(clang::DecayedType *)this,na}
+{(clang::DecayedType *)this,view(left)na}
+{(clang::DecayedType *)this,view(right)na}
 {*(clang::TemplateTypeParmType *)this}
+{*(clang::TemplateTypeParmType *)this,view(cpp)}
 {*(clang::SubstTemplateTypeParmType *)this}
 {*(clang::RecordType *)this}
 {*(clang::RecordType *)this,view(cpp)}
-{*(clang::FunctionProtoType *)this}
+{(clang::FunctionProtoType *)this,na}
+{(clang::FunctionProtoType *)this,view(left)na}
+{(clang::FunctionProtoType *)this,view(right)na}
 {*(clang::TemplateSpecializationType *)this}
 {*(clang::DeducedTemplateSpecializationType *)this}
 {*(clang::InjectedClassNameType *)this}
 {*(clang::PackExpansionType *)this}
 {*(clang::LocInfoType *)this}
 {*this,view(poly)}
+{*this,view(cpp)}
+
 No visualizer yet for {(clang::Type::TypeClass)TypeBits.TC,en}Type 
 Dependent{" ",sb}
 
@@ -62,13 +76,16 @@
 {*this,view(cmn)}  {{{*this,view(poly)}}}
 
   (clang::Type::TypeClass)TypeBits.TC
-  *this,view(flags)
+  this,view(flags)na
   CanonicalType
   *(clang::BuiltinType *)this
   *(clang::PointerType *)this
   *(clang::LValueReferenceType *)this
   *(clang::RValueReferenceType *)this
+  (clang::ConstantArrayType *)this
+  (clang::IncompleteArrayType *)this
   *(clang::AttributedType *)this
+  (clang::DecayedType *)this
   (clang::TemplateTypeParmType *)this
   (clang::SubstTemplateTypeParmType *)this
   (clang::RecordType *)this
@@ -80,6 +97,28 @@
   (clang::LocInfoType *)this
 
   
+  
+
+  ElementType
+
+  
+  
+{ElementType,view(cpp)}
+[{Size}]
+{ElementType,view(cpp)}[{Size}]
+
+  Size
+  (clang::ArrayType *)this
+
+  
+  
+{ElementType,view(cpp)}
+[]
+{ElementType,view(cpp)}[]
+
+  (clang::ArrayType *)this
+
+  
   
 {PointeeType, view(poly)} *
 
@@ -110,9 +149,9 @@
   
   
   
-{(clang::Decl::Kind)DeclKind,en}Decl
+{(clang::Decl::Kind)DeclContextBits.DeclKind,en}Decl
 
-  (clang::Decl::Kind)DeclKind,en
+  (clang::Decl::Kind)DeclContextBits.DeclKind,en
   
 
 
@@ -147,10 +186,14 @@
 {*this,view(TorC)} {*this,view(MaybeEllipses)}{Name,view(cpp)} 
   
   
-template{TemplateParams,view(deref)} {*TemplatedDecl};
+template{TemplateParams,na} {*TemplatedDecl};
+
+  TemplateParams,na
+  TemplatedDecl,na
+
   
   
-{Storage,view(deref)}
+{Storage,na}
 
   Storage
 
@@ -174,7 +217,7 @@
 
   
   
-{*decl,view(cpp)}
+{decl,view(cpp)na}
 {*decl}
 
   *(clang::Type *)this, view(cmn)
@@ -182,8 +225,8 @@
 
   
   
-{*(clang::TagType *)this,view(cpp)}
-{*(clang::TagType *)this}
+{(clang::TagType *)this,view(cpp)na}
+{(clang::TagType *)this,na}
 
   *(clang::TagType *)this
 
@@ -198,7 +241,8 @@
   
   
-{ResultType,view(cpp)}
+
+{ResultType,view(cpp)}
 
 {*(clang::QualType *)(this+1),view(cpp)}{*this,view(parm1)}
 
@@ -211,7 +255,9 @@
 , {*((clang::QualType *)(this+1)+4),view(cpp)}{*this,view(parm5)}
 
 , /* expand for more params */
-{*this,view(retType)}({*this,view(parm0)})
+({*this,view(parm0)}) -> {ResultType,view(cpp)}
+({*this,view(parm0)})
+{this,view(left)na}{this,view(right)na}
 
   ResultType
   
@@ -226,8 +272,24 @@
   *(clang::Type *)this, view(cmn)
 
   
+
+  
+{OriginalTy} adjusted to {AdjustedTy}
+
+  OriginalTy
+  AdjustedTy
+
+  
+  
+{OriginalTy,view(left)}
+{OriginalTy,view(right)}
+{OriginalTy}
+
+  (clang::AdjustedType *)this
+
+  
   
-{*TTPDecl}
+{TTPDecl->

[PATCH] D63437: [CodeGen][ARM] Fix FP16 vector coercion

2019-06-17 Thread Mikhail Maltsev via Phabricator via cfe-commits
miyuki added a comment.

In D63437#1546312 , @ostannard wrote:

> This doesn't look like the right pace to fix this - the backend can handle 
> vectors of i8 and i16, which are also not legal types, so why can't it 
> correctly handle vectors of f16?


IIRC, we decided to handle this in the frontend for consistency with scalar 
fp16 handling: https://reviews.llvm.org/D49987


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63437



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


[PATCH] D57086: Ignore trailing NullStmts in StmtExprs for GCC compatibility

2019-06-17 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

In D57086#1535873 , @domdom wrote:

> Sorry I it's taken me a while to get back to this work. I've rebased the 
> changes and taken advantage of the refactoring to stop modifying the 
> CompoundStmt after creating it. This definitely simplified the changes 
> required in Stmt.h, which is nice.
>
> I've addressed the the need to update the TreeTransformer for the template 
> case, and added a test case for that.


Thanks! I think this is looking good to me, but you should wait for the other 
reviewers before committing in case they have further concerns.

> Something I should ask, it seems like GCC only ignores the NullStmts at the 
> end if it's in C mode. Should clang match this behaviour exactly?

I can't think of a reason that this should only happen in C mode, can you 
@rsmith?




Comment at: clang/test/SemaCXX/statements.cpp:41-52
+template
+T test7(T v) {
+  return ({ // expected-warning{{use of GNU statement expression extension}}
+  T a = v;
+  a;;;
+  });
+}

Be sure to run the patch through clang-format.


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

https://reviews.llvm.org/D57086



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


[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-06-17 Thread JF Bastien via Phabricator via cfe-commits
jfb added a comment.

As a reference: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90885


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

https://reviews.llvm.org/D63423



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


[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-06-17 Thread JF Bastien via Phabricator via cfe-commits
jfb added a comment.

I think you want to avoid warning inside macros as well, say:

  #define AWESOME(x, y) ({ blah blah blah; x ^ y; blah })
  
  AWESOME(2, 10); // probably not a bug




Comment at: test/SemaCXX/warn-xor-as-pow.cpp:13
+res = a ^ b;
+res = 2 ^ 0;
+res = 2 ^ 1; // expected-warning {{result of '2 ^ 1' is 3, maybe you mean 
'1<<1' (2)?}}

`2 ^ 0` seems like it would be a bug too?


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

https://reviews.llvm.org/D63423



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


[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-06-17 Thread JF Bastien via Phabricator via cfe-commits
jfb added a comment.

What does the suggested for for `2 ^ 32` look like? I hope it's not `1 << 32` :)


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

https://reviews.llvm.org/D63423



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


[PATCH] D63369: [AST] Fixed extraneous warnings for binary conditional operator

2019-06-17 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers added inline comments.



Comment at: clang/lib/AST/Expr.cpp:2351-2352
   return false;
 if (!Exp->getLHS())
   return true;
 return Exp->getLHS()->isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx);

rsmith wrote:
> Looks like whoever wrote this expected to handle binary conditional operators 
> here. Maybe delete this check since it's impossible for a ternary conditional 
> operator?
I agree; I assume it's not possible for this case and that they meant to handle 
`BinaryConditionalOperatorClass`.  This should make the the return value a 
simple logical and of the two sides; which more closely follows the comment 
above.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63369



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


[PATCH] D53157: Teach the IRBuilder about constrained fadd and friends

2019-06-17 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments.



Comment at: include/llvm/IR/IRBuilder.h:228
+  /// Enable/Disable use of constrained floating point math
+  void setIsConstrainedFP(bool IsCon) { IsFPConstrained = IsCon; }
+

kpn wrote:
> erichkeane wrote:
> > kpn wrote:
> > > kbarton wrote:
> > > > This is a minor quibble, but the method is setIsConstrainedFP, while 
> > > > the member is IsFPConstrained. 
> > > > I'm not sure if that is intentionally different, or an oversight. 
> > > Yeah, that's an oversight. Fixed.
> > IS this fixed?
> In my working copy, yes.
Maybe this should be more explicit about what exactly it does?  Specifically, 
it changes the behavior of the `CreateF` methods so that they use 
constrained intrinsics instead of the standard instructions.



Comment at: include/llvm/IR/IRBuilder.h:113
+CR_ToZero   ///< This corresponds to "fpround.tozero".
+  };
+

Should these have "FP" in the name somewhere?  And are they really 
IRBuilder-specific concepts, as opposed to something that should be declared as 
part of the interface for working with these intrinsics?

Also, I believe we can use explicit underlying types now in LLVM; it'd be nice 
if we didn't make `IRBuilder` unnecessarily large.



Comment at: include/llvm/IR/IRBuilder.h:255
+  /// Disable use of constrained floating point math
+  void clearIsFPConstrained() { setIsFPConstrained(false); }
+

This seems unnecessary.



Comment at: include/llvm/IR/IRBuilder.h:1138
+
+return MetadataAsValue::get(Context, RoundingMDS);
+  }

Huh?  You build an `MDNode` that wraps an `MDString` and then immediately 
extract the `MDString` from it and drop the `MDNode`?

I think you should just have a function somewhere that returns the correct 
metadata string for a `ConstrainedRoundingKind`, and then the code here is much 
more obvious.  Maybe this can be wherever you declare the enums.  You can also 
have a function that goes the other way and returns an 
`Optional`.

Function name should include `FP`.

Same points apply to the next function.


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

https://reviews.llvm.org/D53157



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


[PATCH] D53157: Teach the IRBuilder about constrained fadd and friends

2019-06-17 Thread Kevin P. Neal via Phabricator via cfe-commits
kpn marked an inline comment as done.
kpn added inline comments.



Comment at: include/llvm/IR/IRBuilder.h:113
+CR_ToZero   ///< This corresponds to "fpround.tozero".
+  };
+

rjmccall wrote:
> Should these have "FP" in the name somewhere?  And are they really 
> IRBuilder-specific concepts, as opposed to something that should be declared 
> as part of the interface for working with these intrinsics?
> 
> Also, I believe we can use explicit underlying types now in LLVM; it'd be 
> nice if we didn't make `IRBuilder` unnecessarily large.
Would it be better to use the RoundingMode and ExceptionBehavior enums in the 
ConstrainedFPIntrinsic class? These enums->strings here get turned back into 
those IntrinsicInst.h enums eventually anyway. But that means pulling in yet 
more headers in IRBuilder.h.

I admit I'm not sure what you mean with your second paragraph. Is that a way of 
saying that, for example, the relevant IntrinsicInst.h enums should be used 
instead?


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

https://reviews.llvm.org/D53157



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


[PATCH] D62619: [analyzer][IDF] Add a control dependency calculator + a new debug checker

2019-06-17 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus updated this revision to Diff 205109.
Szelethus marked 7 inline comments as done.
Szelethus added a comment.

Fixes according to reviewer comments.


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

https://reviews.llvm.org/D62619

Files:
  clang/include/clang/Analysis/Analyses/Dominators.h
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/lib/StaticAnalyzer/Checkers/DebugCheckers.cpp
  clang/test/Analysis/domtest.c
  clang/test/Analysis/domtest.cpp
  clang/unittests/Analysis/CFGDominatorTree.cpp

Index: clang/unittests/Analysis/CFGDominatorTree.cpp
===
--- clang/unittests/Analysis/CFGDominatorTree.cpp
+++ clang/unittests/Analysis/CFGDominatorTree.cpp
@@ -117,6 +117,99 @@
   EXPECT_TRUE(PostDom.dominates(nullptr, ExitBlock));
 }
 
+TEST(CFGDominatorTree, ControlDependency) {
+  const char *Code = R"(bool coin();
+
+void funcWithBranch() {
+  int x = 0;
+  if (coin()) {
+if (coin()) {
+  x = 5;
+}
+int j = 10 / x;
+(void)j;
+  }
+};)";
+  BuildResult Result = BuildCFG(Code);
+  EXPECT_EQ(BuildResult::BuiltCFG, Result.getStatus());
+
+  //  1st if  2nd if
+  //  [B5 (ENTRY)]  -> [B4] -> [B3] -> [B2] -> [B1] -> [B0 (EXIT)]
+  //\\  / /
+  // \-> /
+  //  -->
+
+  CFG *cfg = Result.getCFG();
+
+  // Sanity checks.
+  EXPECT_EQ(cfg->size(), 6u);
+
+  CFGBlock *ExitBlock = *cfg->begin();
+  EXPECT_EQ(ExitBlock, &cfg->getExit());
+
+  CFGBlock *NullDerefBlock = *(cfg->begin() + 1);
+
+  CFGBlock *SecondThenBlock = *(cfg->begin() + 2);
+
+  CFGBlock *SecondIfBlock = *(cfg->begin() + 3);
+  EXPECT_TRUE(hasStmtType(SecondIfBlock));
+
+  CFGBlock *FirstIfBlock = *(cfg->begin() + 4);
+  EXPECT_TRUE(hasStmtType(FirstIfBlock));
+
+  CFGBlock *EntryBlock = *(cfg->begin() + 5);
+  EXPECT_EQ(EntryBlock, &cfg->getEntry());
+
+  ControlDependencyCalculator Control(cfg);
+
+  EXPECT_TRUE(Control.isControlDependent(SecondThenBlock, SecondIfBlock));
+  EXPECT_TRUE(Control.isControlDependent(SecondIfBlock, FirstIfBlock));
+  EXPECT_FALSE(Control.isControlDependent(NullDerefBlock, SecondIfBlock));
+}
+
+TEST(CFGDominatorTree, ControlDependencyWithLoops) {
+  const char *Code = R"(int test3() {
+  int x,y,z;
+
+  x = y = z = 1;
+  if (x > 0) {
+while (x >= 0){
+  while (y >= x) {
+x = x-1;
+y = y/2;
+  }
+}
+  }
+  z = y;
+
+  return 0;
+})";
+  BuildResult Result = BuildCFG(Code);
+  EXPECT_EQ(BuildResult::BuiltCFG, Result.getStatus());
+
+  //   <- [B2] <-
+  //  /  \
+  // [B8 (ENTRY)] -> [B7] -> [B6] ---> [B5] -> [B4] -> [B3]
+  //   \   | \  /
+  //\  |  <-
+  // \  \
+  //  > [B1] -> [B0 (EXIT)]
+
+  CFG *cfg = Result.getCFG();
+
+  ControlDependencyCalculator Control(cfg);
+
+  auto GetBlock = [cfg] (unsigned Index) -> CFGBlock * {
+assert(Index < cfg->size());
+return *(cfg->begin() + Index);
+  };
+
+  // While not immediately obvious, the second block in fact post dominates the
+  // fifth, hence B5 is not a control dependency of 2.
+  EXPECT_FALSE(Control.isControlDependent(GetBlock(5), GetBlock(2)));
+}
+
+
 } // namespace
 } // namespace analysis
 } // namespace clang
Index: clang/test/Analysis/domtest.cpp
===
--- clang/test/Analysis/domtest.cpp
+++ clang/test/Analysis/domtest.cpp
@@ -1,6 +1,7 @@
 // RUN: %clang_analyze_cc1 %s \
 // RUN:   -analyzer-checker=debug.DumpDominators \
 // RUN:   -analyzer-checker=debug.DumpPostDominators \
+// RUN:   -analyzer-checker=debug.DumpControlDependencies \
 // RUN:   2>&1 | FileCheck %s
 
 bool coin();
@@ -20,7 +21,8 @@
 
 //  [B3 (ENTRY)]  -> [B1] -> [B2] -> [B0 (EXIT)]
 
-// CHECK:  Immediate dominance tree (Node#,IDom#):
+// CHECK:  Control dependencies (Node#,Dependency#):
+// CHECK-NEXT: Immediate dominance tree (Node#,IDom#):
 // CHECK-NEXT: (0,2)
 // CHECK-NEXT: (1,3)
 // CHECK-NEXT: (2,1)
@@ -42,13 +44,18 @@
   }
 }
 
-//> [B2] >
-//   /\
-// [B5 (ENTRY)] -> [B4] -> [B3] -

[PATCH] D63371: Rewrite ConstStructBuilder with a mechanism that can cope with splitting and updating constants.

2019-06-17 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

Isn't `[[no_unique_address]]` only significant for empty members?  I'm not sure 
why they need significant support from constant-building, since they expand to 
no meaningful initializer.

We have some code we'll hopefully be upstreaming soon that relies on being able 
to do things with address-of-position placeholder values; I'm a little worried 
that the new structure here doesn't really support them.


Repository:
  rC Clang

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

https://reviews.llvm.org/D63371



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


[PATCH] D62619: [analyzer][IDF] Add a control dependency calculator + a new debug checker

2019-06-17 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added inline comments.



Comment at: clang/include/clang/Analysis/Analyses/Dominators.h:51
+  CFGDominatorTreeImpl(CFG *cfg) {
+buildDominatorTree(cfg);
+  }

kuhar wrote:
> DomTree has a constructor that runs the builder -- why not use it directly?
That is correct, but `DomTreeBase` does not. :/



Comment at: clang/include/clang/Analysis/Analyses/Dominators.h:160
   /// changes.
   void changeImmediateDominator(CFGBlock *N, CFGBlock *NewIDom) {
 DT.changeImmediateDominator(N, NewIDom);

kuhar wrote:
> Do you need it at all? I understand it's a wrapper around dominators, but 
> this API is virtually impossible to use safely.
I agree 100%, I'll get rid of this in another patch.


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

https://reviews.llvm.org/D62619



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


[PATCH] D63369: [AST] Fixed extraneous warnings for binary conditional operator

2019-06-17 Thread Nathan Huckleberry via Phabricator via cfe-commits
Nathan-Huckleberry updated this revision to Diff 205117.
Nathan-Huckleberry added a comment.

- [AST] Cleanup of conditional operator unused warning detection


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63369

Files:
  clang/lib/AST/Expr.cpp
  clang/test/Sema/warn-binary-conditional-expression-unused.c


Index: clang/test/Sema/warn-binary-conditional-expression-unused.c
===
--- /dev/null
+++ clang/test/Sema/warn-binary-conditional-expression-unused.c
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -fsyntax-only -Wunused-value -verify %s
+int main() {
+int a;
+int b;
+a ? : b; //expected-warning{{expression result unused}}
+a ? a : b; //expected-warning{{expression result unused}}
+a ? : ++b;
+a ? a : ++b;
+++a ? : b; //expected-warning{{expression result unused}}
+++a ? a : b; //expected-warning{{expression result unused}}
+++a ? : ++b;
+++a ? a : ++b;
+return 0;
+};
+
Index: clang/lib/AST/Expr.cpp
===
--- clang/lib/AST/Expr.cpp
+++ clang/lib/AST/Expr.cpp
@@ -2346,11 +2346,12 @@
 // be being used for control flow. Only warn if both the LHS and
 // RHS are warnings.
 const ConditionalOperator *Exp = cast(this);
-if (!Exp->getRHS()->isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx))
-  return false;
-if (!Exp->getLHS())
-  return true;
-return Exp->getLHS()->isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx);
+return Exp->getLHS()->isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx) &&
+Exp->getRHS()->isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx);
+  }
+  case BinaryConditionalOperatorClass: {
+auto *Exp = cast(this);
+return Exp->getFalseExpr()->isUnusedResultAWarning(WarnE, Loc, R1, R2, 
Ctx);
   }
 
   case MemberExprClass:


Index: clang/test/Sema/warn-binary-conditional-expression-unused.c
===
--- /dev/null
+++ clang/test/Sema/warn-binary-conditional-expression-unused.c
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -fsyntax-only -Wunused-value -verify %s
+int main() {
+int a;
+int b;
+a ? : b; //expected-warning{{expression result unused}}
+a ? a : b; //expected-warning{{expression result unused}}
+a ? : ++b;
+a ? a : ++b;
+++a ? : b; //expected-warning{{expression result unused}}
+++a ? a : b; //expected-warning{{expression result unused}}
+++a ? : ++b;
+++a ? a : ++b;
+return 0;
+};
+
Index: clang/lib/AST/Expr.cpp
===
--- clang/lib/AST/Expr.cpp
+++ clang/lib/AST/Expr.cpp
@@ -2346,11 +2346,12 @@
 // be being used for control flow. Only warn if both the LHS and
 // RHS are warnings.
 const ConditionalOperator *Exp = cast(this);
-if (!Exp->getRHS()->isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx))
-  return false;
-if (!Exp->getLHS())
-  return true;
-return Exp->getLHS()->isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx);
+return Exp->getLHS()->isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx) &&
+Exp->getRHS()->isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx);
+  }
+  case BinaryConditionalOperatorClass: {
+auto *Exp = cast(this);
+return Exp->getFalseExpr()->isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx);
   }
 
   case MemberExprClass:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D62750: Show note for -Wmissing-prototypes for functions with parameters

2019-06-17 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM!




Comment at: lib/Sema/SemaDecl.cpp:13335
+? FixItHint::CreateInsertion(FTL.getRParenLoc(), 
"void")
+: FixItHint{});
 }

In this case, we could probably generate the parameter declarations for the 
user still, couldn't we? e.g.,
```
int f(); // We could insert "int i, int j" here as the fixit
int g(); // Just like we try to suggest "void" here already

int f(int i, int j) { return i + j; }
int g(void) { return 12; }
```
That could be done in a follow-up patch though.


Repository:
  rC Clang

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

https://reviews.llvm.org/D62750



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


r363585 - [clang][AST] Remove unnecessary 'const'.

2019-06-17 Thread Michael Liao via cfe-commits
Author: hliao
Date: Mon Jun 17 10:47:03 2019
New Revision: 363585

URL: http://llvm.org/viewvc/llvm-project?rev=363585&view=rev
Log:
[clang][AST] Remove unnecessary 'const'.

Modified:
cfe/trunk/include/clang/AST/Expr.h

Modified: cfe/trunk/include/clang/AST/Expr.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Expr.h?rev=363585&r1=363584&r2=363585&view=diff
==
--- cfe/trunk/include/clang/AST/Expr.h (original)
+++ cfe/trunk/include/clang/AST/Expr.h Mon Jun 17 10:47:03 2019
@@ -1024,7 +1024,7 @@ public:
 llvm_unreachable("invalid ResultKind");
   }
   ResultStorageKind getResultStorageKind() const {
-return static_cast(ConstantExprBits.ResultKind);
+return static_cast(ConstantExprBits.ResultKind);
   }
   APValue getAPValueResult() const;
 


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


[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-06-17 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 updated this revision to Diff 205121.
xbolva00 added a comment.

Fixed notes by @jfb. Thank you.


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

https://reviews.llvm.org/D63423

Files:
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaExpr.cpp
  test/Sema/warn-xor-as-pow.c
  test/SemaCXX/warn-xor-as-pow.cpp

Index: test/SemaCXX/warn-xor-as-pow.cpp
===
--- test/SemaCXX/warn-xor-as-pow.cpp
+++ test/SemaCXX/warn-xor-as-pow.cpp
@@ -0,0 +1,68 @@
+// RUN: %clang_cc1 -x c++ -fsyntax-only -verify -Wxor-as-pow %s
+// RUN: %clang_cc1 -x c++ -fsyntax-only -verify %s
+// RUN: %clang_cc1 -x c++ -fsyntax-only -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+
+#define XOR(x, y) (x ^ y)
+#define TWO 2
+#define TEN 10
+#define TWO_ULL 2ULL
+
+void test(unsigned a, unsigned b) {
+  unsigned res;
+  res = a ^ 5;
+  res = 2 ^ b;
+  res = a ^ b;
+  res = 2 ^ 0; // expected-warning {{result of '2 ^ 0' is 2, maybe you mean '1<<0' (1)?}}
+   // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:14}:"1"
+   // expected-note@-2 {{replace expression with '(2) ^ 0' to silence this warning}}
+  res = 2 ^ 1; // expected-warning {{result of '2 ^ 1' is 3, maybe you mean '1<<1' (2)?}}
+   // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:14}:"1<<1"
+   // expected-note@-2 {{replace expression with '(2) ^ 1' to silence this warning}}
+  res = 2 ^ 2; // expected-warning {{result of '2 ^ 2' is 0, maybe you mean '1<<2' (4)?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:14}:"1<<2"
+  // expected-note@-2 {{replace expression with '(2) ^ 2' to silence this warning}}
+  res = 2 ^ 8; // expected-warning {{result of '2 ^ 8' is 10, maybe you mean '1<<8' (256)?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:14}:"1<<8"
+  // expected-note@-2 {{replace expression with '(2) ^ 8' to silence this warning}}
+  res = TWO ^ 8; // expected-warning {{result of 'TWO ^ 8' is 10, maybe you mean '1<<8' (256)?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:16}:"1<<8"
+  // expected-note@-2 {{replace expression with '(TWO) ^ 8' to silence this warning}}
+  res = 2 ^ 16; // expected-warning {{result of '2 ^ 16' is 18, maybe you mean '1<<16' (65536)?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:15}:"1<<16"
+  // expected-note@-2 {{replace expression with '(2) ^ 16' to silence this warning}}
+  res = 2 ^ TEN; // expected-warning {{result of '2 ^ TEN' is 8, maybe you mean '1<= width of type}}
+  // expected-note@-1 {{replace expression with '(2) ^ 32' to silence this warning}}
+  res = 2 ^ 64; // expected-warning {{result of '2 ^ 64' is 66; maybe you mean '1<<64', but shift count >= width of type}}
+  // expected-note@-1 {{replace expression with '(2) ^ 64' to silence this warning}}
+
+  res = 10 ^ 0;
+  res = 10 ^ 1; // expected-warning {{result of '10 ^ 1' is 11, maybe you mean '10'?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:15}:"10"
+  // expected-note@-2 {{replace expression with '(10) ^ 1' to silence this warning}}
+  res = 10 ^ 2; // expected-warning {{result of '10 ^ 2' is 8, maybe you mean '100'?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:15}:"100"
+  // expected-note@-2 {{replace expression with '(10) ^ 2' to silence this warning}}
+  res = 10 ^ 4; // expected-warning {{result of '10 ^ 4' is 14, maybe you mean '1'?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:15}:"1"
+  // expected-note@-2 {{replace expression with '(10) ^ 4' to silence this warning}}
+  res = 10 ^ 10; // expected-warning {{result of '10 ^ 10' is 0, maybe you mean '100'?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:16}:"100"
+  // expected-note@-2 {{replace expression with '(10) ^ 10' to silence this warning}}
+  res = TEN ^ 10; // expected-warning {{result of 'TEN ^ 10' is 0, maybe you mean '100'?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:17}:"100"
+  // expected-note@-2 {{replace expression with '(TEN) ^ 10' to silence this warning}}
+  res = (10) ^ 10;
+  res = 10 xor 10;
+  res = XOR(10, 10);
+}
Index: test/Sema/warn-xor-as-pow.c
===
--- test/Sema/warn-xor-as-pow.c
+++ test/Sema/warn-xor-as-pow.c
@@ -0,0 +1,66 @@
+// RUN: %clang_cc1 -x c -fsyntax-only -verify -Wxor-as-pow %s
+// RUN: %clang_cc1 -x c -fsyntax-only -verify %s
+// RUN: %clang_cc1 -x c -fsyntax-only -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+
+#define XOR(x, y) (x ^ y)
+#define TWO 2
+#define TEN 10
+#define TWO_ULL 2ULL
+
+void test(unsigned a, unsigned b) {
+  unsigned res;
+  res = a ^ 5;
+  res = 2 ^ b;
+  res = a ^ b;
+  res = 2 ^ 0; // expected-warning {{result of '2 ^ 0' is 2, maybe you mean '1<<0' (1)?}}
+   // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:14}:"1"
+  

[PATCH] D63371: Rewrite ConstStructBuilder with a mechanism that can cope with splitting and updating constants.

2019-06-17 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment.

In D63371#1546500 , @rjmccall wrote:

> Isn't `[[no_unique_address]]` only significant for empty members?  I'm not 
> sure why they need significant support from constant-building, since they 
> expand to no meaningful initializer.


It also permits reuse of tail padding for non-static data members, which is the 
complexity that this patch is dealing with (in addition to improving and 
generalizing the support for non-trivial designated initializers).

> We have some code we'll hopefully be upstreaming soon that relies on being 
> able to do things with address-of-position placeholder values; I'm a little 
> worried that the new structure here doesn't really support them.

Can you say a bit more about that? (Do you want to be able to emit a constant 
that denotes a pointer to somewhere else within the same constant being 
emitted, or something like that?) I think this approach should be strictly more 
general than what we had before, but perhaps that means it can't be extended in 
the direction you need?


Repository:
  rC Clang

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

https://reviews.llvm.org/D63371



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


[PATCH] D63139: [Diagnostics] Implement -Wswitch-unreachable

2019-06-17 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: include/clang/Basic/DiagnosticGroups.td:541
 def SwitchEnum : DiagGroup<"switch-enum">;
+def SwitchUnreachable : DiagGroup<"switch-unreachable">;
 def Switch : DiagGroup<"switch">;

I don't think you need this group because there's only one diagnostic it 
controls. You can add the group directly to the warning itself.



Comment at: include/clang/Basic/DiagnosticSemaKinds.td:8192
+def warn_unreachable_stmt_in_switch : Warning<
+  "statement will be never executed">, InGroup, 
DefaultIgnore;
 def warn_bool_switch_condition : Warning<

`InGroup>` and drop the `DefaultIgnore`.



Comment at: lib/Sema/SemaStmt.cpp:864
 
+  if (CompoundStmt *CS = dyn_cast(BodyStmt)) {
+for (auto It = CS->body_begin(); It != CS->body_end(); ++It) {

`const auto *`



Comment at: lib/Sema/SemaStmt.cpp:865-866
+  if (CompoundStmt *CS = dyn_cast(BodyStmt)) {
+for (auto It = CS->body_begin(); It != CS->body_end(); ++It) {
+  auto *S = *It;
+  if (isa(S) || isa(S) || isa(S))

`for (const Stmt *S : CS->body())`



Comment at: lib/Sema/SemaStmt.cpp:871-876
+  } else if (isa(BodyStmt) || isa(BodyStmt) ||
+ isa(BodyStmt)) {
+// No warning
+  } else {
+Diag(BodyStmt->getBeginLoc(), diag::warn_unreachable_stmt_in_switch);
+  }

You should turn this into a negative predicate rather than having an empty body 
with an `else` statement.



Comment at: test/Sema/switch_unreachable.c:2
+// RUN: %clang_cc1 -x c -fsyntax-only -verify -Wswitch-unreachable %s
+// RUN: %clang_cc1 -x c -fsyntax-only -verify -Wall %s
+

Remove the `-Wall` since you want to test that this is on by default.


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

https://reviews.llvm.org/D63139



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


[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-06-17 Thread JF Bastien via Phabricator via cfe-commits
jfb added a comment.

Why have `.c` and `.cpp` tests?




Comment at: test/Sema/warn-xor-as-pow.c:43
+  res = TWO_ULL ^ 16;
+  res = 2 ^ 32; // expected-warning {{result of '2 ^ 32' is 34; maybe you mean 
'1<<32', but shift count >= width of type}}
+  // expected-note@-1 {{replace expression with '(2) ^ 32' to silence this 
warning}}

I think we want to suggest `1LL << 32` or something like that. Whatever we 
usually do with this type of suggestion.



Comment at: test/Sema/warn-xor-as-pow.c:45
+  // expected-note@-1 {{replace expression with '(2) ^ 32' to silence this 
warning}}
+  res = 2 ^ 64; // expected-warning {{result of '2 ^ 64' is 66; maybe you mean 
'1<<64', but shift count >= width of type}}
+  // expected-note@-1 {{replace expression with '(2) ^ 64' to silence this 
warning}}

This one hits a ceiling, we can't really suggest anything for this value IMO. 
Maybe we need to see if the user is doing `2^64 - 1`? In that case we can offer 
a suggestion.


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

https://reviews.llvm.org/D63423



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


[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-06-17 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 updated this revision to Diff 205126.

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

https://reviews.llvm.org/D63423

Files:
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaExpr.cpp
  test/Sema/warn-xor-as-pow.c
  test/SemaCXX/warn-xor-as-pow.cpp

Index: test/SemaCXX/warn-xor-as-pow.cpp
===
--- test/SemaCXX/warn-xor-as-pow.cpp
+++ test/SemaCXX/warn-xor-as-pow.cpp
@@ -0,0 +1,68 @@
+// RUN: %clang_cc1 -x c++ -fsyntax-only -verify -Wxor-as-pow %s
+// RUN: %clang_cc1 -x c++ -fsyntax-only -verify %s
+// RUN: %clang_cc1 -x c++ -fsyntax-only -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+
+#define XOR(x, y) (x ^ y)
+#define TWO 2
+#define TEN 10
+#define TWO_ULL 2ULL
+
+void test(unsigned a, unsigned b) {
+  unsigned res;
+  res = a ^ 5;
+  res = 2 ^ b;
+  res = a ^ b;
+  res = 2 ^ 0; // expected-warning {{result of '2 ^ 0' is 2, maybe you mean '1<<0' (1)?}}
+   // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:14}:"1"
+   // expected-note@-2 {{replace expression with '(2) ^ 0' to silence this warning}}
+  res = 2 ^ 1; // expected-warning {{result of '2 ^ 1' is 3, maybe you mean '1<<1' (2)?}}
+   // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:14}:"1<<1"
+   // expected-note@-2 {{replace expression with '(2) ^ 1' to silence this warning}}
+  res = 2 ^ 2; // expected-warning {{result of '2 ^ 2' is 0, maybe you mean '1<<2' (4)?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:14}:"1<<2"
+  // expected-note@-2 {{replace expression with '(2) ^ 2' to silence this warning}}
+  res = 2 ^ 8; // expected-warning {{result of '2 ^ 8' is 10, maybe you mean '1<<8' (256)?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:14}:"1<<8"
+  // expected-note@-2 {{replace expression with '(2) ^ 8' to silence this warning}}
+  res = TWO ^ 8; // expected-warning {{result of 'TWO ^ 8' is 10, maybe you mean '1<<8' (256)?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:16}:"1<<8"
+  // expected-note@-2 {{replace expression with '(TWO) ^ 8' to silence this warning}}
+  res = 2 ^ 16; // expected-warning {{result of '2 ^ 16' is 18, maybe you mean '1<<16' (65536)?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:15}:"1<<16"
+  // expected-note@-2 {{replace expression with '(2) ^ 16' to silence this warning}}
+  res = 2 ^ TEN; // expected-warning {{result of '2 ^ TEN' is 8, maybe you mean '1<= width of type}}
+  // expected-note@-1 {{replace expression with '(2) ^ 32' to silence this warning}}
+  res = 2 ^ 64; // expected-warning {{result of '2 ^ 64' is 66; maybe you mean '1<<64', but shift count >= width of type}}
+  // expected-note@-1 {{replace expression with '(2) ^ 64' to silence this warning}}
+
+  res = 10 ^ 0;
+  res = 10 ^ 1; // expected-warning {{result of '10 ^ 1' is 11, maybe you mean '10'?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:15}:"10"
+  // expected-note@-2 {{replace expression with '(10) ^ 1' to silence this warning}}
+  res = 10 ^ 2; // expected-warning {{result of '10 ^ 2' is 8, maybe you mean '100'?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:15}:"100"
+  // expected-note@-2 {{replace expression with '(10) ^ 2' to silence this warning}}
+  res = 10 ^ 4; // expected-warning {{result of '10 ^ 4' is 14, maybe you mean '1'?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:15}:"1"
+  // expected-note@-2 {{replace expression with '(10) ^ 4' to silence this warning}}
+  res = 10 ^ 10; // expected-warning {{result of '10 ^ 10' is 0, maybe you mean '100'?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:16}:"100"
+  // expected-note@-2 {{replace expression with '(10) ^ 10' to silence this warning}}
+  res = TEN ^ 10; // expected-warning {{result of 'TEN ^ 10' is 0, maybe you mean '100'?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:17}:"100"
+  // expected-note@-2 {{replace expression with '(TEN) ^ 10' to silence this warning}}
+  res = (10) ^ 10;
+  res = 10 xor 10;
+  res = XOR(10, 10);
+}
Index: test/Sema/warn-xor-as-pow.c
===
--- test/Sema/warn-xor-as-pow.c
+++ test/Sema/warn-xor-as-pow.c
@@ -0,0 +1,66 @@
+// RUN: %clang_cc1 -x c -fsyntax-only -verify -Wxor-as-pow %s
+// RUN: %clang_cc1 -x c -fsyntax-only -verify %s
+// RUN: %clang_cc1 -x c -fsyntax-only -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+
+#define XOR(x, y) (x ^ y)
+#define TWO 2
+#define TEN 10
+#define TWO_ULL 2ULL
+
+void test(unsigned a, unsigned b) {
+  unsigned res;
+  res = a ^ 5;
+  res = 2 ^ b;
+  res = a ^ b;
+  res = 2 ^ 0; // expected-warning {{result of '2 ^ 0' is 2, maybe you mean '1<<0' (1)?}}
+   // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:14}:"1"
+   // expected-note@-2 {{replace expression with '(2) ^ 0'

RE: r362856 - DebugInfo: Add support for 'nodebug' attribute on typedefs and alias templates

2019-06-17 Thread via cfe-commits
Is this really measurable?  All you're suppressing are the typedef DIEs
and their names; the DIEs are small, although I admit the names can take
up space.
(I'm not really objecting, it's just hard to intuit a big size benefit.)
--paulr

> -Original Message-
> From: cfe-commits [mailto:cfe-commits-boun...@lists.llvm.org] On Behalf Of
> David Blaikie via cfe-commits
> Sent: Friday, June 07, 2019 8:01 PM
> To: cfe-commits@lists.llvm.org
> Subject: r362856 - DebugInfo: Add support for 'nodebug' attribute on
> typedefs and alias templates
> 
> Author: dblaikie
> Date: Fri Jun  7 17:01:21 2019
> New Revision: 362856
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=362856&view=rev
> Log:
> DebugInfo: Add support for 'nodebug' attribute on typedefs and alias
> templates
> 
> Seems like a logical extension to me - and of interest because it might
> help reduce the debug info size of libc++ by applying this attribute to
> type traits that have a disproportionate debug info cost compared to the
> benefit (& possibly harm/confusion) they cause users.
> 
> Modified:
> cfe/trunk/include/clang/Basic/Attr.td
> cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
> cfe/trunk/test/CodeGenCXX/debug-info-nodebug.cpp
> cfe/trunk/test/Misc/pragma-attribute-supported-attributes-list.test
> cfe/trunk/test/Sema/attr-nodebug.c
> 
> Modified: cfe/trunk/include/clang/Basic/Attr.td
> URL: http://llvm.org/viewvc/llvm-
> project/cfe/trunk/include/clang/Basic/Attr.td?rev=362856&r1=362855&r2=3628
> 56&view=diff
> ==
> 
> --- cfe/trunk/include/clang/Basic/Attr.td (original)
> +++ cfe/trunk/include/clang/Basic/Attr.td Fri Jun  7 17:01:21 2019
> @@ -1434,7 +1434,7 @@ def NoCommon : InheritableAttr {
> 
>  def NoDebug : InheritableAttr {
>let Spellings = [GCC<"nodebug">];
> -  let Subjects = SubjectList<[FunctionLike, ObjCMethod, NonParmVar]>;
> +  let Subjects = SubjectList<[TypedefName, FunctionLike, ObjCMethod,
> NonParmVar]>;
>let Documentation = [NoDebugDocs];
>  }
> 
> 
> Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
> URL: http://llvm.org/viewvc/llvm-
> project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=362856&r1=362855&r2=3628
> 56&view=diff
> ==
> 
> --- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Fri Jun  7 17:01:21 2019
> @@ -1091,15 +1091,18 @@ llvm::DIType *CGDebugInfo::CreateType(co
>assert(Ty->isTypeAlias());
>llvm::DIType *Src = getOrCreateType(Ty->getAliasedType(), Unit);
> 
> +  auto *AliasDecl =
> +  cast(Ty-
> >getTemplateName().getAsTemplateDecl())
> +  ->getTemplatedDecl();
> +
> +  if (AliasDecl->hasAttr())
> +return Src;
> +
>SmallString<128> NS;
>llvm::raw_svector_ostream OS(NS);
>Ty->getTemplateName().print(OS, getPrintingPolicy(), /*qualified*/
> false);
>printTemplateArgumentList(OS, Ty->template_arguments(),
> getPrintingPolicy());
> 
> -  auto *AliasDecl =
> -  cast(Ty-
> >getTemplateName().getAsTemplateDecl())
> -  ->getTemplatedDecl();
> -
>SourceLocation Loc = AliasDecl->getLocation();
>return DBuilder.createTypedef(Src, OS.str(), getOrCreateFile(Loc),
>  getLineNumber(Loc),
> @@ -1108,15 +,20 @@ llvm::DIType *CGDebugInfo::CreateType(co
> 
>  llvm::DIType *CGDebugInfo::CreateType(const TypedefType *Ty,
>llvm::DIFile *Unit) {
> +  llvm::DIType *Underlying =
> +  getOrCreateType(Ty->getDecl()->getUnderlyingType(), Unit);
> +
> +  if (Ty->getDecl()->hasAttr())
> +return Underlying;
> +
>// We don't set size information, but do specify where the typedef was
>// declared.
>SourceLocation Loc = Ty->getDecl()->getLocation();
> 
>// Typedefs are derived from some other type.
> -  return DBuilder.createTypedef(
> -  getOrCreateType(Ty->getDecl()->getUnderlyingType(), Unit),
> -  Ty->getDecl()->getName(), getOrCreateFile(Loc), getLineNumber(Loc),
> -  getDeclContextDescriptor(Ty->getDecl()));
> +  return DBuilder.createTypedef(Underlying, Ty->getDecl()->getName(),
> +getOrCreateFile(Loc), getLineNumber(Loc),
> +getDeclContextDescriptor(Ty->getDecl()));
>  }
> 
>  static unsigned getDwarfCC(CallingConv CC) {
> 
> Modified: cfe/trunk/test/CodeGenCXX/debug-info-nodebug.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/debug-
> info-nodebug.cpp?rev=362856&r1=362855&r2=362856&view=diff
> ==
> 
> --- cfe/trunk/test/CodeGenCXX/debug-info-nodebug.cpp (original)
> +++ cfe/trunk/test/CodeGenCXX/debug-info-nodebug.cpp Fri Jun  7 17:01:21
> 2019
> @@ -1,5 +1,5 @@
> -// RUN: %clang_cc1 -DSETNODEBUG=0 -emit-llvm -debug-info-kind=limited %s
> -o - | FileCheck %s --check-prefix=YESINFO
>

[PATCH] D53157: Teach the IRBuilder about constrained fadd and friends

2019-06-17 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments.



Comment at: include/llvm/IR/IRBuilder.h:113
+CR_ToZero   ///< This corresponds to "fpround.tozero".
+  };
+

kpn wrote:
> rjmccall wrote:
> > Should these have "FP" in the name somewhere?  And are they really 
> > IRBuilder-specific concepts, as opposed to something that should be 
> > declared as part of the interface for working with these intrinsics?
> > 
> > Also, I believe we can use explicit underlying types now in LLVM; it'd be 
> > nice if we didn't make `IRBuilder` unnecessarily large.
> Would it be better to use the RoundingMode and ExceptionBehavior enums in the 
> ConstrainedFPIntrinsic class? These enums->strings here get turned back into 
> those IntrinsicInst.h enums eventually anyway. But that means pulling in yet 
> more headers in IRBuilder.h.
> 
> I admit I'm not sure what you mean with your second paragraph. Is that a way 
> of saying that, for example, the relevant IntrinsicInst.h enums should be 
> used instead?
I think it's `IRBuilder.h`'s fate to pull in the majority of the headers in 
`IR`, but even if we want to avoid that, duplicating the enums seems like a 
step too far.  If there's too much code in the header declaring that intrinsic, 
you should extract a smaller header that just declares the enums and their 
string conversions.

The second paragraph is asking for these enums to be specifically constrained 
to `uint8_t` so that we aren't wasting a ton of memory everywhere we store 
them.  `IRBuilder` doesn't have strong size constraints, but it'd still be nice 
if all these accumulated features didn't make it hundreds of bytes larger than 
it needs to be.


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

https://reviews.llvm.org/D53157



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


[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-06-17 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 marked an inline comment as done.
xbolva00 added inline comments.



Comment at: test/Sema/warn-xor-as-pow.c:45
+  // expected-note@-1 {{replace expression with '(2) ^ 32' to silence this 
warning}}
+  res = 2 ^ 64; // expected-warning {{result of '2 ^ 64' is 66; maybe you mean 
'1<<64', but shift count >= width of type}}
+  // expected-note@-1 {{replace expression with '(2) ^ 64' to silence this 
warning}}

jfb wrote:
> This one hits a ceiling, we can't really suggest anything for this value IMO. 
> Maybe we need to see if the user is doing `2^64 - 1`? In that case we can 
> offer a suggestion.
Since this requires changes in other parts of code to catch 2^64 - 1, maybe we 
can leave it as a follow-up patch?


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

https://reviews.llvm.org/D63423



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


[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-06-17 Thread JF Bastien via Phabricator via cfe-commits
jfb added inline comments.



Comment at: test/Sema/warn-xor-as-pow.c:45
+  // expected-note@-1 {{replace expression with '(2) ^ 32' to silence this 
warning}}
+  res = 2 ^ 64; // expected-warning {{result of '2 ^ 64' is 66; maybe you mean 
'1<<64', but shift count >= width of type}}
+  // expected-note@-1 {{replace expression with '(2) ^ 64' to silence this 
warning}}

xbolva00 wrote:
> jfb wrote:
> > This one hits a ceiling, we can't really suggest anything for this value 
> > IMO. Maybe we need to see if the user is doing `2^64 - 1`? In that case we 
> > can offer a suggestion.
> Since this requires changes in other parts of code to catch 2^64 - 1, maybe 
> we can leave it as a follow-up patch?
Sure.


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

https://reviews.llvm.org/D63423



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


[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-06-17 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added a comment.

Seems low-value at first glance, but I guess I can't argue with those Twitter 
and codesearch results.

Do you have codesearch results for `^2`? 
https://codesearch.isocpp.org/cgi-bin/cgi_ppsearch?q=%5E+2&search=Search  Seems 
like a lot of false positives (and a lot of code comments turning up in 
codesearch??), but would be perhaps even more valuable to the kinds of users 
who would write `10^x` or `x^2`.

What is going on in this case, and would a warning here be a false positive or 
a true positive? 
https://codesearch.isocpp.org/cgi-bin/cgi_ppsearch?q=10+%5E+7&search=Search




Comment at: lib/Sema/SemaExpr.cpp:10913
+  if (ExprStr.find("0b") != llvm::StringRef::npos)
+return;
+  if (S.getLangOpts().CPlusPlus) {

Why do you special-case `0b` prefix (or suffix — `0x0b` is also special-cased 
by this snippet), but you don't special-case octal or hexadecimal constants? 
I'm sure `x ^ 0x1234` will be a more common spelling than `x ^ 0b1001000110100`.



Comment at: lib/Sema/SemaExpr.cpp:10924
+
+  if (LeftSideValue == 2 || LeftSideValue == 10) {
+llvm::APInt XorValue = LeftSideValue ^ RightSideValue;

Do you have metrics indicating that this line is an improvement over `if (true) 
{`?



Comment at: lib/Sema/SemaExpr.cpp:10959
+S.Diag(Loc, diag::note_xor_used_as_pow_silence)
+<< ("(" + LHSStr + ") ^ " + RHSStr);
+  }

I don't understand why parenthesizing one argument should silence the warning. 
Wouldn't it be more natural to suggest converting both arguments to hexadecimal 
or binary?  That is, convert `10 ^ x` to `0xA ^ x`, or `2 ^ 16` to `0x2 ^ 0x10`.



Comment at: test/Sema/warn-xor-as-pow.c:38
+  res = 0b10 ^ 16;
+  res = 2 ^ 0b100;
+  res = XOR(2, 16);

Please add test cases for `2 ^ 0x4` and `2 ^ 04` and `0x2 ^ 10` and `02 ^ 10`.



Comment at: test/Sema/warn-xor-as-pow.c:39
+  res = 2 ^ 0b100;
+  res = XOR(2, 16);
+  unsigned char two = 2;

I don't understand why this line doesn't warn. Is it because the macro's name 
has the case-insensitive string `xor` in it? Is it because there is a macro 
involved at all? In either case, please add another test case for `res = 
FOOBAR(2, 16)`.

Also `res = EPSILON` where `#define EPSILON 10^-300`. That seems to come up in 
the codesearch results a lot.


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

https://reviews.llvm.org/D63423



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


[PATCH] D63369: [AST] Fixed extraneous warnings for binary conditional operator

2019-06-17 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers added inline comments.



Comment at: clang/lib/AST/Expr.cpp:2348
 // RHS are warnings.
 const ConditionalOperator *Exp = cast(this);
+return Exp->getLHS()->isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx) &&

`const auto *Exp`



Comment at: clang/lib/AST/Expr.cpp:2353
+  case BinaryConditionalOperatorClass: {
+auto *Exp = cast(this);
+return Exp->getFalseExpr()->isUnusedResultAWarning(WarnE, Loc, R1, R2, 
Ctx);

`const auto *Exp`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63369



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


Re: r362856 - DebugInfo: Add support for 'nodebug' attribute on typedefs and alias templates

2019-06-17 Thread David Blaikie via cfe-commits
Oh, yeah - so the trick here is that you don't just drop the typedef, but
you drop an context for that typedef (if it's not otherwise used) - like
namespaces (probably otherwise used) and classes (in the class of C++ type
traits these aren't generally referenced in ways other than the typedef
they contain).

See http://lists.llvm.org/pipermail/libcxx-commits/2019-June/004697.html for
some stats/usage.



On Mon, Jun 17, 2019 at 11:00 AM  wrote:

> Is this really measurable?  All you're suppressing are the typedef DIEs
> and their names; the DIEs are small, although I admit the names can take
> up space.
> (I'm not really objecting, it's just hard to intuit a big size benefit.)
> --paulr
>
> > -Original Message-
> > From: cfe-commits [mailto:cfe-commits-boun...@lists.llvm.org] On Behalf
> Of
> > David Blaikie via cfe-commits
> > Sent: Friday, June 07, 2019 8:01 PM
> > To: cfe-commits@lists.llvm.org
> > Subject: r362856 - DebugInfo: Add support for 'nodebug' attribute on
> > typedefs and alias templates
> >
> > Author: dblaikie
> > Date: Fri Jun  7 17:01:21 2019
> > New Revision: 362856
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=362856&view=rev
> > Log:
> > DebugInfo: Add support for 'nodebug' attribute on typedefs and alias
> > templates
> >
> > Seems like a logical extension to me - and of interest because it might
> > help reduce the debug info size of libc++ by applying this attribute to
> > type traits that have a disproportionate debug info cost compared to the
> > benefit (& possibly harm/confusion) they cause users.
> >
> > Modified:
> > cfe/trunk/include/clang/Basic/Attr.td
> > cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
> > cfe/trunk/test/CodeGenCXX/debug-info-nodebug.cpp
> > cfe/trunk/test/Misc/pragma-attribute-supported-attributes-list.test
> > cfe/trunk/test/Sema/attr-nodebug.c
> >
> > Modified: cfe/trunk/include/clang/Basic/Attr.td
> > URL: http://llvm.org/viewvc/llvm-
> >
> project/cfe/trunk/include/clang/Basic/Attr.td?rev=362856&r1=362855&r2=3628
> > 56&view=diff
> >
> ==
> > 
> > --- cfe/trunk/include/clang/Basic/Attr.td (original)
> > +++ cfe/trunk/include/clang/Basic/Attr.td Fri Jun  7 17:01:21 2019
> > @@ -1434,7 +1434,7 @@ def NoCommon : InheritableAttr {
> >
> >  def NoDebug : InheritableAttr {
> >let Spellings = [GCC<"nodebug">];
> > -  let Subjects = SubjectList<[FunctionLike, ObjCMethod, NonParmVar]>;
> > +  let Subjects = SubjectList<[TypedefName, FunctionLike, ObjCMethod,
> > NonParmVar]>;
> >let Documentation = [NoDebugDocs];
> >  }
> >
> >
> > Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
> > URL: http://llvm.org/viewvc/llvm-
> >
> project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=362856&r1=362855&r2=3628
> > 56&view=diff
> >
> ==
> > 
> > --- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)
> > +++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Fri Jun  7 17:01:21 2019
> > @@ -1091,15 +1091,18 @@ llvm::DIType *CGDebugInfo::CreateType(co
> >assert(Ty->isTypeAlias());
> >llvm::DIType *Src = getOrCreateType(Ty->getAliasedType(), Unit);
> >
> > +  auto *AliasDecl =
> > +  cast(Ty-
> > >getTemplateName().getAsTemplateDecl())
> > +  ->getTemplatedDecl();
> > +
> > +  if (AliasDecl->hasAttr())
> > +return Src;
> > +
> >SmallString<128> NS;
> >llvm::raw_svector_ostream OS(NS);
> >Ty->getTemplateName().print(OS, getPrintingPolicy(), /*qualified*/
> > false);
> >printTemplateArgumentList(OS, Ty->template_arguments(),
> > getPrintingPolicy());
> >
> > -  auto *AliasDecl =
> > -  cast(Ty-
> > >getTemplateName().getAsTemplateDecl())
> > -  ->getTemplatedDecl();
> > -
> >SourceLocation Loc = AliasDecl->getLocation();
> >return DBuilder.createTypedef(Src, OS.str(), getOrCreateFile(Loc),
> >  getLineNumber(Loc),
> > @@ -1108,15 +,20 @@ llvm::DIType *CGDebugInfo::CreateType(co
> >
> >  llvm::DIType *CGDebugInfo::CreateType(const TypedefType *Ty,
> >llvm::DIFile *Unit) {
> > +  llvm::DIType *Underlying =
> > +  getOrCreateType(Ty->getDecl()->getUnderlyingType(), Unit);
> > +
> > +  if (Ty->getDecl()->hasAttr())
> > +return Underlying;
> > +
> >// We don't set size information, but do specify where the typedef was
> >// declared.
> >SourceLocation Loc = Ty->getDecl()->getLocation();
> >
> >// Typedefs are derived from some other type.
> > -  return DBuilder.createTypedef(
> > -  getOrCreateType(Ty->getDecl()->getUnderlyingType(), Unit),
> > -  Ty->getDecl()->getName(), getOrCreateFile(Loc),
> getLineNumber(Loc),
> > -  getDeclContextDescriptor(Ty->getDecl()));
> > +  return DBuilder.createTypedef(Underlying, Ty->getDecl()->getName(),
> > +getOrCreateFile(Loc),
> getLineNumber(Loc),
> > +
> getDeclContextDescriptor(T

[PATCH] D63369: [AST] Fixed extraneous warnings for binary conditional operator

2019-06-17 Thread Nathan Huckleberry via Phabricator via cfe-commits
Nathan-Huckleberry updated this revision to Diff 205129.
Nathan-Huckleberry added a comment.

- [AST] Formatting changes to ConditionalOperator unused detection


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63369

Files:
  clang/lib/AST/Expr.cpp
  clang/test/Sema/warn-binary-conditional-expression-unused.c


Index: clang/test/Sema/warn-binary-conditional-expression-unused.c
===
--- /dev/null
+++ clang/test/Sema/warn-binary-conditional-expression-unused.c
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -fsyntax-only -Wunused-value -verify %s
+int main() {
+int a;
+int b;
+a ? : b; //expected-warning{{expression result unused}}
+a ? a : b; //expected-warning{{expression result unused}}
+a ? : ++b;
+a ? a : ++b;
+++a ? : b; //expected-warning{{expression result unused}}
+++a ? a : b; //expected-warning{{expression result unused}}
+++a ? : ++b;
+++a ? a : ++b;
+return 0;
+};
+
Index: clang/lib/AST/Expr.cpp
===
--- clang/lib/AST/Expr.cpp
+++ clang/lib/AST/Expr.cpp
@@ -2345,12 +2345,13 @@
 // If only one of the LHS or RHS is a warning, the operator might
 // be being used for control flow. Only warn if both the LHS and
 // RHS are warnings.
-const ConditionalOperator *Exp = cast(this);
-if (!Exp->getRHS()->isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx))
-  return false;
-if (!Exp->getLHS())
-  return true;
-return Exp->getLHS()->isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx);
+const auto *Exp = cast(this);
+return Exp->getLHS()->isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx) &&
+   Exp->getRHS()->isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx);
+  }
+  case BinaryConditionalOperatorClass: {
+const auto *Exp = cast(this);
+return Exp->getFalseExpr()->isUnusedResultAWarning(WarnE, Loc, R1, R2, 
Ctx);
   }
 
   case MemberExprClass:


Index: clang/test/Sema/warn-binary-conditional-expression-unused.c
===
--- /dev/null
+++ clang/test/Sema/warn-binary-conditional-expression-unused.c
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -fsyntax-only -Wunused-value -verify %s
+int main() {
+int a;
+int b;
+a ? : b; //expected-warning{{expression result unused}}
+a ? a : b; //expected-warning{{expression result unused}}
+a ? : ++b;
+a ? a : ++b;
+++a ? : b; //expected-warning{{expression result unused}}
+++a ? a : b; //expected-warning{{expression result unused}}
+++a ? : ++b;
+++a ? a : ++b;
+return 0;
+};
+
Index: clang/lib/AST/Expr.cpp
===
--- clang/lib/AST/Expr.cpp
+++ clang/lib/AST/Expr.cpp
@@ -2345,12 +2345,13 @@
 // If only one of the LHS or RHS is a warning, the operator might
 // be being used for control flow. Only warn if both the LHS and
 // RHS are warnings.
-const ConditionalOperator *Exp = cast(this);
-if (!Exp->getRHS()->isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx))
-  return false;
-if (!Exp->getLHS())
-  return true;
-return Exp->getLHS()->isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx);
+const auto *Exp = cast(this);
+return Exp->getLHS()->isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx) &&
+   Exp->getRHS()->isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx);
+  }
+  case BinaryConditionalOperatorClass: {
+const auto *Exp = cast(this);
+return Exp->getFalseExpr()->isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx);
   }
 
   case MemberExprClass:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D63369: [AST] Fixed extraneous warnings for binary conditional operator

2019-06-17 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers accepted this revision.
nickdesaulniers added a comment.

Thanks for the patch and following up on the review comments.  Let's work on 
getting you commit access now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63369



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


[PATCH] D63451: P0840R2: support for [[no_unique_address]] attribute

2019-06-17 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith created this revision.
rsmith added a reviewer: rjmccall.
Herald added subscribers: aheejin, dschuff.
Herald added a project: clang.

Add support for the C++2a [[no_unique_address]] attribute for targets using the 
Itanium C++ ABI.

This depends on D63371 .


Repository:
  rC Clang

https://reviews.llvm.org/D63451

Files:
  include/clang/AST/Decl.h
  include/clang/AST/DeclCXX.h
  include/clang/Basic/Attr.td
  include/clang/Basic/AttrDocs.td
  lib/AST/Decl.cpp
  lib/AST/DeclCXX.cpp
  lib/AST/RecordLayoutBuilder.cpp
  lib/CodeGen/CGExpr.cpp
  lib/CodeGen/CGExprAgg.cpp
  lib/CodeGen/CGExprConstant.cpp
  lib/CodeGen/CGOpenMPRuntime.cpp
  lib/CodeGen/CGRecordLayoutBuilder.cpp
  lib/CodeGen/CodeGenFunction.h
  lib/Sema/SemaDeclAttr.cpp
  test/CodeGenCXX/no-unique-address.cpp
  test/CodeGenCXX/tail-padding.cpp
  test/Layout/no-unique-address.cpp
  utils/TableGen/ClangAttrEmitter.cpp

Index: utils/TableGen/ClangAttrEmitter.cpp
===
--- utils/TableGen/ClangAttrEmitter.cpp
+++ utils/TableGen/ClangAttrEmitter.cpp
@@ -2810,7 +2810,7 @@
 
 // Helper function for GenerateTargetSpecificAttrChecks that alters the 'Test'
 // parameter with only a single check type, if applicable.
-static void GenerateTargetSpecificAttrCheck(const Record *R, std::string &Test,
+static bool GenerateTargetSpecificAttrCheck(const Record *R, std::string &Test,
 std::string *FnName,
 StringRef ListName,
 StringRef CheckAgainst,
@@ -2830,7 +2830,9 @@
 *FnName += Part;
 }
 Test += ")";
+return true;
   }
+  return false;
 }
 
 // Generate a conditional expression to check if the current target satisfies
@@ -2838,10 +2840,12 @@
 // those checks to the Test string. If the FnName string pointer is non-null,
 // append a unique suffix to distinguish this set of target checks from other
 // TargetSpecificAttr records.
-static void GenerateTargetSpecificAttrChecks(const Record *R,
+static bool GenerateTargetSpecificAttrChecks(const Record *R,
  std::vector &Arches,
  std::string &Test,
  std::string *FnName) {
+  bool AnyTargetChecks = false;
+
   // It is assumed that there will be an llvm::Triple object
   // named "T" and a TargetInfo object named "Target" within
   // scope that can be used to determine whether the attribute exists in
@@ -2851,6 +2855,7 @@
   // differently because GenerateTargetRequirements needs to combine the list
   // with ParseKind.
   if (!Arches.empty()) {
+AnyTargetChecks = true;
 Test += " && (";
 for (auto I = Arches.begin(), E = Arches.end(); I != E; ++I) {
   StringRef Part = *I;
@@ -2865,16 +2870,19 @@
   }
 
   // If the attribute is specific to particular OSes, check those.
-  GenerateTargetSpecificAttrCheck(R, Test, FnName, "OSes", "T.getOS()",
-  "llvm::Triple::");
+  AnyTargetChecks |= GenerateTargetSpecificAttrCheck(
+  R, Test, FnName, "OSes", "T.getOS()", "llvm::Triple::");
 
   // If one or more CXX ABIs are specified, check those as well.
   GenerateTargetSpecificAttrCheck(R, Test, FnName, "CXXABIs",
   "Target.getCXXABI().getKind()",
   "TargetCXXABI::");
   // If one or more object formats is specified, check those.
-  GenerateTargetSpecificAttrCheck(R, Test, FnName, "ObjectFormats",
-  "T.getObjectFormat()", "llvm::Triple::");
+  AnyTargetChecks |=
+  GenerateTargetSpecificAttrCheck(R, Test, FnName, "ObjectFormats",
+  "T.getObjectFormat()", "llvm::Triple::");
+
+  return AnyTargetChecks;
 }
 
 static void GenerateHasAttrSpellingStringSwitch(
@@ -3510,7 +3518,7 @@
 
   std::string FnName = "isTarget";
   std::string Test;
-  GenerateTargetSpecificAttrChecks(R, Arches, Test, &FnName);
+  bool UsesT = GenerateTargetSpecificAttrChecks(R, Arches, Test, &FnName);
 
   // If this code has already been generated, simply return the previous
   // instance of it.
@@ -3520,7 +3528,8 @@
 return *I;
 
   OS << "static bool " << FnName << "(const TargetInfo &Target) {\n";
-  OS << "  const llvm::Triple &T = Target.getTriple();\n";
+  if (UsesT)
+OS << "  const llvm::Triple &T = Target.getTriple();\n";
   OS << "  return " << Test << ";\n";
   OS << "}\n\n";
 
Index: test/Layout/no-unique-address.cpp
===
--- /dev/null
+++ test/Layout/no-unique-address.cpp
@@ -0,0 +1,265 @@
+// RUN: %clang_cc1 -std=c++2a -fsyntax-only -triple x86_64-linux-gnu -fdump-record-layouts %s | FileCheck %s
+
+namespace Empty {
+  struct A {};
+  struct B { [[no_unique_address]] A a; char b; };
+  static_assert(

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-06-17 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 marked 2 inline comments as done.
xbolva00 added inline comments.



Comment at: lib/Sema/SemaExpr.cpp:10924
+
+  if (LeftSideValue == 2 || LeftSideValue == 10) {
+llvm::APInt XorValue = LeftSideValue ^ RightSideValue;

Quuxplusone wrote:
> Do you have metrics indicating that this line is an improvement over `if 
> (true) {`?
This is left over of older code :) I will fix it.



Comment at: test/Sema/warn-xor-as-pow.c:39
+  res = 2 ^ 0b100;
+  res = XOR(2, 16);
+  unsigned char two = 2;

Quuxplusone wrote:
> I don't understand why this line doesn't warn. Is it because the macro's name 
> has the case-insensitive string `xor` in it? Is it because there is a macro 
> involved at all? In either case, please add another test case for `res = 
> FOOBAR(2, 16)`.
> 
> Also `res = EPSILON` where `#define EPSILON 10^-300`. That seems to come up 
> in the codesearch results a lot.
EPSILON 10^-300

this is in macro :( 

so maybe if negative RHS, we should check macro too? @jfb 


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

https://reviews.llvm.org/D63423



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


[PATCH] D58033: Add option for emitting dbg info for call site parameters

2019-06-17 Thread Paul Robinson via Phabricator via cfe-commits
probinson added inline comments.



Comment at: lib/Frontend/CompilerInvocation.cpp:755
+  (Arch == llvm::Triple::x86 || Arch == llvm::Triple::x86_64))
+Opts.EnableDebugEntryValues = Args.hasArg(OPT_femit_debug_entry_values);
+

You want to disable entry-values for all targets other than X86?


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

https://reviews.llvm.org/D58033



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


[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-06-17 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 updated this revision to Diff 205136.
xbolva00 added a comment.

Updated.


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

https://reviews.llvm.org/D63423

Files:
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaExpr.cpp
  test/SemaCXX/warn-xor-as-pow.cpp

Index: test/SemaCXX/warn-xor-as-pow.cpp
===
--- test/SemaCXX/warn-xor-as-pow.cpp
+++ test/SemaCXX/warn-xor-as-pow.cpp
@@ -0,0 +1,86 @@
+// RUN: %clang_cc1 -x c -fsyntax-only -verify -Wxor-as-pow %s
+// RUN: %clang_cc1 -x c -fsyntax-only -verify %s
+// RUN: %clang_cc1 -x c -fsyntax-only -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+
+// RUN: %clang_cc1 -x c++ -DKW_XOR -fsyntax-only -verify -Wxor-as-pow %s
+// RUN: %clang_cc1 -x c++ -DKW_XOR -fsyntax-only -verify %s
+// RUN: %clang_cc1 -x c++ -DKW_XOR -fsyntax-only -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+
+#define FOOBAR(x, y) (x * y)
+#define XOR(x, y) (x ^ y)
+#define TWO 2
+#define TEN 10
+#define TWO_ULL 2ULL
+#define EPSILON 10^-300
+
+void test(unsigned a, unsigned b) {
+  unsigned res;
+  res = a ^ 5;
+  res = 2 ^ b;
+  res = a ^ b;
+  res = 2 ^ 0; // expected-warning {{result of '2 ^ 0' is 2, maybe you mean '1<<0' (1)?}}
+   // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:14}:"1"
+   // expected-note@-2 {{replace expression with '0x2 ^ 0' to silence this warning}}
+  res = 2 ^ 1; // expected-warning {{result of '2 ^ 1' is 3, maybe you mean '1<<1' (2)?}}
+   // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:14}:"1<<1"
+   // expected-note@-2 {{replace expression with '0x2 ^ 1' to silence this warning}}
+  res = 2 ^ 2; // expected-warning {{result of '2 ^ 2' is 0, maybe you mean '1<<2' (4)?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:14}:"1<<2"
+  // expected-note@-2 {{replace expression with '0x2 ^ 2' to silence this warning}}
+  res = 2 ^ 8; // expected-warning {{result of '2 ^ 8' is 10, maybe you mean '1<<8' (256)?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:14}:"1<<8"
+  // expected-note@-2 {{replace expression with '0x2 ^ 8' to silence this warning}}
+  res = TWO ^ 8; // expected-warning {{result of 'TWO ^ 8' is 10, maybe you mean '1<<8' (256)?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:16}:"1<<8"
+  // expected-note@-2 {{replace expression with '0x2 ^ 8' to silence this warning}}
+  res = 2 ^ 16; // expected-warning {{result of '2 ^ 16' is 18, maybe you mean '1<<16' (65536)?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:15}:"1<<16"
+  // expected-note@-2 {{replace expression with '0x2 ^ 16' to silence this warning}}
+  res = 2 ^ TEN; // expected-warning {{result of '2 ^ TEN' is 8, maybe you mean '1<= width of type}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:15}:"1LL<<32"
+  // expected-note@-2 {{replace expression with '0x2 ^ 32' to silence this warning}}
+  res = 2 ^ 64; // expected-warning {{result of '2 ^ 64' is 66; maybe you mean '1<<64', but shift count >= width of type}}
+  // expected-note@-1 {{replace expression with '0x2 ^ 64' to silence this warning}}
+
+  res = 10 ^ 0;
+  res = 10 ^ 1; // expected-warning {{result of '10 ^ 1' is 11, maybe you mean '10'?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:15}:"10"
+  // expected-note@-2 {{replace expression with '0xA ^ 1' to silence this warning}}
+  res = 10 ^ 2; // expected-warning {{result of '10 ^ 2' is 8, maybe you mean '100'?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:15}:"100"
+  // expected-note@-2 {{replace expression with '0xA ^ 2' to silence this warning}}
+  res = 10 ^ 4; // expected-warning {{result of '10 ^ 4' is 14, maybe you mean '1'?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:15}:"1"
+  // expected-note@-2 {{replace expression with '0xA ^ 4' to silence this warning}}
+  res = 10 ^ 10; // expected-warning {{result of '10 ^ 10' is 0, maybe you mean '100'?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:16}:"100"
+  // expected-note@-2 {{replace expression with '0xA ^ 10' to silence this warning}}
+  res = TEN ^ 10; // expected-warning {{result of 'TEN ^ 10' is 0, maybe you mean '100'?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:17}:"100"
+  // expected-note@-2 {{replace expression with '0xA ^ 10' to silence this warning}}
+  res = 0xA ^ 10;
+  #if defined(KW_XOR)
+  res = 10 xor 10;
+  #endif
+  res = XOR(10, 10);
+  res = 10^-7;
+}
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -10890,6 +10890,89 @@
   return GetSignedVectorType(vType);
 }
 
+static void diagnoseXorMisusedAsPow(Sema &S, ExprResult &LHS, ExprResult &RHS,
+SourceLocation Loc) {
+  // Do not diagnose macros
+  if (Loc.isMacroID())

[PATCH] D63371: Rewrite ConstStructBuilder with a mechanism that can cope with splitting and updating constants.

2019-06-17 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

In D63371#1546587 , @rsmith wrote:

> In D63371#1546500 , @rjmccall wrote:
>
> > Isn't `[[no_unique_address]]` only significant for empty members?  I'm not 
> > sure why they need significant support from constant-building, since they 
> > expand to no meaningful initializer.
>
>
> It also permits reuse of tail padding for non-static data members, which is 
> the complexity that this patch is dealing with (in addition to improving and 
> generalizing the support for non-trivial designated initializers).


I see.

>> We have some code we'll hopefully be upstreaming soon that relies on being 
>> able to do things with address-of-position placeholder values; I'm a little 
>> worried that the new structure here doesn't really support them.
> 
> Can you say a bit more about that? (Do you want to be able to emit a constant 
> that denotes a pointer to somewhere else within the same constant being 
> emitted, or something like that?) I think this approach should be strictly 
> more general than what we had before, but perhaps that means it can't be 
> extended in the direction you need?

We need to be able to construct constant initializers for certain fields in 
terms of (among other things) a pointer to the current field.  My concern was 
whether this might mess up the indexing to the current field; but it looks like 
we actually discover the right GEP indices retroactively for these aggregate 
constants (unlike e.g. `ConstantInitBuilder`), so the fact that the GEP indices 
might change as we build the constant is not a problem.  As long as the 
`ConstantEmitter` is being passed around and used to build the individual 
fields appropriately, it should be fine.


Repository:
  rC Clang

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

https://reviews.llvm.org/D63371



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


[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-06-17 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 updated this revision to Diff 205140.
xbolva00 added a comment.

Removed useless else block with return.


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

https://reviews.llvm.org/D63423

Files:
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaExpr.cpp
  test/SemaCXX/warn-xor-as-pow.cpp

Index: test/SemaCXX/warn-xor-as-pow.cpp
===
--- test/SemaCXX/warn-xor-as-pow.cpp
+++ test/SemaCXX/warn-xor-as-pow.cpp
@@ -0,0 +1,86 @@
+// RUN: %clang_cc1 -x c -fsyntax-only -verify -Wxor-as-pow %s
+// RUN: %clang_cc1 -x c -fsyntax-only -verify %s
+// RUN: %clang_cc1 -x c -fsyntax-only -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+
+// RUN: %clang_cc1 -x c++ -DKW_XOR -fsyntax-only -verify -Wxor-as-pow %s
+// RUN: %clang_cc1 -x c++ -DKW_XOR -fsyntax-only -verify %s
+// RUN: %clang_cc1 -x c++ -DKW_XOR -fsyntax-only -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+
+#define FOOBAR(x, y) (x * y)
+#define XOR(x, y) (x ^ y)
+#define TWO 2
+#define TEN 10
+#define TWO_ULL 2ULL
+#define EPSILON 10^-300
+
+void test(unsigned a, unsigned b) {
+  unsigned res;
+  res = a ^ 5;
+  res = 2 ^ b;
+  res = a ^ b;
+  res = 2 ^ 0; // expected-warning {{result of '2 ^ 0' is 2, maybe you mean '1<<0' (1)?}}
+   // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:14}:"1"
+   // expected-note@-2 {{replace expression with '0x2 ^ 0' to silence this warning}}
+  res = 2 ^ 1; // expected-warning {{result of '2 ^ 1' is 3, maybe you mean '1<<1' (2)?}}
+   // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:14}:"1<<1"
+   // expected-note@-2 {{replace expression with '0x2 ^ 1' to silence this warning}}
+  res = 2 ^ 2; // expected-warning {{result of '2 ^ 2' is 0, maybe you mean '1<<2' (4)?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:14}:"1<<2"
+  // expected-note@-2 {{replace expression with '0x2 ^ 2' to silence this warning}}
+  res = 2 ^ 8; // expected-warning {{result of '2 ^ 8' is 10, maybe you mean '1<<8' (256)?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:14}:"1<<8"
+  // expected-note@-2 {{replace expression with '0x2 ^ 8' to silence this warning}}
+  res = TWO ^ 8; // expected-warning {{result of 'TWO ^ 8' is 10, maybe you mean '1<<8' (256)?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:16}:"1<<8"
+  // expected-note@-2 {{replace expression with '0x2 ^ 8' to silence this warning}}
+  res = 2 ^ 16; // expected-warning {{result of '2 ^ 16' is 18, maybe you mean '1<<16' (65536)?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:15}:"1<<16"
+  // expected-note@-2 {{replace expression with '0x2 ^ 16' to silence this warning}}
+  res = 2 ^ TEN; // expected-warning {{result of '2 ^ TEN' is 8, maybe you mean '1<= width of type}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:15}:"1LL<<32"
+  // expected-note@-2 {{replace expression with '0x2 ^ 32' to silence this warning}}
+  res = 2 ^ 64; // expected-warning {{result of '2 ^ 64' is 66; maybe you mean '1<<64', but shift count >= width of type}}
+  // expected-note@-1 {{replace expression with '0x2 ^ 64' to silence this warning}}
+
+  res = 10 ^ 0;
+  res = 10 ^ 1; // expected-warning {{result of '10 ^ 1' is 11, maybe you mean '10'?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:15}:"10"
+  // expected-note@-2 {{replace expression with '0xA ^ 1' to silence this warning}}
+  res = 10 ^ 2; // expected-warning {{result of '10 ^ 2' is 8, maybe you mean '100'?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:15}:"100"
+  // expected-note@-2 {{replace expression with '0xA ^ 2' to silence this warning}}
+  res = 10 ^ 4; // expected-warning {{result of '10 ^ 4' is 14, maybe you mean '1'?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:15}:"1"
+  // expected-note@-2 {{replace expression with '0xA ^ 4' to silence this warning}}
+  res = 10 ^ 10; // expected-warning {{result of '10 ^ 10' is 0, maybe you mean '100'?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:16}:"100"
+  // expected-note@-2 {{replace expression with '0xA ^ 10' to silence this warning}}
+  res = TEN ^ 10; // expected-warning {{result of 'TEN ^ 10' is 0, maybe you mean '100'?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:17}:"100"
+  // expected-note@-2 {{replace expression with '0xA ^ 10' to silence this warning}}
+  res = 0xA ^ 10;
+  #if defined(KW_XOR)
+  res = 10 xor 10;
+  #endif
+  res = XOR(10, 10);
+  res = 10^-7;
+}
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -10890,6 +10890,86 @@
   return GetSignedVectorType(vType);
 }
 
+static void diagnoseXorMisusedAsPow(Sema &S, ExprResult &LHS, ExprResult &RHS,
+SourceLocation Loc) {
+  // Do not diagnose

[PATCH] D63376: [clang] Small improvments after Adding APValue to ConstantExpr

2019-06-17 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.

Nice cleanup!




Comment at: clang/include/clang/Sema/Sema.h:10298
+ bool AllowFold = true,
+ bool StoreResult = true);
   ExprResult VerifyIntegerConstantExpression(Expr *E, llvm::APSInt *Result,

Do you need this new flag? No callers are passing it.


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

https://reviews.llvm.org/D63376



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


[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-06-17 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 marked an inline comment as done.
xbolva00 added inline comments.



Comment at: test/SemaCXX/warn-xor-as-pow.cpp:85
+  res = XOR(10, 10);
+  res = 10^-7;
+}

What should we suggest here ? :) Nevative shift ? think we should diagnose it.

I am not sure about 10 ^ 0 case... warn or not ?

@jfb 


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

https://reviews.llvm.org/D63423



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


[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

2019-06-17 Thread John Regehr via Phabricator via cfe-commits
regehr added a comment.

Just wanted to say that I think I agree with the design choices here!


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

https://reviews.llvm.org/D63423



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


[PATCH] D62750: Show note for -Wmissing-prototypes for functions with parameters

2019-06-17 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added inline comments.



Comment at: lib/Sema/SemaDecl.cpp:13335
+? FixItHint::CreateInsertion(FTL.getRParenLoc(), 
"void")
+: FixItHint{});
 }

aaron.ballman wrote:
> In this case, we could probably generate the parameter declarations for the 
> user still, couldn't we? e.g.,
> ```
> int f(); // We could insert "int i, int j" here as the fixit
> int g(); // Just like we try to suggest "void" here already
> 
> int f(int i, int j) { return i + j; }
> int g(void) { return 12; }
> ```
> That could be done in a follow-up patch though.
One difficulty might be that certain types used in the parameter list of the 
definition aren't available where the declaration is, like

```
int f();

struct X { int n; };
int f(struct X *x) { return x->n;}
```

So I'm not sure we can always do it, but sometimes perhaps.


Repository:
  rC Clang

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

https://reviews.llvm.org/D62750



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


[PATCH] D62750: Show note for -Wmissing-prototypes for functions with parameters

2019-06-17 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: lib/Sema/SemaDecl.cpp:13335
+? FixItHint::CreateInsertion(FTL.getRParenLoc(), 
"void")
+: FixItHint{});
 }

aaronpuchert wrote:
> aaron.ballman wrote:
> > In this case, we could probably generate the parameter declarations for the 
> > user still, couldn't we? e.g.,
> > ```
> > int f(); // We could insert "int i, int j" here as the fixit
> > int g(); // Just like we try to suggest "void" here already
> > 
> > int f(int i, int j) { return i + j; }
> > int g(void) { return 12; }
> > ```
> > That could be done in a follow-up patch though.
> One difficulty might be that certain types used in the parameter list of the 
> definition aren't available where the declaration is, like
> 
> ```
> int f();
> 
> struct X { int n; };
> int f(struct X *x) { return x->n;}
> ```
> 
> So I'm not sure we can always do it, but sometimes perhaps.
That's a good point and reason enough to not try for it in this version of the 
patch (if at all).


Repository:
  rC Clang

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

https://reviews.llvm.org/D62750



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


[PATCH] D63371: Rewrite ConstStructBuilder with a mechanism that can cope with splitting and updating constants.

2019-06-17 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments.



Comment at: lib/CodeGen/CGExprConstant.cpp:73
+/// Incremental builder for an llvm::Constant* holding a structure constant.
+class ConstantBuilder : private ConstantBuilderUtils {
+  llvm::SmallVector Elems;

This seems like a very generic name for this type.



Comment at: lib/CodeGen/CGExprConstant.cpp:75
+  llvm::SmallVector Elems;
+  llvm::SmallVector Offsets;
+  CharUnits Size = CharUnits::Zero();

Are there invariants about these?  I assume they're parallel arrays; are they 
kept sorted?



Comment at: lib/CodeGen/CGExprConstant.cpp:76
+  llvm::SmallVector Offsets;
+  CharUnits Size = CharUnits::Zero();
+

This is one past the last byte that's been covered by an actual `Constant*` 
value, or does it include unoccupied padding, or does it exclude even occupied 
padding?



Comment at: lib/CodeGen/CGExprConstant.cpp:98
+Offsets.reserve(N);
+  }
+

Might be worth clarifying what `N` is here.



Comment at: lib/CodeGen/CGExprConstant.cpp:190
+bool ConstantBuilder::addBits(llvm::APInt Bits, uint64_t OffsetInBits,
+  bool AllowOverwrite) {
+  const ASTContext &Context = CGM.getContext();

`AllowOversized` (which you used in the interface) seems like a better name.



Comment at: lib/CodeGen/CGExprConstant.cpp:196
+  // current char.
+  unsigned CharOffset = OffsetInBits % CharWidth;
+

`OffsetWithinChar`?



Comment at: lib/CodeGen/CGExprConstant.cpp:201
+  for (CharUnits Char = Context.toCharUnitsFromBits(OffsetInBits);
+   /**/; ++Char) {
+// Number of bits we want to fill in this byte.

`OffsetInChars`?



Comment at: lib/CodeGen/CGExprConstant.cpp:237
+  if (!LastElemToUpdate)
+return false;
+  assert(*LastElemToUpdate - *FirstElemToUpdate < 2 &&

Especially in the context of the comment above, I think it would be good to 
clarify that both of these are hard "we can't emit this constant" bail-outs.



Comment at: lib/CodeGen/CGExprConstant.cpp:258
+  return false;
+assert(CI->getBitWidth() == CharWidth && "splitAt failed");
+assert((!(CI->getValue() & UpdateMask) || AllowOverwrite) &&

Oh, because we're splitting before and after a single-`CharUnits` range?  That 
seems worthy of a somewhat clearer explanation in the code.

I guess we could have a non-`ConstantInt` single-byte value.  Unlikely but not 
impossible. :)



Comment at: lib/CodeGen/CGExprConstant.cpp:375
+
+  // FIXME: We could theoretically split a ConstantInt if the need ever arose.
+

Does this not come up all the time with bit-fields?  I guess we emit them in 
single-`char` chunks, so it wouldn't.  Probably worth a comment.


Repository:
  rC Clang

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

https://reviews.llvm.org/D63371



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


  1   2   >