r373289 - Don't elide the use of the thread wrapper for a thread_local constinit

[Richard Smith via cfe-commits]

Author: rsmith
Date: Mon Sep 30 18:23:23 2019
New Revision: 373289

URL: http://llvm.org/viewvc/llvm-project?rev=373289=rev
Log:
Don't elide the use of the thread wrapper for a thread_local constinit
variable with non-trivial destruction.

We still need to invoke the thread wrapper to trigger registration of
the destructor call on thread shutdown.

Modified:
cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp
cfe/trunk/test/CodeGenCXX/cxx2a-thread-local-constinit.cpp

Modified: cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp?rev=373289=373288=373289=diff
==
--- cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp (original)
+++ cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp Mon Sep 30 18:23:23 2019
@@ -360,7 +360,8 @@ public:
   }
 
   bool usesThreadWrapperFunction(const VarDecl *VD) const override {
-return !isEmittedWithConstantInitializer(VD);
+return !isEmittedWithConstantInitializer(VD) ||
+   VD->needsDestruction(getContext());
   }
   LValue EmitThreadLocalVarDeclLValue(CodeGenFunction , const VarDecl *VD,
   QualType LValType) override;
@@ -2606,7 +2607,7 @@ void ItaniumCXXABI::EmitThreadLocalInitF
 llvm::GlobalValue *Init = nullptr;
 bool InitIsInitFunc = false;
 bool HasConstantInitialization = false;
-if (isEmittedWithConstantInitializer(VD)) {
+if (!usesThreadWrapperFunction(VD)) {
   HasConstantInitialization = true;
 } else if (VD->hasDefinition()) {
   InitIsInitFunc = true;

Modified: cfe/trunk/test/CodeGenCXX/cxx2a-thread-local-constinit.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/cxx2a-thread-local-constinit.cpp?rev=373289=373288=373289=diff
==
--- cfe/trunk/test/CodeGenCXX/cxx2a-thread-local-constinit.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/cxx2a-thread-local-constinit.cpp Mon Sep 30 
18:23:23 2019
@@ -33,11 +33,6 @@ extern thread_local int c;
 // CHECK: }
 int h() { return c; }
 
-thread_local int c = 0;
-
-int d_init();
-thread_local int d = d_init();
-
 // Note: use of 'c' does not trigger initialization of 'd', because 'c' has a
 // constant initializer.
 // CHECK-LABEL: define weak_odr {{.*}} @_ZTW1c()
@@ -45,3 +40,30 @@ thread_local int d = d_init();
 // CHECK-NOT: call
 // CHECK: ret i32* @c
 // CHECK: }
+
+thread_local int c = 0;
+
+int d_init();
+
+// CHECK: define {{.*}}[[D_INIT:@__cxx_global_var_init[^(]*]](
+// CHECK: call {{.*}} @_Z6d_initv()
+thread_local int d = d_init();
+
+struct Destructed {
+  int n;
+  ~Destructed();
+};
+
+extern thread_local constinit Destructed e;
+// CHECK-LABEL: define i32 @_Z1iv()
+// CHECK: call {{.*}}* @_ZTW1e()
+// CHECK: }
+int i() { return e.n; }
+
+// CHECK: define {{.*}}[[E2_INIT:@__cxx_global_var_init[^(]*]](
+// CHECK: call {{.*}} @__cxa_thread_atexit({{.*}} @_ZN10DestructedD1Ev {{.*}} 
@e2
+thread_local constinit Destructed e2;
+
+// CHECK-LABEL: define {{.*}}__tls_init
+// CHECK: call {{.*}} [[D_INIT]]
+// CHECK: call {{.*}} [[E2_INIT]]


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

[PATCH] D67739: [WebAssembly] Let users know that wasm64 does not exist

[Thomas Lively via Phabricator via cfe-commits]

tlively abandoned this revision.
tlively added a comment.

Closing in favor of D68254 .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67739



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

r373281 - [c++20] Add a C++20 version of the existing turing machine test.

[Richard Smith via cfe-commits]

Author: rsmith
Date: Mon Sep 30 17:47:41 2019
New Revision: 373281

URL: http://llvm.org/viewvc/llvm-project?rev=373281=rev
Log:
[c++20] Add a C++20 version of the existing turing machine test.

Unlike the C++11 version, this one uese mutable state and dynamic
allocation instead of a carefully balanced and ever-accumulating pile of
temporaries.

Added:
cfe/trunk/test/SemaCXX/constexpr-turing-cxx2a.cpp

Added: cfe/trunk/test/SemaCXX/constexpr-turing-cxx2a.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/constexpr-turing-cxx2a.cpp?rev=373281=auto
==
--- cfe/trunk/test/SemaCXX/constexpr-turing-cxx2a.cpp (added)
+++ cfe/trunk/test/SemaCXX/constexpr-turing-cxx2a.cpp Mon Sep 30 17:47:41 2019
@@ -0,0 +1,66 @@
+// RUN: %clang_cc1 -verify -std=c++2a %s
+// expected-no-diagnostics
+
+const unsigned halt = (unsigned)-1;
+
+enum Dir { L, R };
+struct Action {
+  bool tape;
+  Dir dir;
+  unsigned next;
+};
+using State = Action[2];
+
+// An infinite tape!
+struct Tape {
+  constexpr Tape() = default;
+  constexpr ~Tape() {
+if (l) { l->r = nullptr; delete l; }
+if (r) { r->l = nullptr; delete r; }
+  }
+  constexpr Tape *left() {
+if (!l) { l = new Tape; l->r = this; }
+return l;
+  }
+  constexpr Tape *right() {
+if (!r) { r = new Tape; r->l = this; }
+return r;
+  }
+  Tape *l = nullptr;
+  bool val = false;
+  Tape *r = nullptr;
+};
+
+// Run turing machine 'tm' on tape 'tape' from state 'state'. Return number of
+// steps taken until halt.
+constexpr unsigned run(const State *tm) {
+  Tape *tape = new Tape;
+  unsigned state = 0;
+  unsigned steps = 0;
+
+  for (state = 0; state != halt; ++steps) {
+auto [val, dir, next_state] = tm[state][tape->val];
+tape->val = val;
+tape = (dir == L ? tape->left() : tape->right());
+state = next_state;
+  }
+
+  delete tape;
+  return steps;
+}
+
+// 3-state busy beaver. S(bb3) = 21.
+constexpr State bb3[] = {
+  { { true, R, 1 }, { true, R, halt } },
+  { { true, L, 1 }, { false, R, 2 } },
+  { { true, L, 2 }, { true, L, 0 } }
+};
+static_assert(run(bb3) == 21, "");
+
+// 4-state busy beaver. S(bb4) = 107.
+constexpr State bb4[] = {
+  { { true, R, 1 }, { true, L, 1 } },
+  { { true, L, 0 }, { false, L, 2 } },
+  { { true, R, halt }, { true, L, 3 } },
+  { { true, R, 3 }, { false, R, 0 } } };
+static_assert(run(bb4) == 107, "");


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

r373279 - During constant evaluation, handle CXXBindTemporaryExprs for

[Richard Smith via cfe-commits]

Author: rsmith
Date: Mon Sep 30 17:41:16 2019
New Revision: 373279

URL: http://llvm.org/viewvc/llvm-project?rev=373279=rev
Log:
During constant evaluation, handle CXXBindTemporaryExprs for
array-of-class types, not just for class types.

Modified:
cfe/trunk/lib/AST/ExprConstant.cpp
cfe/trunk/test/SemaCXX/constant-expression-cxx2a.cpp

Modified: cfe/trunk/lib/AST/ExprConstant.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=373279=373278=373279=diff
==
--- cfe/trunk/lib/AST/ExprConstant.cpp (original)
+++ cfe/trunk/lib/AST/ExprConstant.cpp Mon Sep 30 17:41:16 2019
@@ -6493,6 +6493,12 @@ public:
 return StmtVisitorTy::Visit(E->getSubExpr()) && Scope.destroy();
   }
 
+  // Temporaries are registered when created, so we don't care about
+  // CXXBindTemporaryExpr.
+  bool VisitCXXBindTemporaryExpr(const CXXBindTemporaryExpr *E) {
+return StmtVisitorTy::Visit(E->getSubExpr());
+  }
+
   bool VisitCXXReinterpretCastExpr(const CXXReinterpretCastExpr *E) {
 CCEDiag(E, diag::note_constexpr_invalid_cast) << 0;
 return static_cast(this)->VisitCastExpr(E);
@@ -8448,13 +8454,6 @@ namespace {
 bool VisitCXXInheritedCtorInitExpr(const CXXInheritedCtorInitExpr *E);
 bool VisitCXXConstructExpr(const CXXConstructExpr *E, QualType T);
 bool VisitCXXStdInitializerListExpr(const CXXStdInitializerListExpr *E);
-
-// Temporaries are registered when created, so we don't care about
-// CXXBindTemporaryExpr.
-bool VisitCXXBindTemporaryExpr(const CXXBindTemporaryExpr *E) {
-  return Visit(E->getSubExpr());
-}
-
 bool VisitBinCmp(const BinaryOperator *E);
   };
 }

Modified: cfe/trunk/test/SemaCXX/constant-expression-cxx2a.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/constant-expression-cxx2a.cpp?rev=373279=373278=373279=diff
==
--- cfe/trunk/test/SemaCXX/constant-expression-cxx2a.cpp (original)
+++ cfe/trunk/test/SemaCXX/constant-expression-cxx2a.cpp Mon Sep 30 17:41:16 
2019
@@ -802,6 +802,11 @@ namespace dtor {
 return true;
   }
   static_assert(run_dtors_on_array_filler());
+
+  // Ensure that we can handle temporary cleanups for array temporaries.
+  struct ArrElem { constexpr ~ArrElem() {} };
+  using Arr = ArrElem[3];
+  static_assert((Arr{}, true));
 }
 
 namespace dynamic_alloc {


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

[PATCH] D38479: Make -mgeneral-regs-only more like GCC's

[George Burgess IV via Phabricator via cfe-commits]

george.burgess.iv added a comment.

> Does this have any significant impact on -fsyntax-only performance?

I'm sure there are pathological cases where this hurts perf, but my intuition 
tells me that we won't get bitten badly by any of them in the real world. It 
should be a branch per cast + full expr for people who don't use it. For those 
who do, we're walking 2 types for each cast (plus maybe constexpr evaluations 
for casts from FP/vec to non-FP/vec values), and walking the types for each 
FullExpr.

Again, you can likely craft code that makes this expensive (which we can likely 
fix with strategically-placed caches), but being bitten by that in practice 
seems somewhat unlikely to me. Happy to try and add caching and/or take numbers 
with caches if you'd like.

To evaluate the build time impact of this, I did 20 builds of aarch64 Linux 
with/without this patch. Of the metrics I tracked (memory, system/user/wall 
time), all reported differences were < their stdev except for user time. User 
time regression with this patch was 0.37%, with a stdev of 0.12%. Happy to try 
to gather -fsyntax-only numbers if there's a simple way to do that.




Comment at: clang/lib/Driver/ToolChains/Arch/AArch64.cpp:194
 Features.push_back("-crypto");
 Features.push_back("-neon");
+// FIXME: Ideally, we'd also like to pass a `+general-regs-only` feature,

efriedma wrote:
> Do we need to get rid of the "-neon" etc.?
Yeah, good call. Looks like I forgot a "RUN:" in my tests checking for whether 
or not asm works, so the thing that should've caught that wasn't enabled. :)



Comment at: clang/lib/Sema/SemaExprCXX.cpp:7997
+  bool isRValueOfIllegalType(const Expr *E) const {
+return E->getValueKind() != VK_LValue && !isGeneralType(E);
+  }

efriedma wrote:
> Should this be `E->isRValue()`?  Not that the difference really matters for 
> C, but it affects rvalue references in C++.
Yeah, flipped to that and added tests -- thanks


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

https://reviews.llvm.org/D38479



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

[PATCH] D38479: Make -mgeneral-regs-only more like GCC's

[George Burgess IV via Phabricator via cfe-commits]

george.burgess.iv updated this revision to Diff 222525.
george.burgess.iv marked 6 inline comments as done.
george.burgess.iv added a comment.
Herald added subscribers: llvm-commits, hiraditya.
Herald added a project: LLVM.

Addressed feedback


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

https://reviews.llvm.org/D38479

Files:
  clang/docs/UsersManual.rst
  clang/include/clang/AST/Expr.h
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/CC1Options.td
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/Expr.cpp
  clang/lib/Driver/ToolChains/Arch/AArch64.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaCast.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/CodeGen/aarch64-mgeneral_regs_only.c
  clang/test/Driver/aarch64-mgeneral_regs_only.c
  clang/test/Sema/aarch64-mgeneral_regs_only.c
  clang/test/SemaCXX/aarch64-mgeneral_regs_only.cpp
  llvm/lib/Target/AArch64/AArch64.td
  llvm/lib/Target/AArch64/AArch64Subtarget.h

Index: llvm/lib/Target/AArch64/AArch64Subtarget.h
===
--- llvm/lib/Target/AArch64/AArch64Subtarget.h
+++ llvm/lib/Target/AArch64/AArch64Subtarget.h
@@ -86,6 +86,9 @@
   bool HasFP16FML = false;
   bool HasSPE = false;
 
+  // FIXME: Backend support for 'enforcing' this would be nice.
+  bool IsGeneralRegsOnly = false;
+
   // ARMv8.1 extensions
   bool HasVH = false;
   bool HasPAN = false;
Index: llvm/lib/Target/AArch64/AArch64.td
===
--- llvm/lib/Target/AArch64/AArch64.td
+++ llvm/lib/Target/AArch64/AArch64.td
@@ -25,6 +25,10 @@
 def FeatureNEON : SubtargetFeature<"neon", "HasNEON", "true",
   "Enable Advanced SIMD instructions", [FeatureFPARMv8]>;
 
+def FeatureGeneralRegsOnly : SubtargetFeature<"general-regs-only",
+  "IsGeneralRegsOnly", "false",
+  "Restrict IR to only use non-fp/simd registers. Doesn't apply to asm.">;
+
 def FeatureSM4 : SubtargetFeature<
 "sm4", "HasSM4", "true",
 "Enable SM3 and SM4 support", [FeatureNEON]>;
Index: clang/test/SemaCXX/aarch64-mgeneral_regs_only.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/aarch64-mgeneral_regs_only.cpp
@@ -0,0 +1,222 @@
+// RUN: %clang_cc1 -triple aarch64-linux-eabi -std=c++11 -general-regs-only %s -verify -DBANNED=float -Wno-unused-value
+// RUN: %clang_cc1 -triple aarch64-linux-eabi -std=c++11 -general-regs-only %s -verify -DBANNED=int '-DVECATTR=__attribute__((ext_vector_type(2)))' -Wno-unused-value
+// RUN: %clang_cc1 -triple aarch64-linux-eabi -std=c++11 -general-regs-only %s -verify -DBANNED=FloatTypedef -Wno-unused-value
+
+using FloatTypedef = float;
+
+#ifndef VECATTR
+#define VECATTR
+#define BannedToInt(x) (x)
+#else
+#define BannedToInt(x) ((x)[0])
+#endif
+
+typedef BANNED BannedTy VECATTR;
+
+namespace default_args {
+int foo(BannedTy = 0); // expected-error 2{{use of floating-point or vector values is disabled}}
+int bar(int = 1.0);
+
+void baz(int a = foo()); // expected-note{{from use of default argument here}}
+void bazz(int a = bar());
+
+void test() {
+  foo(); // expected-note{{from use of default argument here}}
+  bar();
+  baz(); // expected-note{{from use of default argument here}}
+  baz(4);
+  bazz();
+}
+} // namespace default_args
+
+namespace conversions {
+struct ConvertToFloat {
+  explicit operator BannedTy();
+};
+struct ConvertToFloatImplicit { /* implicit */
+  operator BannedTy();
+};
+struct ConvertFromFloat {
+  ConvertFromFloat(BannedTy);
+};
+
+void takeFloat(BannedTy);
+void takeConvertFromFloat(ConvertFromFloat);
+void takeConvertFromFloatRef(const ConvertFromFloat &);
+
+void test() {
+  BannedTy(ConvertToFloat());
+
+  takeFloat(ConvertToFloatImplicit{}); // expected-error{{use of floating-point or vector values is disabled}}
+
+  ConvertFromFloat(0);// expected-error{{use of floating-point or vector values is disabled}}
+  ConvertFromFloat(ConvertToFloatImplicit{}); // expected-error{{use of floating-point or vector values is disabled}}
+
+  ConvertFromFloat{0};// expected-error{{use of floating-point or vector values is disabled}}
+  ConvertFromFloat{ConvertToFloatImplicit{}}; // expected-error{{use of floating-point or vector values is disabled}}
+
+  takeConvertFromFloat(0);// expected-error{{use of floating-point or vector values is disabled}}
+  takeConvertFromFloatRef(0); // expected-error{{use of floating-point or vector values is disabled}}
+
+  takeConvertFromFloat(ConvertFromFloat(0));// expected-error{{use of floating-point or vector values is disabled}}
+  takeConvertFromFloatRef(ConvertFromFloat(0)); // expected-error{{use of floating-point or vector values is disabled}}
+}
+
+void takeImplicitFloat(BannedTy = 

[PATCH] D65917: [clang-tidy] Added check for the Google style guide's category method naming rule.

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore added a comment.

(sorry for the delay )

> The difference is that the exemption list would be per-project, created by 
> whoever turns on the check for the project. I was assuming that this checker 
> would be turned on by each team, rather than globally, and that part of doing 
> so would be specifying which class prefixes should be ignored for that 
> particular project.

The vision that I had in mind for this check was for it to be enabled more 
widely which would require it to be useful by default without requiring options 
to be specified to reduce noise.

> That said, if you want to turn this check on globally, we probably should 
> support either approach, with the default being to check only categories on 
> system frameworks and classes derived from a specific set of opted-in base 
> classes. Then, if a project owner provides a set of prefixes to exempt, it 
> would expand the check to cover everything not exempted. (That's the mode in 
> which we would prefer to use the feature for our team's project.)
> 
> Either way, it is a good idea to auto-exempt any class that isn’t prefixed.

That's a good idea but note that detecting that a class name does not have a 
prefix is not easy because acronyms and initialisms are capitalized (e.g., a 
class named `URLCache` probably does not have a prefix).

> So basically, I'm thinking:
> 
> If no prefix list is provided:
> 
> Categories on:
> 
> System framework classes.
>  A specific list of protected classes and subclasses thereof.
>  If a prefix list is provided:
> 
> Categories on all classes EXCEPT:
> 
> Classes whose prefixes are explicitly excepted.
>  Classes that do not start with at least three capital letters (non-prefixed 
> classes).
>  Does that approach seem reasonable to you?

I think we are moving in a good direction. I suspect that a single option may 
not be the best to control the relevant behaviors.

It sounds like we want the following behaviors:
(1) Require method prefixes on categories on system classes.
(2) Require method prefixes on categories on user classes //except// (2A) user 
classes whose names begin with an exempted prefix and (2B) user classes that 
are derived from an exempted base class.

I think it's unlikely that we would want (1) to be conditional but I think the 
others are likely options that developers would want to control. Based on that 
judgment call, I would imagine three options like so (option names have not 
been thought through extensively): `IncludeUserClasses`, 
`ExemptUserClassPrefixes`, and `ExemptUserBaseClasses`. That way when the check 
is enabled without any options the developer gets enforcement on system classes 
which should reduce a common source of errors and developers that want 
enforcement on their own classes can opt in and use the additional options to 
control enforcement and reduce noise.

WDYT?




Comment at: clang-tools-extra/docs/clang-tidy/checks/list.rst:233
google-objc-global-variable-declaration
+   google-objc-require-category-method-prefixes
google-readability-avoid-underscore-in-googletest-name

Technically this check is not specific to Google Objective-C:
"In order to avoid undefined behavior, it’s best practice to add a prefix to 
method names in categories on framework classes, just like you should add a 
prefix to the names of your own classes. You might choose to use the same three 
letters you use for your class prefixes, but lowercase to follow the usual 
convention for method names, then an underscore, before the rest of the method 
name."
https://developer.apple.com/library/archive/documentation/Cocoa/Conceptual/ProgrammingWithObjectiveC/CustomizingExistingClasses/CustomizingExistingClasses.html

I think we should consider moving this to the objc module 
([rename_check.py](https://github.com/llvm/llvm-project/blob/master/clang-tools-extra/clang-tidy/rename_check.py)
 might be used if we decide that the move is justified).


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

https://reviews.llvm.org/D65917



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

r373276 - [c++20] Fix crash when constant-evaluating an assignment with a

[Richard Smith via cfe-commits]

Author: rsmith
Date: Mon Sep 30 17:07:14 2019
New Revision: 373276

URL: http://llvm.org/viewvc/llvm-project?rev=373276=rev
Log:
[c++20] Fix crash when constant-evaluating an assignment with a
reference member access on its left-hand side.

Modified:
cfe/trunk/lib/AST/ExprConstant.cpp
cfe/trunk/test/SemaCXX/constant-expression-cxx2a.cpp

Modified: cfe/trunk/lib/AST/ExprConstant.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=373276=373275=373276=diff
==
--- cfe/trunk/lib/AST/ExprConstant.cpp (original)
+++ cfe/trunk/lib/AST/ExprConstant.cpp Mon Sep 30 17:07:14 2019
@@ -5258,7 +5258,9 @@ static bool HandleUnionActiveMemberChang
 //   -- If E is of the form A.B, S(E) contains the elements of S(A)...
 if (auto *ME = dyn_cast(E)) {
   auto *FD = dyn_cast(ME->getMemberDecl());
-  if (!FD)
+  // Note that we can't implicitly start the lifetime of a reference,
+  // so we don't need to proceed any further if we reach one.
+  if (!FD || FD->getType()->isReferenceType())
 break;
 
   //... and also contains A.B if B names a union member

Modified: cfe/trunk/test/SemaCXX/constant-expression-cxx2a.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/constant-expression-cxx2a.cpp?rev=373276=373275=373276=diff
==
--- cfe/trunk/test/SemaCXX/constant-expression-cxx2a.cpp (original)
+++ cfe/trunk/test/SemaCXX/constant-expression-cxx2a.cpp Mon Sep 30 17:07:14 
2019
@@ -561,6 +561,29 @@ namespace Union {
 S3 s;
 s.n = 0;
   }
+
+  union ref_member_1 {
+int a;
+int b;
+  };
+  struct ref_member_2 {
+ref_member_1 &
+  };
+  union ref_member_3 {
+ref_member_2 a, b;
+  };
+  constexpr int ref_member_test_1() {
+ref_member_3 r = {.a = {.r = {.a = 1}}};
+r.a.r.b = 2;
+return r.a.r.b;
+  }
+  static_assert(ref_member_test_1() == 2);
+  constexpr int ref_member_test_2() { // expected-error {{never produces a 
constant}}
+ref_member_3 r = {.a = {.r = {.a = 1}}};
+// FIXME: This note isn't great. The 'read' here is reading the referent 
of the reference.
+r.b.r.b = 2; // expected-note {{read of member 'b' of union with active 
member 'a'}}
+return r.b.r.b;
+  }
 }
 
 namespace TwosComplementShifts {


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

[PATCH] D66176: Fix Driver/modules.cpp test to work when build directory name contains '.s'

[Tom Stellard via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL373275: Fix Driver/modules.cpp test to work when build 
directory name contains .s (authored by tstellar, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D66176?vs=05=222521#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D66176

Files:
  cfe/trunk/test/Driver/modules.cpp


Index: cfe/trunk/test/Driver/modules.cpp
===
--- cfe/trunk/test/Driver/modules.cpp
+++ cfe/trunk/test/Driver/modules.cpp
@@ -15,7 +15,7 @@
 // RUN: %clang -std=c++2a %t/module.pcm -S -o %t/module.pcm.o -v 2>&1 | 
FileCheck %s --check-prefix=CHECK-COMPILE
 //
 // CHECK-COMPILE: -cc1 {{.*}} {{-emit-obj|-S}}
-// CHECK-COMPILE-SAME: -o {{.*}}.{{pcm.o|s}}
+// CHECK-COMPILE-SAME: -o {{.*}}module{{2*}}.pcm.o
 // CHECK-COMPILE-SAME: -x pcm
 // CHECK-COMPILE-SAME: {{.*}}.pcm
 


Index: cfe/trunk/test/Driver/modules.cpp
===
--- cfe/trunk/test/Driver/modules.cpp
+++ cfe/trunk/test/Driver/modules.cpp
@@ -15,7 +15,7 @@
 // RUN: %clang -std=c++2a %t/module.pcm -S -o %t/module.pcm.o -v 2>&1 | FileCheck %s --check-prefix=CHECK-COMPILE
 //
 // CHECK-COMPILE: -cc1 {{.*}} {{-emit-obj|-S}}
-// CHECK-COMPILE-SAME: -o {{.*}}.{{pcm.o|s}}
+// CHECK-COMPILE-SAME: -o {{.*}}module{{2*}}.pcm.o
 // CHECK-COMPILE-SAME: -x pcm
 // CHECK-COMPILE-SAME: {{.*}}.pcm
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r373275 - Fix Driver/modules.cpp test to work when build directory name contains '.s'

[Tom Stellard via cfe-commits]

Author: tstellar
Date: Mon Sep 30 16:42:17 2019
New Revision: 373275

URL: http://llvm.org/viewvc/llvm-project?rev=373275=rev
Log:
Fix Driver/modules.cpp test to work when build directory name contains '.s'

Reviewers: dyung, rsmith, hansw

Subscribers: mati865, mgorny, cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/test/Driver/modules.cpp

Modified: cfe/trunk/test/Driver/modules.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/modules.cpp?rev=373275=373274=373275=diff
==
--- cfe/trunk/test/Driver/modules.cpp (original)
+++ cfe/trunk/test/Driver/modules.cpp Mon Sep 30 16:42:17 2019
@@ -15,7 +15,7 @@
 // RUN: %clang -std=c++2a %t/module.pcm -S -o %t/module.pcm.o -v 2>&1 | 
FileCheck %s --check-prefix=CHECK-COMPILE
 //
 // CHECK-COMPILE: -cc1 {{.*}} {{-emit-obj|-S}}
-// CHECK-COMPILE-SAME: -o {{.*}}.{{pcm.o|s}}
+// CHECK-COMPILE-SAME: -o {{.*}}module{{2*}}.pcm.o
 // CHECK-COMPILE-SAME: -x pcm
 // CHECK-COMPILE-SAME: {{.*}}.pcm
 


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

[PATCH] D64991: [analyzer][WIP] Implement a primitive reaching definitions analysis

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus updated this revision to Diff 222514.
Szelethus added a comment.

I'm starting to be really happy with the current direction! I think I'll start 
splitting this up soon, I'm confident that the current interface (after some 
polishing) is general enough to develop incrementally.

- Introduce `Variable`, a base class of `Definition`, which is a (`VarDecl`, 
`FieldChain`) pair.
- Introduce `GenSetBuilder`, a class that owns all `MatchFinder`s and 
associated callbacks used for finding potential writes and invalidations to 
`Variable`s, use exclusively it to build GEN sets.
  - Use a very flexible interface for creating and registering matchers for 
either
- Creating the set of `Variable`s accessible to a function
- Finding statements that may write any of those
- Teasing an `Expr` apart to find `Variable`s in it
  - In an `init()` pass, collect all variables used, or potentially invalidated 
in the function. This allows for invalidations and writes to be done in a 
single pass on a `CFGStmt`.
  - Added a very primitive solution to invalidate variables passed to functions 
as a non-const reference.


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

https://reviews.llvm.org/D64991

Files:
  clang/include/clang/Analysis/Analyses/ReachingDefinitions.h
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/lib/Analysis/CMakeLists.txt
  clang/lib/Analysis/ReachingDefinitions.cpp
  clang/lib/StaticAnalyzer/Checkers/DebugCheckers.cpp
  clang/test/Analysis/dump-definitions.cpp

Index: clang/test/Analysis/dump-definitions.cpp
===
--- /dev/null
+++ clang/test/Analysis/dump-definitions.cpp
@@ -0,0 +1,823 @@
+// RUN: %clang_analyze_cc1 %s \
+// RUN:   -analyzer-checker=debug.DumpCFG \
+// RUN:   -analyzer-checker=debug.DumpGenSets \
+// RUN:   -analyzer-checker=debug.DumpKillSets \
+// RUN:   -analyzer-checker=debug.DumpReachingDefinitions \
+// RUN:   2>&1 | FileCheck %s
+
+int global_var;
+
+int getInt();
+int *getIntPtr();
+bool coin();
+
+void single_vardecl_in_declstmt() {
+  int *ptr = getIntPtr();
+}
+// [B2 (ENTRY)] -> [B1] -> [B0 (EXIT)]
+
+// CHECK:  GEN sets: blockid (varname [blockid, elementid])
+// CHECK-NEXT: 1 (global_var, [1, 2]) 
+// CHECK-NEXT: 1 (ptr, [1, 3]) 
+// CHECK-NEXT: KILL sets: blockid (varname [blockid, elementid])
+// CHECK-NEXT: Reaching definition sets: blockid IN/OUT (varname [blockid, elementid])
+// CHECK-NEXT: 0 IN (global_var, [1, 2]) 
+// CHECK-NEXT: 0 IN (ptr, [1, 3]) 
+// CHECK-NEXT: 0 OUT (global_var, [1, 2]) 
+// CHECK-NEXT: 0 OUT (ptr, [1, 3]) 
+// CHECK-NEXT: 1 OUT (global_var, [1, 2]) 
+// CHECK-NEXT: 1 OUT (ptr, [1, 3]) 
+
+void multiple_vardecl_in_declstmt() {
+  int *ptr = getIntPtr(), i;
+}
+// [B2 (ENTRY)] -> [B1] -> [B0 (EXIT)]
+
+// CHECK:  GEN sets: blockid (varname [blockid, elementid])
+// CHECK-NEXT: 1 (global_var, [1, 2]) 
+// CHECK-NEXT: 1 (ptr, [1, 3]) 
+// CHECK-NEXT: 1 (i, [1, 4]) 
+// CHECK-NEXT: KILL sets: blockid (varname [blockid, elementid])
+// CHECK-NEXT: Reaching definition sets: blockid IN/OUT (varname [blockid, elementid])
+// CHECK-NEXT: 0 IN (global_var, [1, 2]) 
+// CHECK-NEXT: 0 IN (ptr, [1, 3]) 
+// CHECK-NEXT: 0 IN (i, [1, 4]) 
+// CHECK-NEXT: 0 OUT (global_var, [1, 2]) 
+// CHECK-NEXT: 0 OUT (ptr, [1, 3]) 
+// CHECK-NEXT: 0 OUT (i, [1, 4]) 
+// CHECK-NEXT: 1 OUT (global_var, [1, 2]) 
+// CHECK-NEXT: 1 OUT (ptr, [1, 3]) 
+// CHECK-NEXT: 1 OUT (i, [1, 4]) 
+
+void function_and_vardecl_in_declstmt() {
+  int *ptr = getIntPtr(), a();
+}
+// [B2 (ENTRY)] -> [B1] -> [B0 (EXIT)]
+
+// CHECK:  GEN sets: blockid (varname [blockid, elementid])
+// CHECK-NEXT: 1 (global_var, [1, 2]) 
+// CHECK-NEXT: 1 (ptr, [1, 3]) 
+// CHECK-NEXT: KILL sets: blockid (varname [blockid, elementid])
+// CHECK-NEXT: Reaching definition sets: blockid IN/OUT (varname [blockid, elementid])
+// CHECK-NEXT: 0 IN (global_var, [1, 2]) 
+// CHECK-NEXT: 0 IN (ptr, [1, 3]) 
+// CHECK-NEXT: 0 OUT (global_var, [1, 2]) 
+// CHECK-NEXT: 0 OUT (ptr, [1, 3]) 
+// CHECK-NEXT: 1 OUT (global_var, [1, 2]) 
+// CHECK-NEXT: 1 OUT (ptr, [1, 3]) 
+
+void single_def_in_same_block() {
+  int *ptr = getIntPtr();
+
+  if (coin())
+ptr = 0;
+
+  if (!ptr)
+*ptr = 5;
+}
+//-> [B3] ->-> [B1] ->
+//   /  \  /  \
+// [B5 (ENTRY)] -> [B4] --> [B2] ---> [B0 (EXIT)]
+
+// CHECK:  GEN sets: blockid (varname [blockid, elementid])
+// CHECK-NEXT: 3 (ptr, [3, 3]) 
+// CHECK-NEXT: 4 (global_var, [4, 6]) 
+// CHECK-NEXT: 4 (ptr, [4, 3]) 
+// CHECK-NEXT: KILL sets: blockid (varname [blockid, elementid])
+// CHECK-NEXT: 3 (ptr, [4, 3]) 
+// CHECK-NEXT: 4 (ptr, [3, 3]) 
+// CHECK-NEXT: Reaching definition sets: blockid IN/OUT (varname [blockid, elementid])
+// CHECK-NEXT: 0 IN (global_var, [4, 6]) 
+// CHECK-NEXT: 0 IN (ptr, [3, 3]) 
+// CHECK-NEXT: 0 IN (ptr, [4, 3]) 
+// CHECK-NEXT: 0 OUT (global_var, [4, 6]) 
+// 

[PATCH] D68252: [Stats] Add ALWAYS_ENABLED_STATISTIC enabled regardless of LLVM_ENABLE_STATS.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

On release build with assertions disabled and LLVM_ENABLE_STATS being 0 the 
impact on the compile time is the following

  Metric: compile_time
  
  Programoutput0jM4H6 output_KEgWj diff 
 
   test-suite...tTests/2003-05-07-VarArgs.test 0.03 0.02   
-25.2%
   test-suite...sts/2003-05-31-CastToBool.test 0.02 0.02   
22.1% 
   test-suite...ing-flt/Equivalencing-flt.test 0.61 0.73   
20.7% 
   test-suite...s/2009-12-07-StructReturn.test 0.02 0.02   
17.1% 
   test-suite...ts/2003-07-06-IntOverflow.test 0.02 0.02   
17.0% 
   test-suite...ts/block-copied-in-cxxobj.test 0.03 0.03   
16.2% 
   test-suite...e/UnitTests/ObjC/messages.test 0.03 0.04   
15.9% 
   test-suite...r-algebra/kernels/2mm/2mm.test 0.08 0.06   
-14.7%
   test-suite.../instance-method-metadata.test 0.37 0.42   
14.0% 
   test-suite...ource/UnitTests/printargs.test 0.02 0.02   
12.4% 
   test-suite...itTests/2003-05-26-Shorts.test 0.02 0.02   
-12.2%
   test-suite...-06-20-StaticBitfieldInit.test 0.02 0.02   
12.2% 
   test-suite...tTests/2002-05-03-NotTest.test 0.02 0.02   
11.5% 
   test-suite...tTests/C++11/stdthreadbug.test 0.28 0.26   
-10.3%
   test-suite...e/Benchmarks/Misc/flops-7.test 0.03 0.02   
-10.2%
   Geomean difference   
nan% 
 output0jM4H6  output_KEgWjdiff
  count  542.00542.00424.00
  mean   0.550511  0.547688 -0.001379  
  std2.21  2.205268  0.045026  
  min0.00  0.00 -0.252280  
  25%0.015875  0.016725 -0.021092  
  50%0.040950  0.04 -0.000875  
  75%0.350750  0.355700  0.017304  
  max30.343300 30.326700 0.220588

To me it looks that the change in compile time is within the noise.


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

https://reviews.llvm.org/D68252



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

[PATCH] D68252: [Stats] Add ALWAYS_ENABLED_STATISTIC enabled regardless of LLVM_ENABLE_STATS.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai created this revision.
vsapsai added reviewers: dsanders, bogner, rtereshin.
Herald added subscribers: ributzka, dexonsmith, jkorous, hiraditya.
Herald added a project: LLVM.

The intended usage is to measure relatively expensive operations. So the
cost of the statistic is negligible compared to the cost of a measured
operation and can be enabled all the time without impairing the
compilation time.

The change in clang is an example of where ALWAYS_ENABLED_STATISTIC can be
used. I'm also planning to add more stats for header search and modules.

rdar://problem/55715134


https://reviews.llvm.org/D68252

Files:
  clang/include/clang/Basic/FileManager.h
  clang/include/clang/Lex/HeaderSearch.h
  clang/lib/Basic/FileManager.cpp
  clang/lib/Lex/HeaderSearch.cpp
  llvm/include/llvm/ADT/Statistic.h
  llvm/lib/Support/Statistic.cpp
  llvm/unittests/ADT/StatisticTest.cpp

Index: llvm/unittests/ADT/StatisticTest.cpp
===
--- llvm/unittests/ADT/StatisticTest.cpp
+++ llvm/unittests/ADT/StatisticTest.cpp
@@ -17,6 +17,7 @@
 #define DEBUG_TYPE "unittest"
 STATISTIC(Counter, "Counts things");
 STATISTIC(Counter2, "Counts other things");
+ALWAYS_ENABLED_STATISTIC(AlwaysCounter, "Counts things always");
 
 #if LLVM_ENABLE_STATS
 static void
@@ -43,6 +44,12 @@
 #else
   EXPECT_EQ(Counter, 0u);
 #endif
+
+  AlwaysCounter = 0;
+  EXPECT_EQ(AlwaysCounter, 0u);
+  AlwaysCounter++;
+  ++AlwaysCounter;
+  EXPECT_EQ(AlwaysCounter, 2u);
 }
 
 TEST(StatisticTest, Assign) {
@@ -54,6 +61,9 @@
 #else
   EXPECT_EQ(Counter, 0u);
 #endif
+
+  AlwaysCounter = 2;
+  EXPECT_EQ(AlwaysCounter, 2u);
 }
 
 TEST(StatisticTest, API) {
Index: llvm/lib/Support/Statistic.cpp
===
--- llvm/lib/Support/Statistic.cpp
+++ llvm/lib/Support/Statistic.cpp
@@ -57,7 +57,7 @@
 /// This class is also used to look up statistic values from applications that
 /// use LLVM.
 class StatisticInfo {
-  std::vector Stats;
+  std::vector Stats;
 
   friend void llvm::PrintStatistics();
   friend void llvm::PrintStatistics(raw_ostream );
@@ -66,14 +66,12 @@
   /// Sort statistics by debugtype,name,description.
   void sort();
 public:
-  using const_iterator = std::vector::const_iterator;
+  using const_iterator = std::vector::const_iterator;
 
   StatisticInfo();
   ~StatisticInfo();
 
-  void addStatistic(Statistic *S) {
-Stats.push_back(S);
-  }
+  void addStatistic(TrackingStatistic *S) { Stats.push_back(S); }
 
   const_iterator begin() const { return Stats.begin(); }
   const_iterator end() const { return Stats.end(); }
@@ -90,7 +88,7 @@
 
 /// RegisterStatistic - The first time a statistic is bumped, this method is
 /// called.
-void Statistic::RegisterStatistic() {
+void TrackingStatistic::RegisterStatistic() {
   // If stats are enabled, inform StatInfo that this statistic should be
   // printed.
   // llvm_shutdown calls destructors while holding the ManagedStatic mutex.
@@ -135,15 +133,16 @@
 }
 
 void StatisticInfo::sort() {
-  llvm::stable_sort(Stats, [](const Statistic *LHS, const Statistic *RHS) {
-if (int Cmp = std::strcmp(LHS->getDebugType(), RHS->getDebugType()))
-  return Cmp < 0;
+  llvm::stable_sort(
+  Stats, [](const TrackingStatistic *LHS, const TrackingStatistic *RHS) {
+if (int Cmp = std::strcmp(LHS->getDebugType(), RHS->getDebugType()))
+  return Cmp < 0;
 
-if (int Cmp = std::strcmp(LHS->getName(), RHS->getName()))
-  return Cmp < 0;
+if (int Cmp = std::strcmp(LHS->getName(), RHS->getName()))
+  return Cmp < 0;
 
-return std::strcmp(LHS->getDesc(), RHS->getDesc()) < 0;
-  });
+return std::strcmp(LHS->getDesc(), RHS->getDesc()) < 0;
+  });
 }
 
 void StatisticInfo::reset() {
@@ -207,7 +206,7 @@
   // Print all of the statistics.
   OS << "{\n";
   const char *delim = "";
-  for (const Statistic *Stat : Stats.Stats) {
+  for (const TrackingStatistic *Stat : Stats.Stats) {
 OS << delim;
 assert(yaml::needsQuotes(Stat->getDebugType()) == yaml::QuotingType::None &&
"Statistic group/type name is simple.");
Index: llvm/include/llvm/ADT/Statistic.h
===
--- llvm/include/llvm/ADT/Statistic.h
+++ llvm/include/llvm/ADT/Statistic.h
@@ -44,7 +44,7 @@
 class raw_fd_ostream;
 class StringRef;
 
-class Statistic {
+class StatisticBase {
 public:
   const char *DebugType;
   const char *Name;
@@ -52,6 +52,10 @@
   std::atomic Value;
   std::atomic Initialized;
 
+  StatisticBase(const char *DebugType, const char *Name, const char *Desc)
+  : DebugType(DebugType), Name(Name), Desc(Desc), Value(0),
+Initialized(false) {}
+
   unsigned getValue() const { return Value.load(std::memory_order_relaxed); }
   const char *getDebugType() const { return DebugType; }
   const char *getName() const { return Name; }
@@ -68,14 +72,18 @@
 
   // Allow use of this class as the 

[PATCH] D68251: [clang-tidy] Fix module registry name and description for Darwin clang-tidy module.

[Michael Wyman via Phabricator via cfe-commits]

mwyman created this revision.
mwyman added reviewers: stephanemoore, benhamilton, gribozavr.
mwyman added projects: clang, clang-tools-extra, LLVM.
Herald added subscribers: cfe-commits, xazax.hun.

When creating the module, must have copy-pasted from the misc module, and 
forgotten to update the name/description of the module in the registry.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D68251

Files:
  clang-tools-extra/clang-tidy/darwin/DarwinTidyModule.cpp


Index: clang-tools-extra/clang-tidy/darwin/DarwinTidyModule.cpp
===
--- clang-tools-extra/clang-tidy/darwin/DarwinTidyModule.cpp
+++ clang-tools-extra/clang-tidy/darwin/DarwinTidyModule.cpp
@@ -27,7 +27,7 @@
 
 // Register the DarwinTidyModule using this statically initialized variable.
 static ClangTidyModuleRegistry::Add
-X("misc-module", "Adds miscellaneous lint checks.");
+X("darwin-module", "Adds Darwin-specific lint checks.");
 
 // This anchor is used to force the linker to link in the generated object file
 // and thus register the DarwinModule.


Index: clang-tools-extra/clang-tidy/darwin/DarwinTidyModule.cpp
===
--- clang-tools-extra/clang-tidy/darwin/DarwinTidyModule.cpp
+++ clang-tools-extra/clang-tidy/darwin/DarwinTidyModule.cpp
@@ -27,7 +27,7 @@
 
 // Register the DarwinTidyModule using this statically initialized variable.
 static ClangTidyModuleRegistry::Add
-X("misc-module", "Adds miscellaneous lint checks.");
+X("darwin-module", "Adds Darwin-specific lint checks.");
 
 // This anchor is used to force the linker to link in the generated object file
 // and thus register the DarwinModule.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65543: [Windows] Autolink with basenames and add libdir to libpath

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

/Brepro seems orthogonal to me. If you only pass relative paths and 
-no-canonical-prefixes then the embedded path is relative and doesn't impeded 
determinism.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65543



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

r373272 - Fix crash on value-dependent delete-expressions.

[Richard Smith via cfe-commits]

Author: rsmith
Date: Mon Sep 30 15:55:27 2019
New Revision: 373272

URL: http://llvm.org/viewvc/llvm-project?rev=373272=rev
Log:
Fix crash on value-dependent delete-expressions.

We used to miscompute the 'value-dependent' bit, and would crash if we
tried to evaluate a delete expression that should be value-dependent.

Modified:
cfe/trunk/include/clang/AST/ExprCXX.h
cfe/trunk/test/SemaCXX/constant-expression-cxx2a.cpp

Modified: cfe/trunk/include/clang/AST/ExprCXX.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ExprCXX.h?rev=373272=373271=373272=diff
==
--- cfe/trunk/include/clang/AST/ExprCXX.h (original)
+++ cfe/trunk/include/clang/AST/ExprCXX.h Mon Sep 30 15:55:27 2019
@@ -2278,8 +2278,8 @@ public:
   CXXDeleteExpr(QualType Ty, bool GlobalDelete, bool ArrayForm,
 bool ArrayFormAsWritten, bool UsualArrayDeleteWantsSize,
 FunctionDecl *OperatorDelete, Expr *Arg, SourceLocation Loc)
-  : Expr(CXXDeleteExprClass, Ty, VK_RValue, OK_Ordinary, false, false,
- Arg->isInstantiationDependent(),
+  : Expr(CXXDeleteExprClass, Ty, VK_RValue, OK_Ordinary, false,
+ Arg->isValueDependent(), Arg->isInstantiationDependent(),
  Arg->containsUnexpandedParameterPack()),
 OperatorDelete(OperatorDelete), Argument(Arg) {
 CXXDeleteExprBits.GlobalDelete = GlobalDelete;

Modified: cfe/trunk/test/SemaCXX/constant-expression-cxx2a.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/constant-expression-cxx2a.cpp?rev=373272=373271=373272=diff
==
--- cfe/trunk/test/SemaCXX/constant-expression-cxx2a.cpp (original)
+++ cfe/trunk/test/SemaCXX/constant-expression-cxx2a.cpp Mon Sep 30 15:55:27 
2019
@@ -1056,6 +1056,12 @@ namespace delete_random_things {
   static_assert((delete &(int&)(int&&)0, true)); // expected-error {{}} 
expected-note {{delete of pointer '&0' that does not point to a heap-allocated 
object}} expected-note {{temporary created here}}
 }
 
+namespace value_dependent_delete {
+  template void f(T *p) {
+int arr[(delete p, 0)];
+  }
+}
+
 namespace memory_leaks {
   static_assert(*new bool(true)); // expected-error {{}} expected-note 
{{allocation performed here was not deallocated}}
 


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

[PATCH] D63960: [C++20] Add consteval-specific semantic for functions

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

rsmith added a comment.

Very cool, this is an elegant approach.




Comment at: clang/include/clang/Basic/DiagnosticASTKinds.td:57
+def note_consteval_address_accessible : Note<
+  "%select{pointer|reference}0 on a consteval declaration "
+  "is not a constant expression">;

on -> to



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:2366
+def err_invalid_consteval_decl_kind : Error<
+  "operator %select{new|delete|new[]|delete[]}0 cannot be declared consteval">;
 def err_invalid_constexpr : Error<

Please add 's around the function name. Instead of encoding the name as an 
integer, you can stream the declaration itself into the diagnostic and just use 
`"%0 cannot be declared consteval"` here.



Comment at: clang/include/clang/Sema/Sema.h:806-808
+  /// Should be set for most operation.
+  /// It is unset to disable tracking of immediate invocation candidates and
+  /// reference on consteval.

The comment should first describe what this field means, before saying how to 
use it (if necessary). So:

```
/// Whether immediate invocation candidates and references to consteval 
functions should be tracked.
```

or something like that.

That said, consider renaming this to `RebuildingImmediateInvocation` to avoid 
suggesting it's a general mechanism to turn off immediate invocation tracking.



Comment at: clang/include/clang/Sema/Sema.h:1072
+llvm::SmallVector, 8>
+ImmediateInvocationsCandidates;
+

`ImmediateInvocationsCandidates` -> `ImmediateInvocationCandidates`



Comment at: clang/include/clang/Sema/Sema.h:1076
+/// context not already know to be immediately invoked.
+llvm::SmallPtrSet ReferenceOnConsteval;
+

`ReferenceOnConsteval` -> `ReferencesToConsteval` or similar.



Comment at: clang/lib/AST/ExprConstant.cpp:1932
+  if (FD->isConsteval()) {
+Info.FFDiag(Loc, diag::note_consteval_address_accessible)
+<< !Type->isAnyPointerType();

It would be useful for the diagnostic to say what consteval function we have a 
pointer to, and maybe include a note pointing at its declaration.



Comment at: clang/lib/Sema/SemaDeclCXX.cpp:11188-11189
 
+  ConstexprSpecKind ConstexprKind = DetermineSpecialMemberConstexprKind(
+  Constexpr, ClassDecl->hasDefaultedConstevalDefaultCtor());
+

I don't think this is necessary: implicit special members are never 
`consteval`. (We use `DeclareImplicitDefaultConstructor` when there is no 
user-declared default constructor, in which case there can't possibly be a 
defaulted consteval default constructor.) I don't think you need any of the 
`DetermineSpecialMemberConstexprKind` machinery, nor the tracking of 
`DefaultedSpecialMemberIsConsteval` in `CXXRecordDecl`.



Comment at: clang/lib/Sema/SemaExpr.cpp:15032
+return E;
+  if (auto *Call = dyn_cast(E.get()))
+if (auto *DeclRef =

rsmith wrote:
> Please add a comment to this to point out that it's just an optimization, for 
> example:
> 
> "Opportunistically remove the callee from ReferencesToConsteval if we can. 
> It's OK if this fails; we'll also remove this in HandleImmediateInvocations, 
> but catching it here allows us to avoid walking the AST looking for it in 
> simple cases."
Consider using `E.get()->IgnoreImplicit()` here; we want to step over (at 
least) any `CXXBindTemporaryExpr`s inserted by `MaybeBindToTemporary`.



Comment at: clang/lib/Sema/SemaExpr.cpp:15032-15035
+  if (auto *Call = dyn_cast(E.get()))
+if (auto *DeclRef =
+dyn_cast(Call->getCallee()->IgnoreImplicit()))
+  ExprEvalContexts.back().ReferenceOnConsteval.erase(DeclRef);

Please add a comment to this to point out that it's just an optimization, for 
example:

"Opportunistically remove the callee from ReferencesToConsteval if we can. It's 
OK if this fails; we'll also remove this in HandleImmediateInvocations, but 
catching it here allows us to avoid walking the AST looking for it in simple 
cases."



Comment at: clang/lib/Sema/SemaExpr.cpp:15036-15039
+  ConstantExpr *Res = ConstantExpr::Create(
+  getASTContext(), E.get(),
+  ConstantExpr::getStorageKind(E.get()->getType().getTypePtr(),
+   getASTContext()));

We will need an `ExprWithCleanups` wrapping `E.get()` if there might be any 
temporaries within it.

We don't actually know whether there definitely are any such temporaries, but 
we can conservatively assume we might need an `ExprWithCleanups` if 
`Cleanup.exprNeedsCleanups()` returns `true`.

You can test this with something like:

```
struct A { int *p = new int(42); consteval int get() { return *p; } constexpr 
~A() { delete p; } };
int k = A().get();
```

... which should be valid, but 

[PATCH] D68193: In openFileForRead don't cache erroneous entries if the error relates to them being directories. Add tests.

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith added a comment.

Sorry for bouncing you around, but I just had a look at the other user of 
`createFileEntry` and I think the right thing to do is to somehow share the 
code between `DependencyScanningWorkerFilesystem::status` and this.  I suggest 
splitting a function out of `status` (called `getOrCreateFileSystemEntry`?) 
that returns a `CachedFileSystemEntry` (or an error).

The fix I just asked you to make (to only call `stat` once) could be done on 
`getOrCreateFileSystemEntry` as a follow-up, using 
`std::unique_ptr` and changing `getFileEntry` to take that 
instead of a filename.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68193



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

[PATCH] D68245: [Clangd] ExtractFunction: Don't extract body of enclosing function.

[Shaurya Gupta via Phabricator via cfe-commits]

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

This patch disable extraction of the body of the enclosing function.
`void f() [[{}]]`

Extracting this CompoundStmt would leave the enclosing function without
a body.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D68245

Files:
  clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
  clang-tools-extra/clangd/unittests/TweakTests.cpp


Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -631,6 +631,14 @@
 F ([[int x = 0;]])
   )cpp";
   EXPECT_EQ(apply(MacroFailInput), "unavailable");
+
+  // Don't extract if we select the entire function body (CompoundStmt).
+  std::string CompoundFailInput = R"cpp(
+void f() [[{
+  int a;
+}]]
+  )cpp";
+  EXPECT_EQ(apply(CompoundFailInput), "unavailable");
 }
 
 TEST_F(ExtractFunctionTest, ControlFlow) {
Index: clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
@@ -218,6 +218,21 @@
   return toHalfOpenFileRange(SM, LangOpts, 
EnclosingFunction->getSourceRange());
 }
 
+// returns true if Child can be a single RootStmt being extracted from
+// EnclosingFunc.
+bool validSingleChild(const Node *Child, const FunctionDecl *EnclosingFunc) {
+  // Don't extract expressions.
+  // FIXME: We should extract expressions that are "statements" i.e. not
+  // subexpressions
+  if (Child->ASTNode.get())
+return false;
+  // Extracting the body of EnclosingFunc would remove it's definition.
+  if (const Stmt *S = Child->ASTNode.get())
+if (EnclosingFunc->hasBody() && EnclosingFunc->getBody() == S)
+  return false;
+  return true;
+}
+
 // FIXME: Check we're not extracting from the initializer/condition of a 
control
 // flow structure.
 // FIXME: Check that we don't extract the compound statement of the
@@ -229,15 +244,14 @@
   ExtZone.Parent = getParentOfRootStmts(CommonAnc);
   if (!ExtZone.Parent || ExtZone.Parent->Children.empty())
 return llvm::None;
-  // Don't extract expressions.
-  // FIXME: We should extract expressions that are "statements" i.e. not
-  // subexpressions
-  if (ExtZone.Parent->Children.size() == 1 &&
-  ExtZone.getLastRootStmt()->ASTNode.get())
-return llvm::None;
   ExtZone.EnclosingFunction = findEnclosingFunction(ExtZone.Parent);
   if (!ExtZone.EnclosingFunction)
 return llvm::None;
+  // When there is a single RootStmt, we must check if it's valid for
+  // extraction.
+  if (ExtZone.Parent->Children.size() == 1 &&
+  !validSingleChild(ExtZone.getLastRootStmt(), ExtZone.EnclosingFunction))
+return llvm::None;
   if (auto FuncRange =
   computeEnclosingFuncRange(ExtZone.EnclosingFunction, SM, LangOpts))
 ExtZone.EnclosingFuncRange = *FuncRange;


Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -631,6 +631,14 @@
 F ([[int x = 0;]])
   )cpp";
   EXPECT_EQ(apply(MacroFailInput), "unavailable");
+
+  // Don't extract if we select the entire function body (CompoundStmt).
+  std::string CompoundFailInput = R"cpp(
+void f() [[{
+  int a;
+}]]
+  )cpp";
+  EXPECT_EQ(apply(CompoundFailInput), "unavailable");
 }
 
 TEST_F(ExtractFunctionTest, ControlFlow) {
Index: clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
@@ -218,6 +218,21 @@
   return toHalfOpenFileRange(SM, LangOpts, EnclosingFunction->getSourceRange());
 }
 
+// returns true if Child can be a single RootStmt being extracted from
+// EnclosingFunc.
+bool validSingleChild(const Node *Child, const FunctionDecl *EnclosingFunc) {
+  // Don't extract expressions.
+  // FIXME: We should extract expressions that are "statements" i.e. not
+  // subexpressions
+  if (Child->ASTNode.get())
+return false;
+  // Extracting the body of EnclosingFunc would remove it's definition.
+  if (const Stmt *S = Child->ASTNode.get())
+if (EnclosingFunc->hasBody() && EnclosingFunc->getBody() == S)
+  return false;
+  return true;
+}
+
 // FIXME: Check we're not extracting from the initializer/condition of a control
 // flow structure.
 // FIXME: Check that we don't extract the compound statement of the
@@ -229,15 +244,14 @@
   

[PATCH] D67113: ICK_Function_Conversion is a third kind conversion

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert added a comment.

Ping?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67113



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

[PATCH] D68187: [libclang] Use strict prototypes in header

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert abandoned this revision.
aaronpuchert added a comment.

Fixed by @aaron.ballman in rC373213  along 
with another issue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68187



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

[PATCH] D68182: [Clangd] Ensure children are always RootStmt in ExtractFunction (Fixes #153)

[Shaurya Gupta via Phabricator via cfe-commits]

SureYeaah updated this revision to Diff 222499.
SureYeaah added a comment.

Moved Unittest


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68182

Files:
  clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
  clang-tools-extra/clangd/unittests/TweakTests.cpp


Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -554,7 +554,7 @@
   EXPECT_THAT(apply(" [[int a = 5;]] a++; "), StartsWith("fail"));
   // Don't extract return
   EXPECT_THAT(apply(" if(true) [[return;]] "), StartsWith("fail"));
-  
+
 }
 
 TEST_F(ExtractFunctionTest, FileTest) {
@@ -631,6 +631,9 @@
 F ([[int x = 0;]])
   )cpp";
   EXPECT_EQ(apply(MacroFailInput), "unavailable");
+
+  // Shouldn't crash.
+  EXPECT_EQ(apply("void f([[int a]]);"), "unavailable");
 }
 
 TEST_F(ExtractFunctionTest, ControlFlow) {
Index: clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
@@ -111,26 +111,29 @@
 const Node *getParentOfRootStmts(const Node *CommonAnc) {
   if (!CommonAnc)
 return nullptr;
+  const Node* Parent = CommonAnc;
   switch (CommonAnc->Selected) {
-  case SelectionTree::Selection::Unselected:
-// Ensure all Children are RootStmts.
-return llvm::all_of(CommonAnc->Children, isRootStmt) ? CommonAnc : nullptr;
-  case SelectionTree::Selection::Partial:
-// Treat Partially selected VarDecl as completely selected since
-// SelectionTree doesn't always select VarDecls correctly.
-// FIXME: Remove this after D66872 is upstream)
-if (!CommonAnc->ASTNode.get())
-  return nullptr;
-LLVM_FALLTHROUGH;
-  case SelectionTree::Selection::Complete:
-// If the Common Ancestor is completely selected, then it's a root 
statement
-// and its parent will be unselected.
-const Node *Parent = CommonAnc->Parent;
-// If parent is a DeclStmt, even though it's unselected, we consider it a
-// root statement and return its parent. This is done because the VarDecls
-// claim the entire selection range of the Declaration and DeclStmt is
-// always unselected.
-return Parent->ASTNode.get() ? Parent->Parent : Parent;
+case SelectionTree::Selection::Partial:
+  // Treat Partially selected VarDecl as completely selected since
+  // SelectionTree doesn't always select VarDecls correctly.
+  // FIXME: Remove this after D66872 is upstream)
+  if (!CommonAnc->ASTNode.get())
+return nullptr;
+  LLVM_FALLTHROUGH;
+case SelectionTree::Selection::Complete:
+  // If the Common Ancestor is completely selected, then it's a root 
statement
+  // and its parent will be unselected.
+  Parent = CommonAnc->Parent;
+  // If parent is a DeclStmt, even though it's unselected, we consider it a
+  // root statement and return its parent. This is done because the 
VarDecls
+  // claim the entire selection range of the Declaration and DeclStmt is
+  // always unselected.
+  if(Parent->ASTNode.get())
+Parent = Parent->Parent;
+  LLVM_FALLTHROUGH;
+case SelectionTree::Selection::Unselected:
+  // Ensure all Children are RootStmts.
+  return llvm::all_of(Parent->Children, isRootStmt) ? Parent : nullptr;
   }
   llvm_unreachable("Unhandled SelectionTree::Selection enum");
 }


Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -554,7 +554,7 @@
   EXPECT_THAT(apply(" [[int a = 5;]] a++; "), StartsWith("fail"));
   // Don't extract return
   EXPECT_THAT(apply(" if(true) [[return;]] "), StartsWith("fail"));
-  
+
 }
 
 TEST_F(ExtractFunctionTest, FileTest) {
@@ -631,6 +631,9 @@
 F ([[int x = 0;]])
   )cpp";
   EXPECT_EQ(apply(MacroFailInput), "unavailable");
+
+  // Shouldn't crash.
+  EXPECT_EQ(apply("void f([[int a]]);"), "unavailable");
 }
 
 TEST_F(ExtractFunctionTest, ControlFlow) {
Index: clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
@@ -111,26 +111,29 @@
 const Node *getParentOfRootStmts(const Node *CommonAnc) {
   if (!CommonAnc)
 return nullptr;
+  const Node* Parent = CommonAnc;
   switch (CommonAnc->Selected) {
-  case SelectionTree::Selection::Unselected:
-// Ensure all Children are RootStmts.
-return llvm::all_of(CommonAnc->Children, isRootStmt) ? 

r373268 - Make function static that didn't need linkage.

[Erich Keane via cfe-commits]

Author: erichkeane
Date: Mon Sep 30 14:24:04 2019
New Revision: 373268

URL: http://llvm.org/viewvc/llvm-project?rev=373268=rev
Log:
Make function static that didn't need linkage.

In r373247 I added a helper function, but neglected to make it static.

Modified:
cfe/trunk/lib/AST/DeclCXX.cpp

Modified: cfe/trunk/lib/AST/DeclCXX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclCXX.cpp?rev=373268=373267=373268=diff
==
--- cfe/trunk/lib/AST/DeclCXX.cpp (original)
+++ cfe/trunk/lib/AST/DeclCXX.cpp Mon Sep 30 14:24:04 2019
@@ -1399,7 +1399,7 @@ static bool allLookupResultsAreTheSame(c
 }
 #endif
 
-NamedDecl* getLambdaCallOperatorHelper(const CXXRecordDecl ) {
+static NamedDecl* getLambdaCallOperatorHelper(const CXXRecordDecl ) {
   if (!RD.isLambda()) return nullptr;
   DeclarationName Name =
 RD.getASTContext().DeclarationNames.getCXXOperatorName(OO_Call);


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

[PATCH] D67079: [analyzer] CastValueChecker: Model inheritance

[Csaba Dabis via Phabricator via cfe-commits]

Charusso marked 4 inline comments as done.
Charusso added inline comments.



Comment at: clang/test/Analysis/cast-value-hierarchy-fp-suppression.cpp:25-27
+  if (isa(a))
+if (isa(a))
+  clang_analyzer_warnIfReached(); // no-warning

NoQ wrote:
> NoQ wrote:
> > Charusso wrote:
> > > NoQ wrote:
> > > > Charusso wrote:
> > > > > NoQ wrote:
> > > > > > Why is `(isa(a) && isa(a))` deemed possible in the first test 
> > > > > > but not in the second test? o_o
> > > > > In `test_downcast()` we assume that `a` is a record type of `D` where 
> > > > > `D` is a `B` and `D` is a `C`.  However in 
> > > > > `test_downcast_infeasible()` if `a` is not a record type of `D` is 
> > > > > cannot be both `B` and `C` at the same time. That is the purpose of 
> > > > > `CastVisitor`.
> > > > I mean, it contradicts to how the program *actually* works, so we 
> > > > should either not do that, or provide a reaallly compelling 
> > > > explanation of why we do this (as in "Extraordinary Claims Require 
> > > > Extraordinary Evidence").
> > > Are you sure it does not model the program? I have an `Apple` class and I 
> > > have a `Pen` class, until it is not defining an `ApplePen` class it is a 
> > > false assumption to say they are defining an `ApplePen` class. I wanted 
> > > to prefetch that information before the modeling starts, but it was an 
> > > impossible challenge for me, so I have picked that `CastVisitor`-based 
> > > post-elimination idea. In the real world I have removed only two false 
> > > assumptions with the visitor from 1200 reports of LLVM so an `ApplePen` 
> > > is very rare (https://www.youtube.com/watch?v=Ct6BUPvE2sM).
> > I'm sure that the possibility of taking a true branch in `if (x)` only 
> > depends on the value of `x`, not on the contents of the branch.
> I.e., i see the point that you're trying to make (roughly), but it still 
> requires extraordinary evindence and the way you've written the test clearly 
> demonstrates why it needs an extraordinary evidence.
> 
> What you were trying to say is "imagine there's no class `D`, then we 
> probably shouldn't consider the situation that it exists". But in this test 
> the class exists, and you've just written a test about it, and then you're 
> telling me that it doesn't exist. Which is exactly the problem with not 
> considering that `D` exists. And this is exactly why choosing such approach 
> requires a discussion.
> 
> One reason why i doubt the visitor-based solution is that the existence of 
> `D` is not a path-sensitive fact; it either exists or not, it is irrelevant 
> whether it was mentioned in the current path. Therefore scanning all classes 
> in the translation unit, as opposed to classes mentioned on the current path, 
> seems more precise.
Well, that is a better idea, yes. I really like your observation about `if 
(x)`, thanks for them! During the Summer I have learnt a lot, but a lot about 
the issue of the one translation unit, so that is why I was very afraid of 
using it. Let us give it a try.


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

https://reviews.llvm.org/D67079



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

[PATCH] D68227: [clang-format] [PR43372] - clang-format shows replacements in DOS files when no replacement is needed

[MyDeveloperDay via Phabricator via cfe-commits]

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

Being specific about replacing "\r\n"'s with "\n"'s rather than removing just 
\r's


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

https://reviews.llvm.org/D68227

Files:
  clang/lib/Format/Format.cpp
  clang/unittests/Format/SortImportsTestJava.cpp
  clang/unittests/Format/SortIncludesTest.cpp


Index: clang/unittests/Format/SortIncludesTest.cpp
===
--- clang/unittests/Format/SortIncludesTest.cpp
+++ clang/unittests/Format/SortIncludesTest.cpp
@@ -724,6 +724,14 @@
   EXPECT_EQ(Code, sort(Code, "input.h", 0));
 }
 
+TEST_F(SortIncludesTest,
+   DoNotOutputReplacementsForSortedBlocksWithRegroupingWindows) {
+  Style.IncludeBlocks = Style.IBS_Regroup;
+  std::string Code = "#include \"b.h\"\r\n"
+ "\r\n"
+ "#include \r\n";
+  EXPECT_EQ(Code, sort(Code, "input.h", 0));
+}
 
 TEST_F(SortIncludesTest, DoNotRegroupGroupsInGoogleObjCStyle) {
   FmtStyle = getGoogleStyle(FormatStyle::LK_ObjC);
Index: clang/unittests/Format/SortImportsTestJava.cpp
===
--- clang/unittests/Format/SortImportsTestJava.cpp
+++ clang/unittests/Format/SortImportsTestJava.cpp
@@ -285,6 +285,13 @@
   sortIncludes(FmtStyle, Code, GetCodeRange(Code), "input.java").empty());
 }
 
+TEST_F(SortImportsTestJava, NoReplacementsForValidImportsWindows) {
+  std::string Code = "import org.a;\r\n"
+ "import org.b;\r\n";
+  EXPECT_TRUE(
+  sortIncludes(FmtStyle, Code, GetCodeRange(Code), "input.java").empty());
+}
+
 } // end namespace
 } // end namespace format
 } // end namespace clang
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -1825,6 +1825,22 @@
   return std::make_pair(CursorIndex, OffsetToEOL);
 }
 
+// Replace "\r\n" with "\n"
+std::string replaceCRLF(std::string Code) {
+  size_t Pos = 0;
+  while ((Pos = Code.find("\r\n", Pos)) != std::string::npos) {
+Code.replace(Pos, 2, "\n");
+Pos += 1;
+  }
+  return Code;
+}
+
+// Compares two strings but ignores any \r in either strings.
+static bool compareIgnoringCarriageReturns(std::string Result,
+   std::string Code) {
+  return replaceCRLF(Result) == replaceCRLF(Code);
+}
+
 // Sorts and deduplicate a block of includes given by 'Includes' alphabetically
 // adding the necessary replacement to 'Replaces'. 'Includes' must be in strict
 // source order.
@@ -1898,7 +1914,8 @@
 
   // If the #includes are out of order, we generate a single replacement fixing
   // the entire range of blocks. Otherwise, no replacement is generated.
-  if (result == Code.substr(IncludesBeginOffset, IncludesBlockSize))
+  if (compareIgnoringCarriageReturns(
+  result, Code.substr(IncludesBeginOffset, IncludesBlockSize)))
 return;
 
   auto Err = Replaces.add(tooling::Replacement(
@@ -2066,7 +2083,8 @@
 
   // If the imports are out of order, we generate a single replacement fixing
   // the entire block. Otherwise, no replacement is generated.
-  if (result == Code.substr(Imports.front().Offset, ImportsBlockSize))
+  if (compareIgnoringCarriageReturns(
+  result, Code.substr(Imports.front().Offset, ImportsBlockSize)))
 return;
 
   auto Err = Replaces.add(tooling::Replacement(FileName, 
Imports.front().Offset,
@@ -2356,7 +2374,7 @@
 
   auto Env =
   std::make_unique(Code, FileName, Ranges, FirstStartColumn,
- NextStartColumn, LastStartColumn);
+NextStartColumn, LastStartColumn);
   llvm::Optional CurrentCode = None;
   tooling::Replacements Fixes;
   unsigned Penalty = 0;


Index: clang/unittests/Format/SortIncludesTest.cpp
===
--- clang/unittests/Format/SortIncludesTest.cpp
+++ clang/unittests/Format/SortIncludesTest.cpp
@@ -724,6 +724,14 @@
   EXPECT_EQ(Code, sort(Code, "input.h", 0));
 }
 
+TEST_F(SortIncludesTest,
+   DoNotOutputReplacementsForSortedBlocksWithRegroupingWindows) {
+  Style.IncludeBlocks = Style.IBS_Regroup;
+  std::string Code = "#include \"b.h\"\r\n"
+ "\r\n"
+ "#include \r\n";
+  EXPECT_EQ(Code, sort(Code, "input.h", 0));
+}
 
 TEST_F(SortIncludesTest, DoNotRegroupGroupsInGoogleObjCStyle) {
   FmtStyle = getGoogleStyle(FormatStyle::LK_ObjC);
Index: clang/unittests/Format/SortImportsTestJava.cpp
===
--- clang/unittests/Format/SortImportsTestJava.cpp
+++ clang/unittests/Format/SortImportsTestJava.cpp
@@ -285,6 +285,13 @@
   sortIncludes(FmtStyle, Code, GetCodeRange(Code), "input.java").empty());
 }
 
+TEST_F(SortImportsTestJava, 

[PATCH] D67079: [analyzer] CastValueChecker: Model inheritance

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/test/Analysis/cast-value-hierarchy-fp-suppression.cpp:25-27
+  if (isa(a))
+if (isa(a))
+  clang_analyzer_warnIfReached(); // no-warning

NoQ wrote:
> Charusso wrote:
> > NoQ wrote:
> > > Charusso wrote:
> > > > NoQ wrote:
> > > > > Why is `(isa(a) && isa(a))` deemed possible in the first test 
> > > > > but not in the second test? o_o
> > > > In `test_downcast()` we assume that `a` is a record type of `D` where 
> > > > `D` is a `B` and `D` is a `C`.  However in `test_downcast_infeasible()` 
> > > > if `a` is not a record type of `D` is cannot be both `B` and `C` at the 
> > > > same time. That is the purpose of `CastVisitor`.
> > > I mean, it contradicts to how the program *actually* works, so we should 
> > > either not do that, or provide a reaallly compelling explanation 
> > > of why we do this (as in "Extraordinary Claims Require Extraordinary 
> > > Evidence").
> > Are you sure it does not model the program? I have an `Apple` class and I 
> > have a `Pen` class, until it is not defining an `ApplePen` class it is a 
> > false assumption to say they are defining an `ApplePen` class. I wanted to 
> > prefetch that information before the modeling starts, but it was an 
> > impossible challenge for me, so I have picked that `CastVisitor`-based 
> > post-elimination idea. In the real world I have removed only two false 
> > assumptions with the visitor from 1200 reports of LLVM so an `ApplePen` is 
> > very rare (https://www.youtube.com/watch?v=Ct6BUPvE2sM).
> I'm sure that the possibility of taking a true branch in `if (x)` only 
> depends on the value of `x`, not on the contents of the branch.
I.e., i see the point that you're trying to make (roughly), but it still 
requires extraordinary evindence and the way you've written the test clearly 
demonstrates why it needs an extraordinary evidence.

What you were trying to say is "imagine there's no class `D`, then we probably 
shouldn't consider the situation that it exists". But in this test the class 
exists, and you've just written a test about it, and then you're telling me 
that it doesn't exist. Which is exactly the problem with not considering that 
`D` exists. And this is exactly why choosing such approach requires a 
discussion.

One reason why i doubt the visitor-based solution is that the existence of `D` 
is not a path-sensitive fact; it either exists or not, it is irrelevant whether 
it was mentioned in the current path. Therefore scanning all classes in the 
translation unit, as opposed to classes mentioned on the current path, seems 
more precise.


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

https://reviews.llvm.org/D67079



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

[PATCH] D68055: Add -fgnuc-version= to control __GNUC__ and other GCC macros

[Jacob Lifshay via Phabricator via cfe-commits]

programmerjake added a comment.

Shouldn't `__GNUG__` match `__GNUC__`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68055



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

[PATCH] D67837: [CUDA][HIP] Fix host/device check with -fopenmp

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

ping
@ABataev Any comments? Thanks.


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

https://reviews.llvm.org/D67837



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

[PATCH] D67079: [analyzer] CastValueChecker: Model inheritance

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/test/Analysis/cast-value-hierarchy-fp-suppression.cpp:25-27
+  if (isa(a))
+if (isa(a))
+  clang_analyzer_warnIfReached(); // no-warning

Charusso wrote:
> NoQ wrote:
> > Charusso wrote:
> > > NoQ wrote:
> > > > Why is `(isa(a) && isa(a))` deemed possible in the first test but 
> > > > not in the second test? o_o
> > > In `test_downcast()` we assume that `a` is a record type of `D` where `D` 
> > > is a `B` and `D` is a `C`.  However in `test_downcast_infeasible()` if 
> > > `a` is not a record type of `D` is cannot be both `B` and `C` at the same 
> > > time. That is the purpose of `CastVisitor`.
> > I mean, it contradicts to how the program *actually* works, so we should 
> > either not do that, or provide a reaallly compelling explanation of 
> > why we do this (as in "Extraordinary Claims Require Extraordinary 
> > Evidence").
> Are you sure it does not model the program? I have an `Apple` class and I 
> have a `Pen` class, until it is not defining an `ApplePen` class it is a 
> false assumption to say they are defining an `ApplePen` class. I wanted to 
> prefetch that information before the modeling starts, but it was an 
> impossible challenge for me, so I have picked that `CastVisitor`-based 
> post-elimination idea. In the real world I have removed only two false 
> assumptions with the visitor from 1200 reports of LLVM so an `ApplePen` is 
> very rare (https://www.youtube.com/watch?v=Ct6BUPvE2sM).
I'm sure that the possibility of taking a true branch in `if (x)` only depends 
on the value of `x`, not on the contents of the branch.


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

https://reviews.llvm.org/D67079



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

Re: r373258 - [NFC] Fix tests, second try

[Roman Lebedev via cfe-commits]

I'm just wondering, was this not caught in `ninja check-clang` before
the patch that added the warning landed?

On Mon, Sep 30, 2019 at 11:39 PM David Bolvansky via cfe-commits
 wrote:
>
> Author: xbolva00
> Date: Mon Sep 30 13:41:56 2019
> New Revision: 373258
>
> URL: http://llvm.org/viewvc/llvm-project?rev=373258=rev
> Log:
> [NFC] Fix tests, second try
>
> Modified:
> cfe/trunk/test/SemaCXX/warn-sign-conversion.cpp
>
> Modified: cfe/trunk/test/SemaCXX/warn-sign-conversion.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-sign-conversion.cpp?rev=373258=373257=373258=diff
> ==
> --- cfe/trunk/test/SemaCXX/warn-sign-conversion.cpp (original)
> +++ cfe/trunk/test/SemaCXX/warn-sign-conversion.cpp Mon Sep 30 13:41:56 2019
> @@ -70,15 +70,15 @@ namespace test2 {
>  int d1 = 1 ? i : Foo::D; // expected-warning {{operand of ? 
> changes signedness: 'test2::Foo::Named4' to 'int'}}
>  int d2 = 1 ? Foo::D : i; // expected-warning {{operand of ? 
> changes signedness: 'test2::Foo::Named4' to 'int'}}
>  int d3 = 1 ? B : Foo::D; // expected-warning {{operand of ? 
> changes signedness: 'test2::Foo::Named4' to 'int'}}
> -// expected-warning@+1 {{enumeration type mismatch in conditional 
> expression ('test2::Named2' and 'test2::Foo::Named4')}}
> +// expected-warning@-1 {{enumeration type mismatch in conditional 
> expression ('test2::Named2' and 'test2::Foo::Named4')}}
>  int d4 = 1 ? Foo::D : B; // expected-warning {{operand of ? 
> changes signedness: 'test2::Foo::Named4' to 'int'}}
> -// expected-warning@+1 {{enumeration type mismatch in conditional 
> expression ('test2::Foo::Named4' and 'test2::Named2')}}
> +// expected-warning@-1 {{enumeration type mismatch in conditional 
> expression ('test2::Foo::Named4' and 'test2::Named2')}}
>
>  int e1 = 1 ? i : E; // expected-warning {{operand of ? changes 
> signedness: 'test2::Named5' to 'int'}}
>  int e2 = 1 ? E : i; // expected-warning {{operand of ? changes 
> signedness: 'test2::Named5' to 'int'}}
>  int e3 = 1 ? E : B; // expected-warning {{operand of ? changes 
> signedness: 'test2::Named5' to 'int'}}
> -// expected-warning@+1 {{enumeration type mismatch in conditional 
> expression ('test2::Named5' and 'test2::Named2')}}
> +// expected-warning@-1 {{enumeration type mismatch in conditional 
> expression ('test2::Named5' and 'test2::Named2')}}
>  int e4 = 1 ? B : E; // expected-warning {{operand of ? changes 
> signedness: 'test2::Named5' to 'int'}}
> -// expected-warning@+1 {{enumeration type mismatch in conditional 
> expression ('test2::Named2' and 'test2::Named5')}}
> +// expected-warning@-1 {{enumeration type mismatch in conditional 
> expression ('test2::Named2' and 'test2::Named5')}}
>}
>  }
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

RE: r373247 - Teach CallGraph to look into Generic Lambdas.

[Keane, Erich via cfe-commits]

Should be fixe din r373259

From: Nico Weber 
Sent: Monday, September 30, 2019 12:50 PM
To: Keane, Erich 
Cc: cfe-commits 
Subject: Re: r373247 - Teach CallGraph to look into Generic Lambdas.

This broke a few clangd unit tests:

http://lab.llvm.org:8011/builders/clang-ppc64be-linux/builds/38857/steps/ninja%20check%201/logs/FAIL%3A%20Clangd%20Unit%20Tests%3A%3AHover.Structured
http://lab.llvm.org:8011/builders/clang-ppc64be-linux/builds/38857/steps/ninja%20check%201/logs/FAIL%3A%20Clangd%20Unit%20Tests%3A%3AHover.All
http://45.33.8.238/linux/680/step_7.txt

On Mon, Sep 30, 2019 at 3:10 PM Erich Keane via cfe-commits 
mailto:cfe-commits@lists.llvm.org>> wrote:
Author: erichkeane
Date: Mon Sep 30 12:12:29 2019
New Revision: 373247

URL: http://llvm.org/viewvc/llvm-project?rev=373247=rev
Log:
Teach CallGraph to look into Generic Lambdas.

CallGraph visited LambdaExpr by getting the Call Operator from
CXXRecordDecl (LambdaExpr::getCallOperator calls
CXXRecordDecl::getLambdaCallOperator), which replaced generic lambda
call operators with the non-instantiated FunctionDecl.  The result was
that the CallGraph would only pick up non-dependent calls.

This patch does a few things:
1- Extend CXXRecordDecl to have a getDependentLambdaCallOperator, which
will get the FunctionTemplateDecl, rather than immediately getting the
TemplateDecl.
2- Define getLambdaCallOperator and getDependentLambdaCallOperator in
terms of a common function.
3- Extend LambdaExpr with a getDependentCallOperator, which just calls
the above function.
4- Changes CallGraph to handle Generic LambdaExprs.

Modified:
cfe/trunk/include/clang/AST/DeclCXX.h
cfe/trunk/include/clang/AST/ExprCXX.h
cfe/trunk/lib/AST/DeclCXX.cpp
cfe/trunk/lib/AST/ExprCXX.cpp
cfe/trunk/lib/Analysis/CallGraph.cpp
cfe/trunk/test/Analysis/debug-CallGraph.cpp

Modified: cfe/trunk/include/clang/AST/DeclCXX.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclCXX.h?rev=373247=373246=373247=diff
==
--- cfe/trunk/include/clang/AST/DeclCXX.h (original)
+++ cfe/trunk/include/clang/AST/DeclCXX.h Mon Sep 30 12:12:29 2019
@@ -1172,6 +1172,10 @@ public:
   /// if this is a closure type.
   CXXMethodDecl *getLambdaCallOperator() const;

+  /// Retrieve the dependent lambda call operator of the closure type
+  /// if this is a templated closure type.
+  FunctionTemplateDecl *getDependentLambdaCallOperator() const;
+
   /// Retrieve the lambda static invoker, the address of which
   /// is returned by the conversion operator, and the body of which
   /// is forwarded to the lambda call operator.

Modified: cfe/trunk/include/clang/AST/ExprCXX.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ExprCXX.h?rev=373247=373246=373247=diff
==
--- cfe/trunk/include/clang/AST/ExprCXX.h (original)
+++ cfe/trunk/include/clang/AST/ExprCXX.h Mon Sep 30 12:12:29 2019
@@ -1907,6 +1907,10 @@ public:
   /// lambda expression.
   CXXMethodDecl *getCallOperator() const;

+  /// Retrieve the function template call operator associated with this
+  /// lambda expression.
+  FunctionTemplateDecl *getDependentCallOperator() const;
+
   /// If this is a generic lambda expression, retrieve the template
   /// parameter list associated with it, or else return null.
   TemplateParameterList *getTemplateParameterList() const;

Modified: cfe/trunk/lib/AST/DeclCXX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclCXX.cpp?rev=373247=373246=373247=diff
==
--- cfe/trunk/lib/AST/DeclCXX.cpp (original)
+++ cfe/trunk/lib/AST/DeclCXX.cpp Mon Sep 30 12:12:29 2019
@@ -1399,17 +1399,25 @@ static bool allLookupResultsAreTheSame(c
 }
 #endif

-CXXMethodDecl* CXXRecordDecl::getLambdaCallOperator() const {
-  if (!isLambda()) return nullptr;
+NamedDecl* getLambdaCallOperatorHelper(const CXXRecordDecl ) {
+  if (!RD.isLambda()) return nullptr;
   DeclarationName Name =
-getASTContext().DeclarationNames.getCXXOperatorName(OO_Call);
-  DeclContext::lookup_result Calls = lookup(Name);
+RD.getASTContext().DeclarationNames.getCXXOperatorName(OO_Call);
+  DeclContext::lookup_result Calls = RD.lookup(Name);

   assert(!Calls.empty() && "Missing lambda call operator!");
   assert(allLookupResultsAreTheSame(Calls) &&
  "More than one lambda call operator!");
+  return Calls.front();
+}
+
+FunctionTemplateDecl* CXXRecordDecl::getDependentLambdaCallOperator() const {
+  NamedDecl *CallOp = getLambdaCallOperatorHelper(*this);
+  return  dyn_cast(CallOp);
+}

-  NamedDecl *CallOp = Calls.front();
+CXXMethodDecl *CXXRecordDecl::getLambdaCallOperator() const {
+  NamedDecl *CallOp = getLambdaCallOperatorHelper(*this);
   if (const auto *CallOpTmpl = dyn_cast(CallOp))
 return cast(CallOpTmpl->getTemplatedDecl());


Modified: 

r373259 - Fix failure caused by r373247

[Erich Keane via cfe-commits]

Author: erichkeane
Date: Mon Sep 30 13:45:12 2019
New Revision: 373259

URL: http://llvm.org/viewvc/llvm-project?rev=373259=rev
Log:
Fix failure caused by r373247

I incorrectly thought that the 'isLambda' check never fired, so when
splitting up a helper function, I lost the 'nullptr' return value.
ClangD Hover functionality apparently uses this, so the Unittest caught
that.

This patch correctly propogates the nullptr from the helper function.

Modified:
cfe/trunk/lib/AST/DeclCXX.cpp

Modified: cfe/trunk/lib/AST/DeclCXX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclCXX.cpp?rev=373259=373258=373259=diff
==
--- cfe/trunk/lib/AST/DeclCXX.cpp (original)
+++ cfe/trunk/lib/AST/DeclCXX.cpp Mon Sep 30 13:45:12 2019
@@ -1413,11 +1413,15 @@ NamedDecl* getLambdaCallOperatorHelper(c
 
 FunctionTemplateDecl* CXXRecordDecl::getDependentLambdaCallOperator() const {
   NamedDecl *CallOp = getLambdaCallOperatorHelper(*this);
-  return  dyn_cast(CallOp);
+  return  dyn_cast_or_null(CallOp);
 }
 
 CXXMethodDecl *CXXRecordDecl::getLambdaCallOperator() const {
   NamedDecl *CallOp = getLambdaCallOperatorHelper(*this);
+
+  if (CallOp == nullptr)
+return nullptr;
+
   if (const auto *CallOpTmpl = dyn_cast(CallOp))
 return cast(CallOpTmpl->getTemplatedDecl());
 


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

[PATCH] D67079: [analyzer] CastValueChecker: Model inheritance

[Csaba Dabis via Phabricator via cfe-commits]

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



Comment at: clang/test/Analysis/cast-value-hierarchy-fp-suppression.cpp:25-27
+  if (isa(a))
+if (isa(a))
+  clang_analyzer_warnIfReached(); // no-warning

NoQ wrote:
> Charusso wrote:
> > NoQ wrote:
> > > Why is `(isa(a) && isa(a))` deemed possible in the first test but 
> > > not in the second test? o_o
> > In `test_downcast()` we assume that `a` is a record type of `D` where `D` 
> > is a `B` and `D` is a `C`.  However in `test_downcast_infeasible()` if `a` 
> > is not a record type of `D` is cannot be both `B` and `C` at the same time. 
> > That is the purpose of `CastVisitor`.
> I mean, it contradicts to how the program *actually* works, so we should 
> either not do that, or provide a reaallly compelling explanation of 
> why we do this (as in "Extraordinary Claims Require Extraordinary Evidence").
Are you sure it does not model the program? I have an `Apple` class and I have 
a `Pen` class, until it is not defining an `ApplePen` class it is a false 
assumption to say they are defining an `ApplePen` class. I wanted to prefetch 
that information before the modeling starts, but it was an impossible challenge 
for me, so I have picked that `CastVisitor`-based post-elimination idea. In the 
real world I have removed only two false assumptions with the visitor from 1200 
reports of LLVM so an `ApplePen` is very rare 
(https://www.youtube.com/watch?v=Ct6BUPvE2sM).


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

https://reviews.llvm.org/D67079



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

r373258 - [NFC] Fix tests, second try

[David Bolvansky via cfe-commits]

Author: xbolva00
Date: Mon Sep 30 13:41:56 2019
New Revision: 373258

URL: http://llvm.org/viewvc/llvm-project?rev=373258=rev
Log:
[NFC] Fix tests, second try

Modified:
cfe/trunk/test/SemaCXX/warn-sign-conversion.cpp

Modified: cfe/trunk/test/SemaCXX/warn-sign-conversion.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-sign-conversion.cpp?rev=373258=373257=373258=diff
==
--- cfe/trunk/test/SemaCXX/warn-sign-conversion.cpp (original)
+++ cfe/trunk/test/SemaCXX/warn-sign-conversion.cpp Mon Sep 30 13:41:56 2019
@@ -70,15 +70,15 @@ namespace test2 {
 int d1 = 1 ? i : Foo::D; // expected-warning {{operand of ? changes 
signedness: 'test2::Foo::Named4' to 'int'}}
 int d2 = 1 ? Foo::D : i; // expected-warning {{operand of ? changes 
signedness: 'test2::Foo::Named4' to 'int'}}
 int d3 = 1 ? B : Foo::D; // expected-warning {{operand of ? changes 
signedness: 'test2::Foo::Named4' to 'int'}}
-// expected-warning@+1 {{enumeration type mismatch in conditional 
expression ('test2::Named2' and 'test2::Foo::Named4')}}
+// expected-warning@-1 {{enumeration type mismatch in conditional 
expression ('test2::Named2' and 'test2::Foo::Named4')}}
 int d4 = 1 ? Foo::D : B; // expected-warning {{operand of ? changes 
signedness: 'test2::Foo::Named4' to 'int'}}
-// expected-warning@+1 {{enumeration type mismatch in conditional 
expression ('test2::Foo::Named4' and 'test2::Named2')}}
+// expected-warning@-1 {{enumeration type mismatch in conditional 
expression ('test2::Foo::Named4' and 'test2::Named2')}}
 
 int e1 = 1 ? i : E; // expected-warning {{operand of ? changes signedness: 
'test2::Named5' to 'int'}}
 int e2 = 1 ? E : i; // expected-warning {{operand of ? changes signedness: 
'test2::Named5' to 'int'}}
 int e3 = 1 ? E : B; // expected-warning {{operand of ? changes signedness: 
'test2::Named5' to 'int'}}
-// expected-warning@+1 {{enumeration type mismatch in conditional 
expression ('test2::Named5' and 'test2::Named2')}}
+// expected-warning@-1 {{enumeration type mismatch in conditional 
expression ('test2::Named5' and 'test2::Named2')}}
 int e4 = 1 ? B : E; // expected-warning {{operand of ? changes signedness: 
'test2::Named5' to 'int'}}
-// expected-warning@+1 {{enumeration type mismatch in conditional 
expression ('test2::Named2' and 'test2::Named5')}}
+// expected-warning@-1 {{enumeration type mismatch in conditional 
expression ('test2::Named2' and 'test2::Named5')}}
   }
 }


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

r373257 - [OPENMP50]Mark declare variant attribute as inheritable.

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Mon Sep 30 13:39:29 2019
New Revision: 373257

URL: http://llvm.org/viewvc/llvm-project?rev=373257=rev
Log:
[OPENMP50]Mark declare variant attribute as inheritable.

Attribute must be inherited by the redeclarations.

Modified:
cfe/trunk/include/clang/Basic/Attr.td
cfe/trunk/test/OpenMP/declare_variant_ast_print.cpp

Modified: cfe/trunk/include/clang/Basic/Attr.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=373257=373256=373257=diff
==
--- cfe/trunk/include/clang/Basic/Attr.td (original)
+++ cfe/trunk/include/clang/Basic/Attr.td Mon Sep 30 13:39:29 2019
@@ -3281,11 +3281,12 @@ def OMPAllocateDecl : InheritableAttr {
   let Documentation = [Undocumented];
 }
 
-def OMPDeclareVariant : Attr {
+def OMPDeclareVariant : InheritableAttr {
   let Spellings = [Pragma<"omp", "declare variant">];
   let Subjects = SubjectList<[Function]>;
   let SemaHandler = 0;
   let HasCustomParsing = 1;
+  let InheritEvenIfAlreadyPresent = 1;
   let Documentation = [OMPDeclareVariantDocs];
   let Args = [
 ExprArgument<"VariantFuncRef">,

Modified: cfe/trunk/test/OpenMP/declare_variant_ast_print.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/declare_variant_ast_print.cpp?rev=373257=373256=373257=diff
==
--- cfe/trunk/test/OpenMP/declare_variant_ast_print.cpp (original)
+++ cfe/trunk/test/OpenMP/declare_variant_ast_print.cpp Mon Sep 30 13:39:29 2019
@@ -148,6 +148,8 @@ auto fn_deduced1() { return 0; }
 // CHECK-NEXT: #pragma omp declare variant(SpecialFuncs::bar) 
match(implementation={vendor(llvm)})
 // CHECK-NEXT: void foo1() {
 // CHECK-NEXT: }
+// CHECK-NEXT: #pragma omp declare variant(SpecialFuncs::baz) 
match(implementation={vendor(unknown)})
+// CHECK-NEXT: void xxx();
 // CHECK-NEXT: } s;
 struct SpecialFuncs {
   void vd() {}
@@ -162,8 +164,15 @@ struct SpecialFuncs {
 #pragma omp declare variant(SpecialFuncs::bar) 
match(implementation={vendor(ibm)}, implementation={vendor(llvm)})
 #pragma omp declare variant(SpecialFuncs::baz) 
match(implementation={vendor(unknown)})
   void foo1() {}
+#pragma omp declare variant(SpecialFuncs::baz) 
match(implementation={vendor(unknown)})
+  void xxx();
 } s;
 
+// CHECK:  #pragma omp declare variant(SpecialFuncs::baz) 
match(implementation={vendor(unknown)})
+// CHECK-NEXT: void SpecialFuncs::xxx() {
+// CHECK-NEXT: }
+void SpecialFuncs::xxx() {}
+
 // CHECK:  static void static_f_variant() {
 // CHECK-NEXT: }
 static void static_f_variant() {}


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

[PATCH] D68242: [clang-format] [PR42417] clang-format inserts a space after '->' for operator->() overloading

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay created this revision.
MyDeveloperDay added reviewers: klimek, owenpan, byoungyoung.
MyDeveloperDay added a project: clang-format.
Herald added a project: clang.

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

This revision removes the extra space between the opertor-> and the parens ()

class Bug {

  auto operator-> () -> int*;
  auto operator++(int) -> int;

};


Repository:
  rC Clang

https://reviews.llvm.org/D68242

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -4830,6 +4830,10 @@
 
 TEST_F(FormatTest, TrailingReturnType) {
   verifyFormat("auto foo() -> int;\n");
+  // correct trailing return type spacing
+  verifyFormat("auto operator->() -> int;\n");
+  verifyFormat("auto operator++(int) -> int;\n");
+
   verifyFormat("struct S {\n"
"  auto bar() const -> int;\n"
"};");
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1387,7 +1387,9 @@
Style.Language == FormatStyle::LK_Java) {
   Current.Type = TT_LambdaArrow;
 } else if (Current.is(tok::arrow) && AutoFound && Line.MustBeDeclaration &&
-   Current.NestingLevel == 0) {
+   Current.NestingLevel == 0 &&
+   !Current.Previous->is(tok::kw_operator)) {
+  // not auto operator->() -> xxx;
   Current.Type = TT_TrailingReturnArrow;
   TrailingReturnFound = true;
 } else if (Current.is(tok::star) ||


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -4830,6 +4830,10 @@
 
 TEST_F(FormatTest, TrailingReturnType) {
   verifyFormat("auto foo() -> int;\n");
+  // correct trailing return type spacing
+  verifyFormat("auto operator->() -> int;\n");
+  verifyFormat("auto operator++(int) -> int;\n");
+
   verifyFormat("struct S {\n"
"  auto bar() const -> int;\n"
"};");
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1387,7 +1387,9 @@
Style.Language == FormatStyle::LK_Java) {
   Current.Type = TT_LambdaArrow;
 } else if (Current.is(tok::arrow) && AutoFound && Line.MustBeDeclaration &&
-   Current.NestingLevel == 0) {
+   Current.NestingLevel == 0 &&
+   !Current.Previous->is(tok::kw_operator)) {
+  // not auto operator->() -> xxx;
   Current.Type = TT_TrailingReturnArrow;
   TrailingReturnFound = true;
 } else if (Current.is(tok::star) ||
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68185: [Diagnostics] Warn when class implements a copy constructor/copy assignment operator, but missing the copy assignment operator/copy constructor

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

+1 for clang-tidy. GCC's closest to this is -Weffc++ which limits the rule of 3 
warning to only "classes that have dynamic memory allocation" (though I'm not 
sure exactly what conditions it uses to decide that - it doesn't warn on 
anything in the test cases provided in this patch, it does warn if you add a 
dtor to Clz that deletes buf:
rule3.cpp:1:7: warning: ‘class Clz’ has pointer data members [-Weffc++]
 class Clz // expected-warning {{class implements custom copy assignment 
operator but missing custom copy constructor}}

  ^~~

rule3.cpp:1:7: warning:   but does not override ‘Clz(const Clz&)’ [-Weffc++]


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

https://reviews.llvm.org/D68185



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

[PATCH] D68227: [clang-format] [PR43372] - clang-format shows replacements in DOS files when no replacement is needed

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

Through experiments, I notice that both result and the Code contains "\r\n", 
because although we construct the results with "\n" the actual text of the 
include has I believe trailing "\r" or "\r\n", I notice this when I thought one 
fix I tried was to trim the include text.

``result += Imports[Index].Text.trim()```

However, this had the effect of dos2unixing the ```result``` and the include 
section of a windows file

but I take your point about the  "\r" without the "\r\n"

I'll change the code but I believe it will need to be

  if (stripNewlineCRs(result) == stripNewlineCRs(...))

I'll send an updated review


Repository:
  rC Clang

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

https://reviews.llvm.org/D68227



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

r373256 - [NFCI] Updated broken test

[David Bolvansky via cfe-commits]

Author: xbolva00
Date: Mon Sep 30 13:23:22 2019
New Revision: 373256

URL: http://llvm.org/viewvc/llvm-project?rev=373256=rev
Log:
[NFCI] Updated broken test

Modified:
cfe/trunk/test/SemaCXX/warn-sign-conversion.cpp

Modified: cfe/trunk/test/SemaCXX/warn-sign-conversion.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-sign-conversion.cpp?rev=373256=373255=373256=diff
==
--- cfe/trunk/test/SemaCXX/warn-sign-conversion.cpp (original)
+++ cfe/trunk/test/SemaCXX/warn-sign-conversion.cpp Mon Sep 30 13:23:22 2019
@@ -70,11 +70,15 @@ namespace test2 {
 int d1 = 1 ? i : Foo::D; // expected-warning {{operand of ? changes 
signedness: 'test2::Foo::Named4' to 'int'}}
 int d2 = 1 ? Foo::D : i; // expected-warning {{operand of ? changes 
signedness: 'test2::Foo::Named4' to 'int'}}
 int d3 = 1 ? B : Foo::D; // expected-warning {{operand of ? changes 
signedness: 'test2::Foo::Named4' to 'int'}}
+// expected-warning@+1 {{enumeration type mismatch in conditional 
expression ('test2::Named2' and 'test2::Foo::Named4')}}
 int d4 = 1 ? Foo::D : B; // expected-warning {{operand of ? changes 
signedness: 'test2::Foo::Named4' to 'int'}}
+// expected-warning@+1 {{enumeration type mismatch in conditional 
expression ('test2::Foo::Named4' and 'test2::Named2')}}
 
 int e1 = 1 ? i : E; // expected-warning {{operand of ? changes signedness: 
'test2::Named5' to 'int'}}
 int e2 = 1 ? E : i; // expected-warning {{operand of ? changes signedness: 
'test2::Named5' to 'int'}}
 int e3 = 1 ? E : B; // expected-warning {{operand of ? changes signedness: 
'test2::Named5' to 'int'}}
+// expected-warning@+1 {{enumeration type mismatch in conditional 
expression ('test2::Named5' and 'test2::Named2')}}
 int e4 = 1 ? B : E; // expected-warning {{operand of ? changes signedness: 
'test2::Named5' to 'int'}}
+// expected-warning@+1 {{enumeration type mismatch in conditional 
expression ('test2::Named2' and 'test2::Named5')}}
   }
 }


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

[PATCH] D67079: [analyzer] CastValueChecker: Model inheritance

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/test/Analysis/cast-value-hierarchy-fp-suppression.cpp:25-27
+  if (isa(a))
+if (isa(a))
+  clang_analyzer_warnIfReached(); // no-warning

Charusso wrote:
> NoQ wrote:
> > Why is `(isa(a) && isa(a))` deemed possible in the first test but not 
> > in the second test? o_o
> In `test_downcast()` we assume that `a` is a record type of `D` where `D` is 
> a `B` and `D` is a `C`.  However in `test_downcast_infeasible()` if `a` is 
> not a record type of `D` is cannot be both `B` and `C` at the same time. That 
> is the purpose of `CastVisitor`.
I mean, it contradicts to how the program *actually* works, so we should either 
not do that, or provide a reaallly compelling explanation of why we do 
this (as in "Extraordinary Claims Require Extraordinary Evidence").


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

https://reviews.llvm.org/D67079



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

[PATCH] D67079: [analyzer] CastValueChecker: Model inheritance

[Csaba Dabis via Phabricator via cfe-commits]

Charusso marked 2 inline comments as done.
Charusso added a comment.

In D67079#1688648 , @NoQ wrote:

> In D67079#1687577 , @Charusso wrote:
>
> > - `CastVisitor` is the new facility which decides whether the assumptions 
> > are appropriate.
> > - The math is still WIP.
>
>
> You need the math to decide whether delaying the decision to a visitor is the 
> right approach.


Yes, Soon(tm).




Comment at: clang/test/Analysis/cast-value-hierarchy-fp-suppression.cpp:25-27
+  if (isa(a))
+if (isa(a))
+  clang_analyzer_warnIfReached(); // no-warning

NoQ wrote:
> Why is `(isa(a) && isa(a))` deemed possible in the first test but not 
> in the second test? o_o
In `test_downcast()` we assume that `a` is a record type of `D` where `D` is a 
`B` and `D` is a `C`.  However in `test_downcast_infeasible()` if `a` is not a 
record type of `D` is cannot be both `B` and `C` at the same time. That is the 
purpose of `CastVisitor`.


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

https://reviews.llvm.org/D67079



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

[PATCH] D68227: [clang-format] [PR43372] - clang-format shows replacements in DOS files when no replacement is needed

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir added inline comments.



Comment at: clang/lib/Format/Format.cpp:1891
+  if (compareIgnoringCarriageReturns(
+  result, Code.substr(IncludesBeginOffset, IncludesBlockSize)))
 return;

Here `compartIgnoringCarriageReturns` is a bit imprecise: there could be `\r`-s 
not followed by `\n`-s in the source code.
Since we have constructed `result` with newlines as `\n`-s in this code, I'd 
suggest to instead implement a function that replaces all occurrences of `\r\n` 
with `\n` in a string (something like "stripNewlineCRs") and have a comparison 
like `result == stripNewlineCRs(...)` here. That shouldn't be much harder than 
this approach and will be a bit more precise.
I've looked around the Format code for preexisting functionality we can reuse, 
but it seems most of the `\r\n` handling is concerned with outputting the right 
newline encoding and not with reading it.


Repository:
  rC Clang

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

https://reviews.llvm.org/D68227



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

[PATCH] D67079: [analyzer] CastValueChecker: Model inheritance

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

In D67079#1687577 , @Charusso wrote:

> - `CastVisitor` is the new facility which decides whether the assumptions are 
> appropriate.
> - The math is still WIP.


You need the math to decide whether delaying the decision to a visitor is the 
right approach.




Comment at: clang/test/Analysis/cast-value-hierarchy-fp-suppression.cpp:25-27
+  if (isa(a))
+if (isa(a))
+  clang_analyzer_warnIfReached(); // no-warning

Why is `(isa(a) && isa(a))` deemed possible in the first test but not in 
the second test? o_o


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

https://reviews.llvm.org/D67079



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

[PATCH] D68199: [analyzer] DynamicTypeInfo: Simplify the API

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

I actually like the idea, it makes it consistent with other maps. But you'll 
need to clean up memory management here. Given that you can't modify the state 
in `getDynamicTypeInfo()`, i guess you'll have to resort to smart pointers.




Comment at: clang/lib/StaticAnalyzer/Core/DynamicType.cpp:53-57
   if (const auto *TR = dyn_cast(MR))
-return DynamicTypeInfo(TR->getLocationType(), /*CanBeSub=*/false);
+return new DynamicTypeInfo(TR->getLocationType(), /*CanBeSub=*/false);
 
-  if (const auto *SR = dyn_cast(MR)) {
-SymbolRef Sym = SR->getSymbol();
-return DynamicTypeInfo(Sym->getType());
-  }
+  if (const auto *SR = dyn_cast(MR))
+return new DynamicTypeInfo(SR->getSymbol()->getType());

Do i have to do a `delete` manually every time i call this function?



Comment at: clang/lib/StaticAnalyzer/Core/DynamicType.cpp:96
+  return setDynamicTypeInfo(State, MR,
+new DynamicTypeInfo(NewTy, CanBeSubClassed));
 }

Who is responsible for deallocating this memory?


Repository:
  rC Clang

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

https://reviews.llvm.org/D68199



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

[PATCH] D67919: [Diagnostics] Warn if enumeration type mismatch in conditional expression

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rL373252: [Diagnostics] Warn if enumeration type mismatch in 
conditional expression (authored by xbolva00, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D67919?vs=222478=222480#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D67919

Files:
  cfe/trunk/lib/Sema/SemaChecking.cpp
  cfe/trunk/test/Sema/warn-conditional-emum-types-mismatch.c


Index: cfe/trunk/lib/Sema/SemaChecking.cpp
===
--- cfe/trunk/lib/Sema/SemaChecking.cpp
+++ cfe/trunk/lib/Sema/SemaChecking.cpp
@@ -11308,6 +11308,32 @@
   return IL;
 }
 
+static void CheckConditionalWithEnumTypes(Sema , SourceLocation Loc,
+  Expr *LHS, Expr *RHS) {
+  QualType LHSStrippedType = LHS->IgnoreParenImpCasts()->getType();
+  QualType RHSStrippedType = RHS->IgnoreParenImpCasts()->getType();
+
+  const auto *LHSEnumType = LHSStrippedType->getAs();
+  if (!LHSEnumType)
+return;
+  const auto *RHSEnumType = RHSStrippedType->getAs();
+  if (!RHSEnumType)
+return;
+
+  // Ignore anonymous enums.
+  if (!LHSEnumType->getDecl()->hasNameForLinkage())
+return;
+  if (!RHSEnumType->getDecl()->hasNameForLinkage())
+return;
+
+  if (S.Context.hasSameUnqualifiedType(LHSStrippedType, RHSStrippedType))
+return;
+
+  S.Diag(Loc, diag::warn_conditional_mixed_enum_types)
+  << LHSStrippedType << RHSStrippedType << LHS->getSourceRange()
+  << RHS->getSourceRange();
+}
+
 static void DiagnoseIntInBoolContext(Sema , Expr *E) {
   E = E->IgnoreParenImpCasts();
   SourceLocation ExprLoc = E->getExprLoc();
@@ -11799,6 +11825,8 @@
   bool Suspicious = false;
   CheckConditionalOperand(S, E->getTrueExpr(), T, CC, Suspicious);
   CheckConditionalOperand(S, E->getFalseExpr(), T, CC, Suspicious);
+  CheckConditionalWithEnumTypes(S, E->getBeginLoc(), E->getTrueExpr(),
+E->getFalseExpr());
 
   if (T->isBooleanType())
 DiagnoseIntInBoolContext(S, E);
Index: cfe/trunk/test/Sema/warn-conditional-emum-types-mismatch.c
===
--- cfe/trunk/test/Sema/warn-conditional-emum-types-mismatch.c
+++ cfe/trunk/test/Sema/warn-conditional-emum-types-mismatch.c
@@ -0,0 +1,39 @@
+// RUN: %clang_cc1 -x c -fsyntax-only -verify -Wenum-compare %s
+// RUN: %clang_cc1 -x c -fsyntax-only -verify  %s
+// RUN: %clang_cc1 -x c++ -fsyntax-only -verify -Wenum-compare %s
+// RUN: %clang_cc1 -x c++ -fsyntax-only -verify %s
+
+enum ro { A = 0x10 };
+enum rw { B = 0xFF };
+enum { C = 0x1A};
+
+enum {
+  STATUS_SUCCESS,
+  STATUS_FAILURE,
+  MAX_BASE_STATUS_CODE
+};
+
+enum ExtendedStatusCodes {
+  STATUS_SOMETHING_INTERESTING = MAX_BASE_STATUS_CODE + 1000,
+};
+
+
+int get_flag(int cond) {
+  return cond ? A : B; 
+  #ifdef __cplusplus
+  // expected-warning@-2 {{enumeration type mismatch in conditional expression 
('ro' and 'rw')}}
+  #else 
+  // expected-no-diagnostics
+  #endif
+}
+
+// In the following cases we purposefully differ from GCC and dont warn because
+// this code pattern is quite sensitive and we dont want to produce so many 
false positives.
+
+int get_flag_anon_enum(int cond) {
+  return cond ? A : C;
+}
+
+int foo(int c) {
+  return c ? STATUS_SOMETHING_INTERESTING : STATUS_SUCCESS;
+}


Index: cfe/trunk/lib/Sema/SemaChecking.cpp
===
--- cfe/trunk/lib/Sema/SemaChecking.cpp
+++ cfe/trunk/lib/Sema/SemaChecking.cpp
@@ -11308,6 +11308,32 @@
   return IL;
 }
 
+static void CheckConditionalWithEnumTypes(Sema , SourceLocation Loc,
+  Expr *LHS, Expr *RHS) {
+  QualType LHSStrippedType = LHS->IgnoreParenImpCasts()->getType();
+  QualType RHSStrippedType = RHS->IgnoreParenImpCasts()->getType();
+
+  const auto *LHSEnumType = LHSStrippedType->getAs();
+  if (!LHSEnumType)
+return;
+  const auto *RHSEnumType = RHSStrippedType->getAs();
+  if (!RHSEnumType)
+return;
+
+  // Ignore anonymous enums.
+  if (!LHSEnumType->getDecl()->hasNameForLinkage())
+return;
+  if (!RHSEnumType->getDecl()->hasNameForLinkage())
+return;
+
+  if (S.Context.hasSameUnqualifiedType(LHSStrippedType, RHSStrippedType))
+return;
+
+  S.Diag(Loc, diag::warn_conditional_mixed_enum_types)
+  << LHSStrippedType << RHSStrippedType << LHS->getSourceRange()
+  << RHS->getSourceRange();
+}
+
 static void DiagnoseIntInBoolContext(Sema , Expr *E) {
   E = E->IgnoreParenImpCasts();
   SourceLocation ExprLoc = E->getExprLoc();
@@ -11799,6 +11825,8 @@
   bool Suspicious = false;
   CheckConditionalOperand(S, E->getTrueExpr(), T, CC, Suspicious);
   CheckConditionalOperand(S, E->getFalseExpr(), T, CC, Suspicious);
+  

[PATCH] D67919: [Diagnostics] Warn if enumeration type mismatch in conditional expression

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

xbolva00 updated this revision to Diff 222478.
xbolva00 marked an inline comment as done.
xbolva00 added a comment.

Added comment why we differ


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

https://reviews.llvm.org/D67919

Files:
  lib/Sema/SemaChecking.cpp
  test/Sema/warn-conditional-emum-types-mismatch.c


Index: test/Sema/warn-conditional-emum-types-mismatch.c
===
--- test/Sema/warn-conditional-emum-types-mismatch.c
+++ test/Sema/warn-conditional-emum-types-mismatch.c
@@ -0,0 +1,39 @@
+// RUN: %clang_cc1 -x c -fsyntax-only -verify -Wenum-compare %s
+// RUN: %clang_cc1 -x c -fsyntax-only -verify  %s
+// RUN: %clang_cc1 -x c++ -fsyntax-only -verify -Wenum-compare %s
+// RUN: %clang_cc1 -x c++ -fsyntax-only -verify %s
+
+enum ro { A = 0x10 };
+enum rw { B = 0xFF };
+enum { C = 0x1A};
+
+enum {
+  STATUS_SUCCESS,
+  STATUS_FAILURE,
+  MAX_BASE_STATUS_CODE
+};
+
+enum ExtendedStatusCodes {
+  STATUS_SOMETHING_INTERESTING = MAX_BASE_STATUS_CODE + 1000,
+};
+
+
+int get_flag(int cond) {
+  return cond ? A : B; 
+  #ifdef __cplusplus
+  // expected-warning@-2 {{enumeration type mismatch in conditional expression 
('ro' and 'rw')}}
+  #else 
+  // expected-no-diagnostics
+  #endif
+}
+
+// In the following cases we purposefully differ from GCC and dont warn because
+// this code pattern is quite sensitive and we dont want to produce so many 
false positives.
+
+int get_flag_anon_enum(int cond) {
+  return cond ? A : C;
+}
+
+int foo(int c) {
+  return c ? STATUS_SOMETHING_INTERESTING : STATUS_SUCCESS;
+}
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -11308,6 +11308,32 @@
   return IL;
 }
 
+static void CheckConditionalWithEnumTypes(Sema , SourceLocation Loc,
+  Expr *LHS, Expr *RHS) {
+  QualType LHSStrippedType = LHS->IgnoreParenImpCasts()->getType();
+  QualType RHSStrippedType = RHS->IgnoreParenImpCasts()->getType();
+
+  const auto *LHSEnumType = LHSStrippedType->getAs();
+  if (!LHSEnumType)
+return;
+  const auto *RHSEnumType = RHSStrippedType->getAs();
+  if (!RHSEnumType)
+return;
+
+  // Ignore anonymous enums.
+  if (!LHSEnumType->getDecl()->hasNameForLinkage())
+return;
+  if (!RHSEnumType->getDecl()->hasNameForLinkage())
+return;
+
+  if (S.Context.hasSameUnqualifiedType(LHSStrippedType, RHSStrippedType))
+return;
+
+  S.Diag(Loc, diag::warn_conditional_mixed_enum_types)
+  << LHSStrippedType << RHSStrippedType << LHS->getSourceRange()
+  << RHS->getSourceRange();
+}
+
 static void DiagnoseIntInBoolContext(Sema , Expr *E) {
   E = E->IgnoreParenImpCasts();
   SourceLocation ExprLoc = E->getExprLoc();
@@ -11799,6 +11825,8 @@
   bool Suspicious = false;
   CheckConditionalOperand(S, E->getTrueExpr(), T, CC, Suspicious);
   CheckConditionalOperand(S, E->getFalseExpr(), T, CC, Suspicious);
+  CheckConditionalWithEnumTypes(S, E->getBeginLoc(), E->getTrueExpr(),
+E->getFalseExpr());
 
   if (T->isBooleanType())
 DiagnoseIntInBoolContext(S, E);


Index: test/Sema/warn-conditional-emum-types-mismatch.c
===
--- test/Sema/warn-conditional-emum-types-mismatch.c
+++ test/Sema/warn-conditional-emum-types-mismatch.c
@@ -0,0 +1,39 @@
+// RUN: %clang_cc1 -x c -fsyntax-only -verify -Wenum-compare %s
+// RUN: %clang_cc1 -x c -fsyntax-only -verify  %s
+// RUN: %clang_cc1 -x c++ -fsyntax-only -verify -Wenum-compare %s
+// RUN: %clang_cc1 -x c++ -fsyntax-only -verify %s
+
+enum ro { A = 0x10 };
+enum rw { B = 0xFF };
+enum { C = 0x1A};
+
+enum {
+  STATUS_SUCCESS,
+  STATUS_FAILURE,
+  MAX_BASE_STATUS_CODE
+};
+
+enum ExtendedStatusCodes {
+  STATUS_SOMETHING_INTERESTING = MAX_BASE_STATUS_CODE + 1000,
+};
+
+
+int get_flag(int cond) {
+  return cond ? A : B; 
+  #ifdef __cplusplus
+  // expected-warning@-2 {{enumeration type mismatch in conditional expression ('ro' and 'rw')}}
+  #else 
+  // expected-no-diagnostics
+  #endif
+}
+
+// In the following cases we purposefully differ from GCC and dont warn because
+// this code pattern is quite sensitive and we dont want to produce so many false positives.
+
+int get_flag_anon_enum(int cond) {
+  return cond ? A : C;
+}
+
+int foo(int c) {
+  return c ? STATUS_SOMETHING_INTERESTING : STATUS_SUCCESS;
+}
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -11308,6 +11308,32 @@
   return IL;
 }
 
+static void CheckConditionalWithEnumTypes(Sema , SourceLocation Loc,
+  Expr *LHS, Expr *RHS) {
+  QualType LHSStrippedType = LHS->IgnoreParenImpCasts()->getType();
+  QualType RHSStrippedType = RHS->IgnoreParenImpCasts()->getType();
+
+  const auto *LHSEnumType = 

r373252 - [Diagnostics] Warn if enumeration type mismatch in conditional expression

[David Bolvansky via cfe-commits]

Author: xbolva00
Date: Mon Sep 30 12:55:50 2019
New Revision: 373252

URL: http://llvm.org/viewvc/llvm-project?rev=373252=rev
Log:
[Diagnostics] Warn if enumeration type mismatch in conditional expression

Summary:
- Useful warning
- GCC compatibility (GCC warns in C++ mode)

Reviewers: rsmith, aaron.ballman

Reviewed By: aaron.ballman

Subscribers: cfe-commits

Tags: #clang

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

Added:
cfe/trunk/test/Sema/warn-conditional-emum-types-mismatch.c
Modified:
cfe/trunk/lib/Sema/SemaChecking.cpp

Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=373252=373251=373252=diff
==
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Mon Sep 30 12:55:50 2019
@@ -11308,6 +11308,32 @@ static const IntegerLiteral *getIntegerL
   return IL;
 }
 
+static void CheckConditionalWithEnumTypes(Sema , SourceLocation Loc,
+  Expr *LHS, Expr *RHS) {
+  QualType LHSStrippedType = LHS->IgnoreParenImpCasts()->getType();
+  QualType RHSStrippedType = RHS->IgnoreParenImpCasts()->getType();
+
+  const auto *LHSEnumType = LHSStrippedType->getAs();
+  if (!LHSEnumType)
+return;
+  const auto *RHSEnumType = RHSStrippedType->getAs();
+  if (!RHSEnumType)
+return;
+
+  // Ignore anonymous enums.
+  if (!LHSEnumType->getDecl()->hasNameForLinkage())
+return;
+  if (!RHSEnumType->getDecl()->hasNameForLinkage())
+return;
+
+  if (S.Context.hasSameUnqualifiedType(LHSStrippedType, RHSStrippedType))
+return;
+
+  S.Diag(Loc, diag::warn_conditional_mixed_enum_types)
+  << LHSStrippedType << RHSStrippedType << LHS->getSourceRange()
+  << RHS->getSourceRange();
+}
+
 static void DiagnoseIntInBoolContext(Sema , Expr *E) {
   E = E->IgnoreParenImpCasts();
   SourceLocation ExprLoc = E->getExprLoc();
@@ -11799,6 +11825,8 @@ static void CheckConditionalOperator(Sem
   bool Suspicious = false;
   CheckConditionalOperand(S, E->getTrueExpr(), T, CC, Suspicious);
   CheckConditionalOperand(S, E->getFalseExpr(), T, CC, Suspicious);
+  CheckConditionalWithEnumTypes(S, E->getBeginLoc(), E->getTrueExpr(),
+E->getFalseExpr());
 
   if (T->isBooleanType())
 DiagnoseIntInBoolContext(S, E);

Added: cfe/trunk/test/Sema/warn-conditional-emum-types-mismatch.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/warn-conditional-emum-types-mismatch.c?rev=373252=auto
==
--- cfe/trunk/test/Sema/warn-conditional-emum-types-mismatch.c (added)
+++ cfe/trunk/test/Sema/warn-conditional-emum-types-mismatch.c Mon Sep 30 
12:55:50 2019
@@ -0,0 +1,39 @@
+// RUN: %clang_cc1 -x c -fsyntax-only -verify -Wenum-compare %s
+// RUN: %clang_cc1 -x c -fsyntax-only -verify  %s
+// RUN: %clang_cc1 -x c++ -fsyntax-only -verify -Wenum-compare %s
+// RUN: %clang_cc1 -x c++ -fsyntax-only -verify %s
+
+enum ro { A = 0x10 };
+enum rw { B = 0xFF };
+enum { C = 0x1A};
+
+enum {
+  STATUS_SUCCESS,
+  STATUS_FAILURE,
+  MAX_BASE_STATUS_CODE
+};
+
+enum ExtendedStatusCodes {
+  STATUS_SOMETHING_INTERESTING = MAX_BASE_STATUS_CODE + 1000,
+};
+
+
+int get_flag(int cond) {
+  return cond ? A : B; 
+  #ifdef __cplusplus
+  // expected-warning@-2 {{enumeration type mismatch in conditional expression 
('ro' and 'rw')}}
+  #else 
+  // expected-no-diagnostics
+  #endif
+}
+
+// In the following cases we purposefully differ from GCC and dont warn because
+// this code pattern is quite sensitive and we dont want to produce so many 
false positives.
+
+int get_flag_anon_enum(int cond) {
+  return cond ? A : C;
+}
+
+int foo(int c) {
+  return c ? STATUS_SOMETHING_INTERESTING : STATUS_SUCCESS;
+}


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

RE: r373247 - Teach CallGraph to look into Generic Lambdas.

[Keane, Erich via cfe-commits]

I saw that, thanks!  I’m looking into them now.  I can revert if it takes me 
too long.

From: Nico Weber 
Sent: Monday, September 30, 2019 12:50 PM
To: Keane, Erich 
Cc: cfe-commits 
Subject: Re: r373247 - Teach CallGraph to look into Generic Lambdas.

This broke a few clangd unit tests:

http://lab.llvm.org:8011/builders/clang-ppc64be-linux/builds/38857/steps/ninja%20check%201/logs/FAIL%3A%20Clangd%20Unit%20Tests%3A%3AHover.Structured
http://lab.llvm.org:8011/builders/clang-ppc64be-linux/builds/38857/steps/ninja%20check%201/logs/FAIL%3A%20Clangd%20Unit%20Tests%3A%3AHover.All
http://45.33.8.238/linux/680/step_7.txt

On Mon, Sep 30, 2019 at 3:10 PM Erich Keane via cfe-commits 
mailto:cfe-commits@lists.llvm.org>> wrote:
Author: erichkeane
Date: Mon Sep 30 12:12:29 2019
New Revision: 373247

URL: http://llvm.org/viewvc/llvm-project?rev=373247=rev
Log:
Teach CallGraph to look into Generic Lambdas.

CallGraph visited LambdaExpr by getting the Call Operator from
CXXRecordDecl (LambdaExpr::getCallOperator calls
CXXRecordDecl::getLambdaCallOperator), which replaced generic lambda
call operators with the non-instantiated FunctionDecl.  The result was
that the CallGraph would only pick up non-dependent calls.

This patch does a few things:
1- Extend CXXRecordDecl to have a getDependentLambdaCallOperator, which
will get the FunctionTemplateDecl, rather than immediately getting the
TemplateDecl.
2- Define getLambdaCallOperator and getDependentLambdaCallOperator in
terms of a common function.
3- Extend LambdaExpr with a getDependentCallOperator, which just calls
the above function.
4- Changes CallGraph to handle Generic LambdaExprs.

Modified:
cfe/trunk/include/clang/AST/DeclCXX.h
cfe/trunk/include/clang/AST/ExprCXX.h
cfe/trunk/lib/AST/DeclCXX.cpp
cfe/trunk/lib/AST/ExprCXX.cpp
cfe/trunk/lib/Analysis/CallGraph.cpp
cfe/trunk/test/Analysis/debug-CallGraph.cpp

Modified: cfe/trunk/include/clang/AST/DeclCXX.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclCXX.h?rev=373247=373246=373247=diff
==
--- cfe/trunk/include/clang/AST/DeclCXX.h (original)
+++ cfe/trunk/include/clang/AST/DeclCXX.h Mon Sep 30 12:12:29 2019
@@ -1172,6 +1172,10 @@ public:
   /// if this is a closure type.
   CXXMethodDecl *getLambdaCallOperator() const;

+  /// Retrieve the dependent lambda call operator of the closure type
+  /// if this is a templated closure type.
+  FunctionTemplateDecl *getDependentLambdaCallOperator() const;
+
   /// Retrieve the lambda static invoker, the address of which
   /// is returned by the conversion operator, and the body of which
   /// is forwarded to the lambda call operator.

Modified: cfe/trunk/include/clang/AST/ExprCXX.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ExprCXX.h?rev=373247=373246=373247=diff
==
--- cfe/trunk/include/clang/AST/ExprCXX.h (original)
+++ cfe/trunk/include/clang/AST/ExprCXX.h Mon Sep 30 12:12:29 2019
@@ -1907,6 +1907,10 @@ public:
   /// lambda expression.
   CXXMethodDecl *getCallOperator() const;

+  /// Retrieve the function template call operator associated with this
+  /// lambda expression.
+  FunctionTemplateDecl *getDependentCallOperator() const;
+
   /// If this is a generic lambda expression, retrieve the template
   /// parameter list associated with it, or else return null.
   TemplateParameterList *getTemplateParameterList() const;

Modified: cfe/trunk/lib/AST/DeclCXX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclCXX.cpp?rev=373247=373246=373247=diff
==
--- cfe/trunk/lib/AST/DeclCXX.cpp (original)
+++ cfe/trunk/lib/AST/DeclCXX.cpp Mon Sep 30 12:12:29 2019
@@ -1399,17 +1399,25 @@ static bool allLookupResultsAreTheSame(c
 }
 #endif

-CXXMethodDecl* CXXRecordDecl::getLambdaCallOperator() const {
-  if (!isLambda()) return nullptr;
+NamedDecl* getLambdaCallOperatorHelper(const CXXRecordDecl ) {
+  if (!RD.isLambda()) return nullptr;
   DeclarationName Name =
-getASTContext().DeclarationNames.getCXXOperatorName(OO_Call);
-  DeclContext::lookup_result Calls = lookup(Name);
+RD.getASTContext().DeclarationNames.getCXXOperatorName(OO_Call);
+  DeclContext::lookup_result Calls = RD.lookup(Name);

   assert(!Calls.empty() && "Missing lambda call operator!");
   assert(allLookupResultsAreTheSame(Calls) &&
  "More than one lambda call operator!");
+  return Calls.front();
+}
+
+FunctionTemplateDecl* CXXRecordDecl::getDependentLambdaCallOperator() const {
+  NamedDecl *CallOp = getLambdaCallOperatorHelper(*this);
+  return  dyn_cast(CallOp);
+}

-  NamedDecl *CallOp = Calls.front();
+CXXMethodDecl *CXXRecordDecl::getLambdaCallOperator() const {
+  NamedDecl *CallOp = getLambdaCallOperatorHelper(*this);
   if (const auto *CallOpTmpl = dyn_cast(CallOp))

Re: r373247 - Teach CallGraph to look into Generic Lambdas.

[Nico Weber via cfe-commits]

This broke a few clangd unit tests:

http://lab.llvm.org:8011/builders/clang-ppc64be-linux/builds/38857/steps/ninja%20check%201/logs/FAIL%3A%20Clangd%20Unit%20Tests%3A%3AHover.Structured
http://lab.llvm.org:8011/builders/clang-ppc64be-linux/builds/38857/steps/ninja%20check%201/logs/FAIL%3A%20Clangd%20Unit%20Tests%3A%3AHover.All
http://45.33.8.238/linux/680/step_7.txt

On Mon, Sep 30, 2019 at 3:10 PM Erich Keane via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: erichkeane
> Date: Mon Sep 30 12:12:29 2019
> New Revision: 373247
>
> URL: http://llvm.org/viewvc/llvm-project?rev=373247=rev
> Log:
> Teach CallGraph to look into Generic Lambdas.
>
> CallGraph visited LambdaExpr by getting the Call Operator from
> CXXRecordDecl (LambdaExpr::getCallOperator calls
> CXXRecordDecl::getLambdaCallOperator), which replaced generic lambda
> call operators with the non-instantiated FunctionDecl.  The result was
> that the CallGraph would only pick up non-dependent calls.
>
> This patch does a few things:
> 1- Extend CXXRecordDecl to have a getDependentLambdaCallOperator, which
> will get the FunctionTemplateDecl, rather than immediately getting the
> TemplateDecl.
> 2- Define getLambdaCallOperator and getDependentLambdaCallOperator in
> terms of a common function.
> 3- Extend LambdaExpr with a getDependentCallOperator, which just calls
> the above function.
> 4- Changes CallGraph to handle Generic LambdaExprs.
>
> Modified:
> cfe/trunk/include/clang/AST/DeclCXX.h
> cfe/trunk/include/clang/AST/ExprCXX.h
> cfe/trunk/lib/AST/DeclCXX.cpp
> cfe/trunk/lib/AST/ExprCXX.cpp
> cfe/trunk/lib/Analysis/CallGraph.cpp
> cfe/trunk/test/Analysis/debug-CallGraph.cpp
>
> Modified: cfe/trunk/include/clang/AST/DeclCXX.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclCXX.h?rev=373247=373246=373247=diff
>
> ==
> --- cfe/trunk/include/clang/AST/DeclCXX.h (original)
> +++ cfe/trunk/include/clang/AST/DeclCXX.h Mon Sep 30 12:12:29 2019
> @@ -1172,6 +1172,10 @@ public:
>/// if this is a closure type.
>CXXMethodDecl *getLambdaCallOperator() const;
>
> +  /// Retrieve the dependent lambda call operator of the closure type
> +  /// if this is a templated closure type.
> +  FunctionTemplateDecl *getDependentLambdaCallOperator() const;
> +
>/// Retrieve the lambda static invoker, the address of which
>/// is returned by the conversion operator, and the body of which
>/// is forwarded to the lambda call operator.
>
> Modified: cfe/trunk/include/clang/AST/ExprCXX.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ExprCXX.h?rev=373247=373246=373247=diff
>
> ==
> --- cfe/trunk/include/clang/AST/ExprCXX.h (original)
> +++ cfe/trunk/include/clang/AST/ExprCXX.h Mon Sep 30 12:12:29 2019
> @@ -1907,6 +1907,10 @@ public:
>/// lambda expression.
>CXXMethodDecl *getCallOperator() const;
>
> +  /// Retrieve the function template call operator associated with this
> +  /// lambda expression.
> +  FunctionTemplateDecl *getDependentCallOperator() const;
> +
>/// If this is a generic lambda expression, retrieve the template
>/// parameter list associated with it, or else return null.
>TemplateParameterList *getTemplateParameterList() const;
>
> Modified: cfe/trunk/lib/AST/DeclCXX.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclCXX.cpp?rev=373247=373246=373247=diff
>
> ==
> --- cfe/trunk/lib/AST/DeclCXX.cpp (original)
> +++ cfe/trunk/lib/AST/DeclCXX.cpp Mon Sep 30 12:12:29 2019
> @@ -1399,17 +1399,25 @@ static bool allLookupResultsAreTheSame(c
>  }
>  #endif
>
> -CXXMethodDecl* CXXRecordDecl::getLambdaCallOperator() const {
> -  if (!isLambda()) return nullptr;
> +NamedDecl* getLambdaCallOperatorHelper(const CXXRecordDecl ) {
> +  if (!RD.isLambda()) return nullptr;
>DeclarationName Name =
> -getASTContext().DeclarationNames.getCXXOperatorName(OO_Call);
> -  DeclContext::lookup_result Calls = lookup(Name);
> +RD.getASTContext().DeclarationNames.getCXXOperatorName(OO_Call);
> +  DeclContext::lookup_result Calls = RD.lookup(Name);
>
>assert(!Calls.empty() && "Missing lambda call operator!");
>assert(allLookupResultsAreTheSame(Calls) &&
>   "More than one lambda call operator!");
> +  return Calls.front();
> +}
> +
> +FunctionTemplateDecl* CXXRecordDecl::getDependentLambdaCallOperator()
> const {
> +  NamedDecl *CallOp = getLambdaCallOperatorHelper(*this);
> +  return  dyn_cast(CallOp);
> +}
>
> -  NamedDecl *CallOp = Calls.front();
> +CXXMethodDecl *CXXRecordDecl::getLambdaCallOperator() const {
> +  NamedDecl *CallOp = getLambdaCallOperatorHelper(*this);
>if (const auto *CallOpTmpl = dyn_cast(CallOp))
>  return cast(CallOpTmpl->getTemplatedDecl());
>
>
> 

[PATCH] D64671: [clang-tidy] New check: misc-init-local-variables

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Do you need someone to commit this on your behalf (sorry for not asking that 
question sooner)?


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

https://reviews.llvm.org/D64671



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

[PATCH] D67706: [clang][analyzer] Using CallDescription in StreamChecker.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:90-106
+  FnCheck identifyCall(const CallEvent , CheckerContext ,
+   const CallExpr *CE) const;
+
+  void evalFopen(CheckerContext , const CallExpr *CE) const;
+  void evalTmpfile(CheckerContext , const CallExpr *CE) const;
+  void evalFclose(CheckerContext , const CallExpr *CE) const;
+  void evalFread(CheckerContext , const CallExpr *CE) const;

balazske wrote:
> NoQ wrote:
> > For most purposes it's more convenient to pass `CallEvent` around. The only 
> > moment when you actually need the `CallExpr` is when you're doing the 
> > binding for the return value, which happens once, so it's fine to call 
> > `.getOriginExpr()` directly at this point.
> The CallExpr is used at many places to get arguments and other data. There is 
> a `CallEvent::getArgSVal` that can be used instead but at some places 
> CallExpr is used in other ways. I do not see much benefit of change the 
> passed `CallExpr` to `CallEvent`.
> at some places CallExpr is used in other ways

Can we add more convenient getters to `CallEvent` to help with those? 'Cause 
`CallEvent` has strictly more information than `CallExpr` (combining syntactic 
information with path-sensitive information), it's all about convenience.

I also see that `CallExpr` is stored in `StreamState` (which is something you 
can't do with a `CallEvent` because it's short-lived) but i suspect that it's 
actually never read from there and i doubt that it was the right solution 
anyway. I.e., no other checker needs that and i don't have a reason to believe 
that `StreamChecker` is special.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67706



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

[PATCH] D67545: [clang-tidy] Added DefaultOperatorNewCheck.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.
Herald added a subscriber: hiraditya.



Comment at: clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp:68-70
+// MEM
+CheckFactories.registerCheck(
+"cert-mem57-cpp");

The `MEM` section should come before `MSC`.



Comment at: 
clang-tools-extra/clang-tidy/cert/DefaultOperatorNewAlignmentCheck.cpp:24
+void DefaultOperatorNewAlignmentCheck::check(
+const MatchFinder::MatchResult ) {
+  // Get the found 'new' expression.

Didn't we decide the check should not diagnose in C++17 or later?



Comment at: 
clang-tools-extra/clang-tidy/cert/DefaultOperatorNewAlignmentCheck.cpp:29-30
+  // Skip placement new: This is a user-defined allocation.
+  if (NewExpr->getNumPlacementArgs() > 0)
+return;
+  QualType T = NewExpr->getAllocatedType();

I think this should be hoisted into a local AST matcher rather than be part of 
the `check()` call. Something like `unless(isPlacementNew())` when registering 
the check.



Comment at: 
clang-tools-extra/clang-tidy/cert/DefaultOperatorNewAlignmentCheck.cpp:47
+  // The user-specified alignment (in bits).
+  const unsigned SpecifiedAlignment = D->getMaxAlignment();
+  // Double-check if no alignment was specified.

Please drop top-level `const` on anything that's not a pointer or a reference 
(that's not a style we typically follow).



Comment at: 
clang-tools-extra/clang-tidy/cert/DefaultOperatorNewAlignmentCheck.cpp:59
+  const unsigned CharWidth = Context.getTargetInfo().getCharWidth();
+  if (HasDefaultOperatorNew && OverAligned /*&& !NewExpr->passAlignment()*/)
+diag(NewExpr->getBeginLoc(),

Commented-out code?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67545



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

[PATCH] D67919: [Diagnostics] Warn if enumeration type mismatch in conditional expression

[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 aside from a commenting request.




Comment at: test/Sema/warn-conditional-emum-types-mismatch.c:19
+
+int get_flag_anon_enum(int cond) {
+  return cond ? A : C;

xbolva00 wrote:
> aaron.ballman wrote:
> > xbolva00 wrote:
> > > Gcc warns here, but Clang does not warn when A != C..
> > > 
> > > So not sure here..
> > My gut reaction is that I think Clang should warn here as well because the 
> > code pattern is confusing, but I'd also say that if there's a lot of false 
> > positives where the code is sensible, it may make sense to suppress the 
> > diagnostic. One situation I was thinking of where you could run into 
> > something like this is:
> > ```
> > enum {
> >   STATUS_SUCCESS,
> >   STATUS_FAILURE,
> >   ...
> >   MAX_BASE_STATUS_CODE
> > };
> > 
> > enum ExtendedStatusCodes {
> >   STATUS_SOMETHING_INTERESTING = MAX_BASE_STATUS_CODE + 1000,
> >   ...
> > };
> > 
> > int whatever(void) {
> >   return some_condition() ? STATUS_SOMETHING_INTERESTING : STATUS_SUCCESS;
> > }
> > ```
> +1
Can you add some comments to the test case explaining that we purposefully 
differ from GCC here and why?


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

https://reviews.llvm.org/D67919



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

[PATCH] D68193: In openFileForRead don't cache erroneous entries if the error relates to them being directories. Add tests.

[Kousik Kumar via Phabricator via cfe-commits]

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



Comment at: 
clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp:236-237
 if (!CacheEntry.isValid()) {
+  llvm::vfs::FileSystem  = getUnderlyingFS();
+  auto MaybeStatus = FS.status(Filename);
+

dexonsmith wrote:
> It seems wasteful to do an extra stat here when the file is already open in 
> `createFileEntry`.
> 
> Can you instead change `createFileEntry` to return 
> `std::errc::is_a_directory` as appropriate to avoid the extra filesystem 
> access?  (Is it possible that it's already doing that, and you just need to 
> check for that?)
Thanks for the suggestion, done.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68193



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

[PATCH] D68193: In openFileForRead don't cache erroneous entries if the error relates to them being directories. Add tests.

[Kousik Kumar via Phabricator via cfe-commits]

kousikk updated this revision to Diff 222474.
kousikk added a comment.

Avoid the extra stat() call


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68193

Files:
  clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
  clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp


Index: clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
===
--- clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
+++ clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
@@ -25,6 +25,8 @@
   llvm::ErrorOr Stat = (*MaybeFile)->status();
   if (!Stat)
 return Stat.getError();
+  if (Stat->isDirectory())
+return std::make_error_code(std::errc::is_a_directory);
 
   llvm::vfs::File  = **MaybeFile;
   llvm::ErrorOr> MaybeBuffer =
@@ -233,15 +235,14 @@
 CachedFileSystemEntry  = SharedCacheEntry.Value;
 
 if (!CacheEntry.isValid()) {
-  llvm::vfs::FileSystem  = getUnderlyingFS();
-  auto MaybeStatus = FS.status(Filename);
-
-  if (MaybeStatus && MaybeStatus->isDirectory())
-return llvm::ErrorOr>(
-  std::make_error_code(std::errc::is_a_directory));
-
   CacheEntry = CachedFileSystemEntry::createFileEntry(
-  Filename, FS, !KeepOriginalSource);
+Filename, getUnderlyingFS(), !KeepOriginalSource);
+  if (CacheEntry.getError() == std::errc::is_a_directory) {
+// Reset the CacheEntry to avoid setting an error entry in the
+// cache for the given directory path.
+CacheEntry = CachedFileSystemEntry();
+return 
llvm::ErrorOr>(std::errc::is_a_directory);
+  }
 }
 
 Result = 
Index: 
clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
===
--- 
clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
+++ 
clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
@@ -80,6 +80,11 @@
 return MaybeStat->getName();
   }
 
+  std::error_code getError() const {
+assert(isValid() && "not initialized");
+return MaybeStat.getError();
+  }
+
   /// Return the mapping between location -> distance that is used to speed up
   /// the block skipping in the preprocessor.
   const PreprocessorSkippedRangeMapping () const {


Index: clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
===
--- clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
+++ clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
@@ -25,6 +25,8 @@
   llvm::ErrorOr Stat = (*MaybeFile)->status();
   if (!Stat)
 return Stat.getError();
+  if (Stat->isDirectory())
+return std::make_error_code(std::errc::is_a_directory);
 
   llvm::vfs::File  = **MaybeFile;
   llvm::ErrorOr> MaybeBuffer =
@@ -233,15 +235,14 @@
 CachedFileSystemEntry  = SharedCacheEntry.Value;
 
 if (!CacheEntry.isValid()) {
-  llvm::vfs::FileSystem  = getUnderlyingFS();
-  auto MaybeStatus = FS.status(Filename);
-
-  if (MaybeStatus && MaybeStatus->isDirectory())
-return llvm::ErrorOr>(
-  std::make_error_code(std::errc::is_a_directory));
-
   CacheEntry = CachedFileSystemEntry::createFileEntry(
-  Filename, FS, !KeepOriginalSource);
+Filename, getUnderlyingFS(), !KeepOriginalSource);
+  if (CacheEntry.getError() == std::errc::is_a_directory) {
+// Reset the CacheEntry to avoid setting an error entry in the
+// cache for the given directory path.
+CacheEntry = CachedFileSystemEntry();
+return llvm::ErrorOr>(std::errc::is_a_directory);
+  }
 }
 
 Result = 
Index: clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
===
--- clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
+++ clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
@@ -80,6 +80,11 @@
 return MaybeStat->getName();
   }
 
+  std::error_code getError() const {
+assert(isValid() && "not initialized");
+return MaybeStat.getError();
+  }
+
   /// Return the mapping between location -> distance that is used to speed up
   /// the block skipping in the preprocessor.
   const PreprocessorSkippedRangeMapping () const {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r373247 - Teach CallGraph to look into Generic Lambdas.

[Erich Keane via cfe-commits]

Author: erichkeane
Date: Mon Sep 30 12:12:29 2019
New Revision: 373247

URL: http://llvm.org/viewvc/llvm-project?rev=373247=rev
Log:
Teach CallGraph to look into Generic Lambdas.

CallGraph visited LambdaExpr by getting the Call Operator from
CXXRecordDecl (LambdaExpr::getCallOperator calls
CXXRecordDecl::getLambdaCallOperator), which replaced generic lambda
call operators with the non-instantiated FunctionDecl.  The result was
that the CallGraph would only pick up non-dependent calls.

This patch does a few things:
1- Extend CXXRecordDecl to have a getDependentLambdaCallOperator, which
will get the FunctionTemplateDecl, rather than immediately getting the
TemplateDecl.
2- Define getLambdaCallOperator and getDependentLambdaCallOperator in
terms of a common function.
3- Extend LambdaExpr with a getDependentCallOperator, which just calls
the above function.
4- Changes CallGraph to handle Generic LambdaExprs.

Modified:
cfe/trunk/include/clang/AST/DeclCXX.h
cfe/trunk/include/clang/AST/ExprCXX.h
cfe/trunk/lib/AST/DeclCXX.cpp
cfe/trunk/lib/AST/ExprCXX.cpp
cfe/trunk/lib/Analysis/CallGraph.cpp
cfe/trunk/test/Analysis/debug-CallGraph.cpp

Modified: cfe/trunk/include/clang/AST/DeclCXX.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclCXX.h?rev=373247=373246=373247=diff
==
--- cfe/trunk/include/clang/AST/DeclCXX.h (original)
+++ cfe/trunk/include/clang/AST/DeclCXX.h Mon Sep 30 12:12:29 2019
@@ -1172,6 +1172,10 @@ public:
   /// if this is a closure type.
   CXXMethodDecl *getLambdaCallOperator() const;
 
+  /// Retrieve the dependent lambda call operator of the closure type
+  /// if this is a templated closure type.
+  FunctionTemplateDecl *getDependentLambdaCallOperator() const;
+
   /// Retrieve the lambda static invoker, the address of which
   /// is returned by the conversion operator, and the body of which
   /// is forwarded to the lambda call operator.

Modified: cfe/trunk/include/clang/AST/ExprCXX.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ExprCXX.h?rev=373247=373246=373247=diff
==
--- cfe/trunk/include/clang/AST/ExprCXX.h (original)
+++ cfe/trunk/include/clang/AST/ExprCXX.h Mon Sep 30 12:12:29 2019
@@ -1907,6 +1907,10 @@ public:
   /// lambda expression.
   CXXMethodDecl *getCallOperator() const;
 
+  /// Retrieve the function template call operator associated with this
+  /// lambda expression.
+  FunctionTemplateDecl *getDependentCallOperator() const;
+
   /// If this is a generic lambda expression, retrieve the template
   /// parameter list associated with it, or else return null.
   TemplateParameterList *getTemplateParameterList() const;

Modified: cfe/trunk/lib/AST/DeclCXX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclCXX.cpp?rev=373247=373246=373247=diff
==
--- cfe/trunk/lib/AST/DeclCXX.cpp (original)
+++ cfe/trunk/lib/AST/DeclCXX.cpp Mon Sep 30 12:12:29 2019
@@ -1399,17 +1399,25 @@ static bool allLookupResultsAreTheSame(c
 }
 #endif
 
-CXXMethodDecl* CXXRecordDecl::getLambdaCallOperator() const {
-  if (!isLambda()) return nullptr;
+NamedDecl* getLambdaCallOperatorHelper(const CXXRecordDecl ) {
+  if (!RD.isLambda()) return nullptr;
   DeclarationName Name =
-getASTContext().DeclarationNames.getCXXOperatorName(OO_Call);
-  DeclContext::lookup_result Calls = lookup(Name);
+RD.getASTContext().DeclarationNames.getCXXOperatorName(OO_Call);
+  DeclContext::lookup_result Calls = RD.lookup(Name);
 
   assert(!Calls.empty() && "Missing lambda call operator!");
   assert(allLookupResultsAreTheSame(Calls) &&
  "More than one lambda call operator!");
+  return Calls.front();
+}
+
+FunctionTemplateDecl* CXXRecordDecl::getDependentLambdaCallOperator() const {
+  NamedDecl *CallOp = getLambdaCallOperatorHelper(*this);
+  return  dyn_cast(CallOp);
+}
 
-  NamedDecl *CallOp = Calls.front();
+CXXMethodDecl *CXXRecordDecl::getLambdaCallOperator() const {
+  NamedDecl *CallOp = getLambdaCallOperatorHelper(*this);
   if (const auto *CallOpTmpl = dyn_cast(CallOp))
 return cast(CallOpTmpl->getTemplatedDecl());
 

Modified: cfe/trunk/lib/AST/ExprCXX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprCXX.cpp?rev=373247=373246=373247=diff
==
--- cfe/trunk/lib/AST/ExprCXX.cpp (original)
+++ cfe/trunk/lib/AST/ExprCXX.cpp Mon Sep 30 12:12:29 2019
@@ -1218,6 +1218,11 @@ CXXMethodDecl *LambdaExpr::getCallOperat
   return Record->getLambdaCallOperator();
 }
 
+FunctionTemplateDecl *LambdaExpr::getDependentCallOperator() const {
+  CXXRecordDecl *Record = getLambdaClass();
+  return Record->getDependentLambdaCallOperator();
+}
+
 TemplateParameterList *LambdaExpr::getTemplateParameterList() 

r373243 - [OPENMP50]Do not emit warning for the function with the currently

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Mon Sep 30 11:24:35 2019
New Revision: 373243

URL: http://llvm.org/viewvc/llvm-project?rev=373243=rev
Log:
[OPENMP50]Do not emit warning for the function with the currently
defined body.

If the function is currently defined, we should not emit a warning that
it might be emitted already because it was not really emitted.

Modified:
cfe/trunk/lib/Sema/SemaOpenMP.cpp
cfe/trunk/test/OpenMP/declare_variant_messages.c
cfe/trunk/test/OpenMP/declare_variant_messages.cpp

Modified: cfe/trunk/lib/Sema/SemaOpenMP.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOpenMP.cpp?rev=373243=373242=373243=diff
==
--- cfe/trunk/lib/Sema/SemaOpenMP.cpp (original)
+++ cfe/trunk/lib/Sema/SemaOpenMP.cpp Mon Sep 30 11:24:35 2019
@@ -4936,8 +4936,9 @@ Sema::checkOpenMPDeclareVariantFunction(
 << FD->getLocation();
 
   // Check if the function was emitted already.
-  if ((LangOpts.EmitAllDecls && FD->isDefined()) ||
-  Context.DeclMustBeEmitted(FD))
+  const FunctionDecl *Definition;
+  if (!FD->isThisDeclarationADefinition() && FD->isDefined(Definition) &&
+  (LangOpts.EmitAllDecls || Context.DeclMustBeEmitted(Definition)))
 Diag(SR.getBegin(), diag::warn_omp_declare_variant_after_emitted)
 << FD->getLocation();
 

Modified: cfe/trunk/test/OpenMP/declare_variant_messages.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/declare_variant_messages.c?rev=373243=373242=373243=diff
==
--- cfe/trunk/test/OpenMP/declare_variant_messages.c (original)
+++ cfe/trunk/test/OpenMP/declare_variant_messages.c Mon Sep 30 11:24:35 2019
@@ -79,9 +79,13 @@ int bar() {
 // expected-warning@+1 {{'#pragma omp declare variant' cannot be applied for 
function after first usage; the original function might be used}}
 #pragma omp declare variant(after_use_variant) match(xxx={})
 int after_use(void);
-// expected-warning@+1 {{#pragma omp declare variant' cannot be applied to the 
function that was defined already; the original function might be used}}
 #pragma omp declare variant(after_use_variant) match(xxx={})
 int defined(void) { return 0; }
+int defined1(void) { return 0; }
+// expected-warning@+1 {{#pragma omp declare variant' cannot be applied to the 
function that was defined already; the original function might be used}}
+#pragma omp declare variant(after_use_variant) match(xxx={})
+int defined1(void);
+
 
 int diff_cc_variant(void);
 // expected-error@+1 {{function with '#pragma omp declare variant' has a 
different calling convention}}

Modified: cfe/trunk/test/OpenMP/declare_variant_messages.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/declare_variant_messages.cpp?rev=373243=373242=373243=diff
==
--- cfe/trunk/test/OpenMP/declare_variant_messages.cpp (original)
+++ cfe/trunk/test/OpenMP/declare_variant_messages.cpp Mon Sep 30 11:24:35 2019
@@ -172,10 +172,14 @@ auto fn_deduced_variant() { return 0; }
 int fn_deduced();
 
 int fn_deduced_variant1();
-// expected-warning@+1 {{'#pragma omp declare variant' cannot be applied to 
the function that was defined already; the original function might be used}}
 #pragma omp declare variant(fn_deduced_variant1) match(xxx = {})
 auto fn_deduced1() { return 0; }
 
+auto fn_deduced3() { return 0; }
+// expected-warning@+1 {{'#pragma omp declare variant' cannot be applied to 
the function that was defined already; the original function might be used}}
+#pragma omp declare variant(fn_deduced_variant1) match(xxx = {})
+auto fn_deduced3();
+
 auto fn_deduced_variant2() { return 0; }
 // expected-error@+1 {{variant in '#pragma omp declare variant' with type 'int 
()' is incompatible with type 'float (*)()'}}
 #pragma omp declare variant(fn_deduced_variant2) match(xxx = {})


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

[PATCH] D68114: Fix for expanding __pragmas in macro arguments

[Amy Huang via Phabricator via cfe-commits]

akhuang updated this revision to Diff 222464.
akhuang added a comment.

comments/whitespace/test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68114

Files:
  clang/lib/Lex/Pragma.cpp
  clang/test/Preprocessor/pragma_microsoft.c

Index: clang/test/Preprocessor/pragma_microsoft.c
===
--- clang/test/Preprocessor/pragma_microsoft.c
+++ clang/test/Preprocessor/pragma_microsoft.c
@@ -51,6 +51,8 @@
   __pragma(warning(pop)); \
 }
 
+#define PRAGMA_IN_ARGS(p) p
+
 void f()
 {
   __pragma() // expected-warning{{unknown pragma ignored}}
@@ -64,8 +66,16 @@
 // CHECK: #pragma warning(disable: 1)
 // CHECK: ; 1 + (2 > 3) ? 4 : 5;
 // CHECK: #pragma warning(pop)
-}
 
+  // Check that macro arguments can contain __pragma.
+  PRAGMA_IN_ARGS(MACRO_WITH__PRAGMA) // expected-warning {{lower precedence}} \
+ // expected-note 2 {{place parentheses}} \
+ // expected-warning {{expression result unused}}
+// CHECK: #pragma warning(push)
+// CHECK: #pragma warning(disable: 1)
+// CHECK: ; 1 + (2 > 3) ? 4 : 5;
+// CHECK: #pragma warning(pop)
+}
 
 // This should include macro_arg_directive even though the include
 // is looking for test.h  This allows us to assign to "n"
Index: clang/lib/Lex/Pragma.cpp
===
--- clang/lib/Lex/Pragma.cpp
+++ clang/lib/Lex/Pragma.cpp
@@ -328,11 +328,43 @@
 /// HandleMicrosoft__pragma - Like Handle_Pragma except the pragma text
 /// is not enclosed within a string literal.
 void Preprocessor::HandleMicrosoft__pragma(Token ) {
+  // During macro pre-expansion, check the syntax now but put the tokens back
+  // into the token stream for later consumption. Same as Handle_Pragma.
+  struct TokenCollector {
+Preprocessor 
+bool Collect;
+SmallVector Tokens;
+Token 
+
+void lex() {
+  if (Collect)
+Tokens.push_back(Tok);
+  Self.Lex(Tok);
+}
+
+void revert() {
+  assert(Collect && "did not collect tokens");
+  assert(!Tokens.empty() && "collected unexpected number of tokens");
+
+  // Push the pragma content tokens into the token stream.
+  auto Toks = std::make_unique(Tokens.size());
+  std::copy(Tokens.begin() + 1, Tokens.end(), Toks.get());
+  Toks[Tokens.size() - 1] = Tok;
+  Self.EnterTokenStream(std::move(Toks), Tokens.size(),
+/*DisableMacroExpansion*/ true,
+/*IsReinject*/ true);
+
+  // ... and return the _Pragma token unchanged.
+  Tok = *Tokens.begin();
+}
+  };
+  TokenCollector Toks = {*this, InMacroArgPreExpansion, {}, Tok};
+
   // Remember the pragma token location.
   SourceLocation PragmaLoc = Tok.getLocation();
 
   // Read the '('.
-  Lex(Tok);
+  Toks.lex();
   if (Tok.isNot(tok::l_paren)) {
 Diag(PragmaLoc, diag::err__Pragma_malformed);
 return;
@@ -341,14 +373,14 @@
   // Get the tokens enclosed within the __pragma(), as well as the final ')'.
   SmallVector PragmaToks;
   int NumParens = 0;
-  Lex(Tok);
+  Toks.lex();
   while (Tok.isNot(tok::eof)) {
 PragmaToks.push_back(Tok);
 if (Tok.is(tok::l_paren))
   NumParens++;
 else if (Tok.is(tok::r_paren) && NumParens-- == 0)
   break;
-Lex(Tok);
+Toks.lex();
   }
 
   if (Tok.is(tok::eof)) {
@@ -356,6 +388,12 @@
 return;
   }
 
+  // If we're expanding a macro argument, put the tokens back.
+  if (InMacroArgPreExpansion) {
+Toks.revert();
+return;
+  }
+
   PragmaToks.front().setFlag(Token::LeadingSpace);
 
   // Replace the ')' with an EOD to mark the end of the pragma.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D14484: Formatting constructor initializer lists by putting them always on different lines

[Pooya Daravi via Phabricator via cfe-commits]

puya added a comment.

For what it's worth the original post is also the style used at our company. I 
am hesitant to patch clang-format as I don't want to have to maintain it so I 
hope this is added.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D14484



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

[PATCH] D68141: [Alignment][NFC] Remove AllocaInst::setAlignment(unsigned)

[Alexander Richardson via Phabricator via cfe-commits]

arichardson added inline comments.



Comment at: llvm/trunk/lib/Target/AArch64/AArch64StackTagging.cpp:65
 
-static constexpr unsigned kTagGranuleSize = 16;
+static const Align kTagGranuleSize = Align(16);
 

gchatelet wrote:
> arichardson wrote:
> > Can't the Align ctor be constexpr? Will this result in a `Log2` call at 
> > runtime?
> The implementation of Log2 is quite complex and depends on the implementation 
> of [[ 
> https://github.com/llvm/llvm-project/blob/02ada9bd2b41d850876a483bede59715e7550c1e/llvm/include/llvm/Support/MathExtras.h#L127
>  | countLeadingZeros ]].
> GCC, clang, ICC can compute the value at compile time but there may be some 
> compiler that can't.
> https://godbolt.org/z/ytwWHn
> 
> I could solve this issue by introducing a `LogAlign()` function returning an 
> `Align`. This function could be `constexpr`. What do you think? Is it worth 
> the additional complexity?
Since there won't a global ctor at -O2 or higher it probably doesn't matter.

I'm not sure if there have been any of these cases yet, but I would think that 
having a `LogAlign/Align::fromLog()/etc.` might still be useful for some cases 
where you get the alignment as log2 instead of in bytes?
This would avoid additional work for the compiler and might be easier to read 
since you can omit the `1

[PATCH] D66696: [ObjC generics] Fix not inheriting type bounds in categories/extensions.

[Erik Pilkington via Phabricator via cfe-commits]

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

Okay, LGTM.


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

https://reviews.llvm.org/D66696



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

[PATCH] D67592: [Clang] Use -main-file-name for source filename if not set

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Yup, test is happy now, thanks!


Repository:
  rL LLVM

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

https://reviews.llvm.org/D67592



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

[PATCH] D67592: [Clang] Use -main-file-name for source filename if not set

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson added a comment.

In D67592#1688413 , @thakis wrote:

> This is failing on my mac like so:


This should be fixed by r373237. I didn't get any email with buildbot failures, 
but someone mailed one to me manually. Not sure why it only fails sometimes.

> 
> 
>   FAIL: Clang :: Frontend/stdin-input.c (1 of 1)
>    TEST 'Clang :: Frontend/stdin-input.c' FAILED 
> 
>   Script:
>   --
>   : 'RUN: at line 1';   cat 
> /Users/thakis/src/llvm-project/clang/test/Frontend/stdin-input.c | 
> /Users/thakis/src/llvm-project/out/gn/bin/clang -emit-llvm -g -S  -Xclang 
> -main-file-name -Xclang test/foo.c -x c - -o - | 
> /Users/thakis/src/llvm-project/out/gn/bin/FileCheck 
> /Users/thakis/src/llvm-project/clang/test/Frontend/stdin-input.c
>   --
>   Exit Code: 1
>   
>   Command Output (stderr):
>   --
>   + : 'RUN: at line 1'
>   + cat /Users/thakis/src/llvm-project/clang/test/Frontend/stdin-input.c
>   + /Users/thakis/src/llvm-project/out/gn/bin/clang -emit-llvm -g -S -Xclang 
> -main-file-name -Xclang test/foo.c -x c - -o -
>   + /Users/thakis/src/llvm-project/out/gn/bin/FileCheck 
> /Users/thakis/src/llvm-project/clang/test/Frontend/stdin-input.c
>   /Users/thakis/src/llvm-project/clang/test/Frontend/stdin-input.c:5:11: 
> error: CHECK: expected string not found in input
>   // CHECK: !1 = !DIFile(filename: "test/foo.c"
> ^
>   :3:1: note: scanning from here
>   target datalayout = 
> "e-m:o-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
>   ^
>   :24:1: note: possible intended match here
>   !6 = !DIFile(filename: "test/foo.c", directory: 
> "/Users/thakis/src/llvm-project/out/gn/obj/clang/test/Frontend")
>   ^
>   
>   --
>   
>   
>   Testing Time: 0.06s
>   
>   Failing Tests (1):
>   Clang :: Frontend/stdin-input.c
> 
> 
> If I manually run `bin/llvm-lit clang/test/Frontend/stdin-input.c` it fails, 
> but if I run the command it claims to be running it passes.
> 
> If I replace the `FileCheck` on the run line with `> tmp.txt` I see what the 
> failing FileCheck is seeing:
> 
>   !0 = !{i32 2, !"SDK Version", [2 x i32] [i32 10, i32 14]}
>   !1 = !{i32 2, !"Dwarf Version", i32 4}
>   !2 = !{i32 2, !"Debug Info Version", i32 3}
>   !3 = !{i32 1, !"wchar_size", i32 4}
>   !4 = !{i32 7, !"PIC Level", i32 2}
>   !5 = distinct !DICompileUnit(language: DW_LANG_C99, file: !6, producer: 
> "clang version 10.0.0 ", isOptimized: false, runtimeVersion: 0, emissionKind: 
> FullDebug, enums: !7, nameTableKind: None)
>   !6 = !DIFile(filename: "test/foo.c", directory: 
> "/Users/thakis/src/llvm-project/out/gn/obj/clang/test/Frontend")
> 
> 
> I'm guessing lit sets some env var that makes clang emit these additional 
> values (?)




Repository:
  rL LLVM

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

https://reviews.llvm.org/D67592



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

r373237 - Fix buildbot failure from r373217 (don't match metadata id exactly)

[Teresa Johnson via cfe-commits]

Author: tejohnson
Date: Mon Sep 30 10:26:48 2019
New Revision: 373237

URL: http://llvm.org/viewvc/llvm-project?rev=373237=rev
Log:
Fix buildbot failure from r373217 (don't match metadata id exactly)

Fix this failure by ignoring the id of the metadata being checked:

http://green.lab.llvm.org/green/job/clang-stage1-cmake-RA-incremental/3046/consoleFull#-21332887158254eaf0-7326-4999-85b0-388101f2d404

Modified:
cfe/trunk/test/Frontend/stdin-input.c

Modified: cfe/trunk/test/Frontend/stdin-input.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Frontend/stdin-input.c?rev=373237=373236=373237=diff
==
--- cfe/trunk/test/Frontend/stdin-input.c (original)
+++ cfe/trunk/test/Frontend/stdin-input.c Mon Sep 30 10:26:48 2019
@@ -2,6 +2,6 @@
 // RUN: -Xclang -main-file-name -Xclang test/foo.c -x c - -o - | FileCheck %s
 // CHECK: ; ModuleID = 'test/foo.c'
 // CHECK: source_filename = "test/foo.c"
-// CHECK: !1 = !DIFile(filename: "test/foo.c"
+// CHECK: !DIFile(filename: "test/foo.c"
 
 int main() {}


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

[PATCH] D67592: [Clang] Use -main-file-name for source filename if not set

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

This is failing on my mac like so:

  FAIL: Clang :: Frontend/stdin-input.c (1 of 1)
   TEST 'Clang :: Frontend/stdin-input.c' FAILED 

  Script:
  --
  : 'RUN: at line 1';   cat 
/Users/thakis/src/llvm-project/clang/test/Frontend/stdin-input.c | 
/Users/thakis/src/llvm-project/out/gn/bin/clang -emit-llvm -g -S  -Xclang 
-main-file-name -Xclang test/foo.c -x c - -o - | 
/Users/thakis/src/llvm-project/out/gn/bin/FileCheck 
/Users/thakis/src/llvm-project/clang/test/Frontend/stdin-input.c
  --
  Exit Code: 1
  
  Command Output (stderr):
  --
  + : 'RUN: at line 1'
  + cat /Users/thakis/src/llvm-project/clang/test/Frontend/stdin-input.c
  + /Users/thakis/src/llvm-project/out/gn/bin/clang -emit-llvm -g -S -Xclang 
-main-file-name -Xclang test/foo.c -x c - -o -
  + /Users/thakis/src/llvm-project/out/gn/bin/FileCheck 
/Users/thakis/src/llvm-project/clang/test/Frontend/stdin-input.c
  /Users/thakis/src/llvm-project/clang/test/Frontend/stdin-input.c:5:11: error: 
CHECK: expected string not found in input
  // CHECK: !1 = !DIFile(filename: "test/foo.c"
^
  :3:1: note: scanning from here
  target datalayout = 
"e-m:o-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
  ^
  :24:1: note: possible intended match here
  !6 = !DIFile(filename: "test/foo.c", directory: 
"/Users/thakis/src/llvm-project/out/gn/obj/clang/test/Frontend")
  ^
  
  --
  
  
  Testing Time: 0.06s
  
  Failing Tests (1):
  Clang :: Frontend/stdin-input.c

If I manually run `bin/llvm-lit clang/test/Frontend/stdin-input.c` it fails, 
but if I run the command it claims to be running it passes.

If I replace the `FileCheck` on the run line with `> tmp.txt` I see what the 
failing FileCheck is seeing:

  !0 = !{i32 2, !"SDK Version", [2 x i32] [i32 10, i32 14]}
  !1 = !{i32 2, !"Dwarf Version", i32 4}
  !2 = !{i32 2, !"Debug Info Version", i32 3}
  !3 = !{i32 1, !"wchar_size", i32 4}
  !4 = !{i32 7, !"PIC Level", i32 2}
  !5 = distinct !DICompileUnit(language: DW_LANG_C99, file: !6, producer: 
"clang version 10.0.0 ", isOptimized: false, runtimeVersion: 0, emissionKind: 
FullDebug, enums: !7, nameTableKind: None)
  !6 = !DIFile(filename: "test/foo.c", directory: 
"/Users/thakis/src/llvm-project/out/gn/obj/clang/test/Frontend")

I'm guessing lit sets some env var that makes clang emit these additional 
values (?)


Repository:
  rL LLVM

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

https://reviews.llvm.org/D67592



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

[PATCH] D67830: [AArch64][SVE] Implement punpk[hi|lo] intrinsics

[Kerry McLaughlin via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL373232: [AArch64][SVE] Implement punpk[hi|lo] intrinsics 
(authored by kmclaughlin, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D67830?vs=221010=222458#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D67830

Files:
  llvm/trunk/include/llvm/IR/IntrinsicsAArch64.td
  llvm/trunk/lib/IR/Function.cpp
  llvm/trunk/lib/Target/AArch64/AArch64SVEInstrInfo.td
  llvm/trunk/lib/Target/AArch64/SVEInstrFormats.td
  llvm/trunk/test/CodeGen/AArch64/sve-intrinsics-pred-operations.ll

Index: llvm/trunk/test/CodeGen/AArch64/sve-intrinsics-pred-operations.ll
===
--- llvm/trunk/test/CodeGen/AArch64/sve-intrinsics-pred-operations.ll
+++ llvm/trunk/test/CodeGen/AArch64/sve-intrinsics-pred-operations.ll
@@ -0,0 +1,65 @@
+; RUN: llc -mtriple=aarch64-linux-gnu -mattr=+sve < %s | FileCheck %s
+
+;
+; PUNPKHI
+;
+
+define  @punpkhi_b16( %a) {
+; CHECK-LABEL: punpkhi_b16
+; CHECK: punpkhi p0.h, p0.b
+; CHECK-NEXT: ret
+  %res = call  @llvm.aarch64.sve.punpkhi.nxv8i1( %a)
+  ret  %res
+}
+
+define  @punpkhi_b8( %a) {
+; CHECK-LABEL: punpkhi_b8
+; CHECK: punpkhi p0.h, p0.b
+; CHECK-NEXT: ret
+  %res = call  @llvm.aarch64.sve.punpkhi.nxv4i1( %a)
+  ret  %res
+}
+
+define  @punpkhi_b4( %a) {
+; CHECK-LABEL: punpkhi_b4
+; CHECK: punpkhi p0.h, p0.b
+; CHECK-NEXT: ret
+  %res = call  @llvm.aarch64.sve.punpkhi.nxv2i1( %a)
+  ret  %res
+}
+
+;
+; PUNPKLO
+;
+
+define  @punpklo_b16( %a) {
+; CHECK-LABEL: punpklo_b16
+; CHECK: punpklo p0.h, p0.b
+; CHECK-NEXT: ret
+  %res = call  @llvm.aarch64.sve.punpklo.nxv8i1( %a)
+  ret  %res
+}
+
+define  @punpklo_b8( %a) {
+; CHECK-LABEL: punpklo_b8
+; CHECK: punpklo p0.h, p0.b
+; CHECK-NEXT: ret
+  %res = call  @llvm.aarch64.sve.punpklo.nxv4i1( %a)
+  ret  %res
+}
+
+define  @punpklo_b4( %a) {
+; CHECK-LABEL: punpklo_b4
+; CHECK: punpklo p0.h, p0.b
+; CHECK-NEXT: ret
+  %res = call  @llvm.aarch64.sve.punpklo.nxv2i1( %a)
+  ret  %res
+}
+
+declare  @llvm.aarch64.sve.punpkhi.nxv8i1()
+declare  @llvm.aarch64.sve.punpkhi.nxv4i1()
+declare  @llvm.aarch64.sve.punpkhi.nxv2i1()
+
+declare  @llvm.aarch64.sve.punpklo.nxv8i1()
+declare  @llvm.aarch64.sve.punpklo.nxv4i1()
+declare  @llvm.aarch64.sve.punpklo.nxv2i1()
Index: llvm/trunk/lib/Target/AArch64/SVEInstrFormats.td
===
--- llvm/trunk/lib/Target/AArch64/SVEInstrFormats.td
+++ llvm/trunk/lib/Target/AArch64/SVEInstrFormats.td
@@ -283,6 +283,11 @@
 // SVE pattern match helpers.
 //===--===//
 
+class SVE_1_Op_Pat
+: Pat<(vtd (op vt1:$Op1)),
+  (inst $Op1)>;
+
 class SVE_3_Op_Pat
 : Pat<(vtd (op vt1:$Op1, vt2:$Op2, vt3:$Op3)),
@@ -4280,6 +4285,14 @@
   let Inst{3-0}   = Pd;
 }
 
+multiclass sve_int_perm_punpk {
+  def NAME : sve_int_perm_punpk;
+
+  def : SVE_1_Op_Pat(NAME)>;
+  def : SVE_1_Op_Pat(NAME)>;
+  def : SVE_1_Op_Pat(NAME)>;
+}
+
 class sve_int_rdffr_pred
 : I<(outs PPR8:$Pd), (ins PPRAny:$Pg),
   asm, "\t$Pd, $Pg/z",
Index: llvm/trunk/lib/Target/AArch64/AArch64SVEInstrInfo.td
===
--- llvm/trunk/lib/Target/AArch64/AArch64SVEInstrInfo.td
+++ llvm/trunk/lib/Target/AArch64/AArch64SVEInstrInfo.td
@@ -216,8 +216,8 @@
   defm UUNPKLO_ZZ : sve_int_perm_unpk<0b10, "uunpklo">;
   defm UUNPKHI_ZZ : sve_int_perm_unpk<0b11, "uunpkhi">;
 
-  def  PUNPKLO_PP : sve_int_perm_punpk<0b0, "punpklo">;
-  def  PUNPKHI_PP : sve_int_perm_punpk<0b1, "punpkhi">;
+  defm PUNPKLO_PP : sve_int_perm_punpk<0b0, "punpklo", int_aarch64_sve_punpklo>;
+  defm PUNPKHI_PP : sve_int_perm_punpk<0b1, "punpkhi", int_aarch64_sve_punpkhi>;
 
   defm MOVPRFX_ZPzZ : sve_int_movprfx_pred_zero<0b000, "movprfx">;
   defm MOVPRFX_ZPmZ : sve_int_movprfx_pred_merge<0b001, "movprfx">;
Index: llvm/trunk/lib/IR/Function.cpp
===
--- llvm/trunk/lib/IR/Function.cpp
+++ llvm/trunk/lib/IR/Function.cpp
@@ -1211,8 +1211,9 @@
 }
 case IITDescriptor::HalfVecArgument:
   // If this is a forward reference, defer the check for later.
-  return D.getArgumentNumber() >= ArgTys.size() ||
- !isa(ArgTys[D.getArgumentNumber()]) ||
+  if (D.getArgumentNumber() >= ArgTys.size())
+return IsDeferredCheck || DeferCheck(Ty);
+  return !isa(ArgTys[D.getArgumentNumber()]) ||
  VectorType::getHalfElementsVectorType(
  cast(ArgTys[D.getArgumentNumber()])) != Ty;
 case IITDescriptor::SameVecWidthArgument: {
Index: llvm/trunk/include/llvm/IR/IntrinsicsAArch64.td
===
--- llvm/trunk/include/llvm/IR/IntrinsicsAArch64.td
+++ llvm/trunk/include/llvm/IR/IntrinsicsAArch64.td
@@ -768,6 +768,11 

[PATCH] D68193: In openFileForRead don't cache erroneous entries if the error relates to them being directories. Add tests.

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith requested changes to this revision.
dexonsmith added inline comments.
This revision now requires changes to proceed.



Comment at: 
clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp:21-22
   // Load the file and its content from the file system.
   llvm::ErrorOr> MaybeFile =
   FS.openFileForRead(Filename);
   if (!MaybeFile)

Note that the file is opened here.



Comment at: 
clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp:236-237
 if (!CacheEntry.isValid()) {
+  llvm::vfs::FileSystem  = getUnderlyingFS();
+  auto MaybeStatus = FS.status(Filename);
+

It seems wasteful to do an extra stat here when the file is already open in 
`createFileEntry`.

Can you instead change `createFileEntry` to return `std::errc::is_a_directory` 
as appropriate to avoid the extra filesystem access?  (Is it possible that it's 
already doing that, and you just need to check for that?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68193



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

[PATCH] D67695: [clangd] Implement getBeginning for overloaded operators.

[Ilya Biryukov via Phabricator via cfe-commits]

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

Mostly LGTM




Comment at: clang-tools-extra/clangd/SourceCode.cpp:258
+
+TokenKind getTokenKind(SourceLocation Loc, const SourceManager ,
+   const LangOptions ) {

hokein wrote:
> ilya-biryukov wrote:
> > NIT: add `static` for consistency with the rest of the function.
> I think the static is redundant here, as the function is in the anonymous 
> namespace, I removed the `static` on the function above.
LG, that's also fine.

I suggested 'static' because other functions in this file are using it (in 
addition to being in the anonymous namespace, I believe).




Comment at: clang-tools-extra/clangd/unittests/SourceCodeTests.cpp:330
+   "/^/ comments", // non-interesting token
+   "void f(int abc) { abc ^ ++; }",// whitespace
+   "void f(int abc) { ^abc^++; }", // range of identifier

Do we test `++^^abc` anywhere?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67695



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

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

[David Goldman via Phabricator via cfe-commits]

dgoldman added a comment.
Herald added a subscriber: usaxena95.

In D64799#1651908 , @vsapsai wrote:

> For the record, there was another change regarding the delayed typos in 
> `clang::Sema::~Sema()`: D62648  [Sema][Typo] 
> Fix assertion failure for expressions with multiple typos.


Note that D62648  only makes fixes to the typo 
correction application (e.g. when `CorrectDelayedTyposInExpr` is called), in 
particular caused by the recursive nature of typo correction (correcting typos 
might introduce more typos!). The assertion failure can still be caused by 
failing to correct Typos (e.g not calling `CorrectDelayedTyposInExpr` for a 
given Expression).

This patch seems like a good idea as currently it only impacts clangd/libclang 
in practice. But here a few thoughts about potentially fixing the root cause 
here:

- It looks as if when initially implemented 

 typos were meant to be propagated up through the `ExprEvalContext` (see the 
commit description) as opposed to automatically clearing for every context, 
perhaps due to the issues described above. It's unclear what the intended 
behavior is though - when should `CorrectDelayedTyposInExpr` be called? When 
are we in a `stable state` to do typo correction?
- From my limited understanding there is one main issue here: failure to call 
`CorrectDelayedTyposInExpr` for an expression. When fixing the typo correction 
handling mentioned above, this could happen during `TreeTransform` due to an 
Expression being created (along with a typo) and then being thrown away (due to 
an error). The problem here is that once it is discarded, it becomes 
unreachable. I'm assuming the proper handling here is to 
`CorrectDelayedTyposInExpr` more frequently, but I'm not quite sure.
  - Could `Sema` check if a `TypoExpr` becomes unreachable? If so, we can check 
for unreachable `TypoExprs`, diagnose them, and potentially if in `DEBUG` emit 
a warning (although we'd need to track where a `TypoExpr` was created to really 
understand why it was thrown away).



In D64799#1605514 , @ilya-biryukov 
wrote:

> Tried the suggested approach and ran into the issue described above.
>  Either we make the diagnostics less useful ('did you mean foo::bar?' turns 
> into 'unresolved identifier bar'; without mentioning the correction)  or we 
> even stop providing some corrections in addition to that.
>
> On the other hand, I agree that over time we will start emitting too many 
> diagnostics at the end of TU if keep the patch as is. That is not a good way.
>  There should be a better option for emitting the uncorrected diagnostics 
> closer to where they are produced, but I don't have a good idea on what it 
> should be off the top of my head. Ideas warmly welcome.


I might be wrong here, but I thought that diagnostics were delayed until typo 
correction has been completed on an expression. For example, you'll only get 
something like `unresolved identifier bar` instead of `did you mean foo::bar?` 
only when you call the `DiagHandler` with either a proper `TypoCorrection` or 
an empty one. If so, I'm not sure how you'd get into this case if you're 
calling `CorrectDelayedTyposInExpr` and tracking which typos have been resolved 
already.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64799



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

[PATCH] D68157: [X86][ABI] Keep empty class argument passing by value compatible with GCC.

[Michael Liao via Phabricator via cfe-commits]

hliao added a comment.

ping for review


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68157



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

[PATCH] D68117: [DWARF-5] Support for C++11 defaulted, deleted member functions.

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

In D68117#1686929 , @SouraVX wrote:

> In D68117#1686235 , @aprantl wrote:
>
> > This needs a lot more test coverage: The frontend cases aren't all covered 
> > by frontend checks and neither is the dwarf output.
>
>
> Do you mean to add more test cases ?  Could you please elaborate on this


I think it would be good to add a separate CFE test for this instead of 
piggybacking on clang/test/CodeGenCXX/dbg-info-all-calls-described.cpp whose 
indiates that it is really testing something else. The new test should fail if 
I comment out any of the changes in CreateCXXMemberFunction. Second, there 
needs to be a test in llvm/test/DebugInfo that takes LLVM IR as input and 
checks that the new attributes are showing up in the dwarfdump output.




Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:1608
 
+  if (CGM.getCodeGenOpts().DwarfVersion >= 5) {
+// DWARF-5 support for, defaulted, deleted member functions

The clang frontend should not be the one making the decision here, if possible 
it would be better to unconditionally emit this info in LLVM IR and then filter 
it out in the backend. I could imagine that the PDB backend might find this 
info useful, too (but I don't know).


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

https://reviews.llvm.org/D68117



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

[PATCH] D68023: [AArch64][SVE] Implement int_aarch64_sve_cnt intrinsic

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes accepted this revision.
c-rhodes added a comment.
This revision is now accepted and ready to land.

LGTM




Comment at: llvm/lib/Target/AArch64/AArch64SVEInstrInfo.td:100-104
+  defm CLS_ZPmZ  : sve_int_un_pred_arit_1<   0b000, "cls", null_frag>;
+  defm CLZ_ZPmZ  : sve_int_un_pred_arit_1<   0b001, "clz", null_frag>;
+  defm CNT_ZPmZ  : sve_int_un_pred_arit_1<   0b010, "cnt", 
int_aarch64_sve_cnt>;
+  defm CNOT_ZPmZ : sve_int_un_pred_arit_1<   0b011, "cnot", null_frag>;
+  defm NOT_ZPmZ  : sve_int_un_pred_arit_1<   0b110, "not", null_frag>;

nit: align last argument


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

https://reviews.llvm.org/D68023



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

[PATCH] D68227: [clang-format] [PR43372] - clang-format shows replacements in DOS files when no replacement is needed

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay created this revision.
MyDeveloperDay added reviewers: krasimir, klimek, owenpan, ioeric.
MyDeveloperDay added a project: clang-format.
Herald added a project: clang.
MyDeveloperDay edited the summary of this revision.

This is a patch to fix PR43372 (https://bugs.llvm.org/show_bug.cgi?id=43372) - 
clang-format can't format file with includes, ( which really keep providing 
replacements for already sorted headers.)

A similar issue was addressed by @krasimir in D60199: [clang-format] Do not 
emit replacements while regrouping if Cpp includes are OK 
, however, this seemingly only prevented the 
issue when the files being formatted did not contain windows line endings (\r\n)

It's possible this is related to 
https://twitter.com/StephanTLavavej/status/1176722938243895296 given who 
@STL_MSFT  works for!

As people often used the existence of replacements to determine if a file needs 
clang-formatting, this is probably pretty important for windows users

There may be a better way of comparing 2 strings and ignoring \r (which appear 
in both Results and Code), I couldn't choose between this idiom or the copy_if 
approach, but I'm happy to change it to whatever people consider more 
performant.


Repository:
  rC Clang

https://reviews.llvm.org/D68227

Files:
  clang/lib/Format/Format.cpp
  clang/unittests/Format/SortImportsTestJava.cpp
  clang/unittests/Format/SortIncludesTest.cpp


Index: clang/unittests/Format/SortIncludesTest.cpp
===
--- clang/unittests/Format/SortIncludesTest.cpp
+++ clang/unittests/Format/SortIncludesTest.cpp
@@ -653,6 +653,14 @@
   EXPECT_EQ(Code, sort(Code, "input.h", 0));
 }
 
+TEST_F(SortIncludesTest,
+   DoNotOutputReplacementsForSortedBlocksWithRegroupingWindows) {
+  Style.IncludeBlocks = Style.IBS_Regroup;
+  std::string Code = "#include \"b.h\"\r\n"
+ "\r\n"
+ "#include \r\n";
+  EXPECT_EQ(Code, sort(Code, "input.h", 0));
+}
 
 TEST_F(SortIncludesTest, DoNotRegroupGroupsInGoogleObjCStyle) {
   FmtStyle = getGoogleStyle(FormatStyle::LK_ObjC);
Index: clang/unittests/Format/SortImportsTestJava.cpp
===
--- clang/unittests/Format/SortImportsTestJava.cpp
+++ clang/unittests/Format/SortImportsTestJava.cpp
@@ -285,6 +285,13 @@
   sortIncludes(FmtStyle, Code, GetCodeRange(Code), "input.java").empty());
 }
 
+TEST_F(SortImportsTestJava, NoReplacementsForValidImportsWindows) {
+  std::string Code = "import org.a;\r\n"
+ "import org.b;\r\n";
+  EXPECT_TRUE(
+  sortIncludes(FmtStyle, Code, GetCodeRange(Code), "input.java").empty());
+}
+
 } // end namespace
 } // end namespace format
 } // end namespace clang
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -1808,6 +1808,14 @@
   return std::make_pair(CursorIndex, OffsetToEOL);
 }
 
+// Compares two strings but ignores any \r in either strings.
+static bool compareIgnoringCarriageReturns(std::string Result,
+   std::string Code) {
+  Result.erase(std::remove(Result.begin(), Result.end(), '\r'), Result.end());
+  Code.erase(std::remove(Code.begin(), Code.end(), '\r'), Code.end());
+  return Result == Code;
+}
+
 // Sorts and deduplicate a block of includes given by 'Includes' alphabetically
 // adding the necessary replacement to 'Replaces'. 'Includes' must be in strict
 // source order.
@@ -1879,7 +1887,8 @@
 
   // If the #includes are out of order, we generate a single replacement fixing
   // the entire range of blocks. Otherwise, no replacement is generated.
-  if (result == Code.substr(IncludesBeginOffset, IncludesBlockSize))
+  if (compareIgnoringCarriageReturns(
+  result, Code.substr(IncludesBeginOffset, IncludesBlockSize)))
 return;
 
   auto Err = Replaces.add(tooling::Replacement(
@@ -2044,7 +2053,8 @@
 
   // If the imports are out of order, we generate a single replacement fixing
   // the entire block. Otherwise, no replacement is generated.
-  if (result == Code.substr(Imports.front().Offset, ImportsBlockSize))
+  if (compareIgnoringCarriageReturns(
+  result, Code.substr(Imports.front().Offset, ImportsBlockSize)))
 return;
 
   auto Err = Replaces.add(tooling::Replacement(FileName, 
Imports.front().Offset,
@@ -2334,7 +2344,7 @@
 
   auto Env =
   std::make_unique(Code, FileName, Ranges, FirstStartColumn,
- NextStartColumn, LastStartColumn);
+NextStartColumn, LastStartColumn);
   llvm::Optional CurrentCode = None;
   tooling::Replacements Fixes;
   unsigned Penalty = 0;


Index: clang/unittests/Format/SortIncludesTest.cpp
===
--- 

[PATCH] D67592: [Clang] Use -main-file-name for source filename if not set

[Teresa Johnson via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL373217: [Clang] Use -main-file-name for source filename if 
not set (authored by tejohnson, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D67592?vs=51=222437#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D67592

Files:
  cfe/trunk/include/clang/Driver/CC1Options.td
  cfe/trunk/lib/CodeGen/ModuleBuilder.cpp
  cfe/trunk/test/Frontend/stdin-input.c


Index: cfe/trunk/include/clang/Driver/CC1Options.td
===
--- cfe/trunk/include/clang/Driver/CC1Options.td
+++ cfe/trunk/include/clang/Driver/CC1Options.td
@@ -687,7 +687,7 @@
 def version : Flag<["-"], "version">,
   HelpText<"Print the compiler version">;
 def main_file_name : Separate<["-"], "main-file-name">,
-  HelpText<"Main file name to use for debug info">;
+  HelpText<"Main file name to use for debug info and source if missing">;
 def split_dwarf_output : Separate<["-"], "split-dwarf-output">,
   HelpText<"File name to use for split dwarf debug info output">;
 
Index: cfe/trunk/test/Frontend/stdin-input.c
===
--- cfe/trunk/test/Frontend/stdin-input.c
+++ cfe/trunk/test/Frontend/stdin-input.c
@@ -0,0 +1,7 @@
+// RUN: cat %s | %clang -emit-llvm -g -S \
+// RUN: -Xclang -main-file-name -Xclang test/foo.c -x c - -o - | FileCheck %s
+// CHECK: ; ModuleID = 'test/foo.c'
+// CHECK: source_filename = "test/foo.c"
+// CHECK: !1 = !DIFile(filename: "test/foo.c"
+
+int main() {}
Index: cfe/trunk/lib/CodeGen/ModuleBuilder.cpp
===
--- cfe/trunk/lib/CodeGen/ModuleBuilder.cpp
+++ cfe/trunk/lib/CodeGen/ModuleBuilder.cpp
@@ -65,6 +65,13 @@
   private:
 SmallVector DeferredInlineMemberFuncDefs;
 
+static llvm::StringRef ExpandModuleName(llvm::StringRef ModuleName,
+const CodeGenOptions ) {
+  if (ModuleName == "-" && !CGO.MainFileName.empty())
+return CGO.MainFileName;
+  return ModuleName;
+}
+
   public:
 CodeGeneratorImpl(DiagnosticsEngine , llvm::StringRef ModuleName,
   const HeaderSearchOptions ,
@@ -73,7 +80,8 @@
   CoverageSourceInfo *CoverageInfo = nullptr)
 : Diags(diags), Ctx(nullptr), HeaderSearchOpts(HSO),
   PreprocessorOpts(PPO), CodeGenOpts(CGO), HandlingTopLevelDecls(0),
-  CoverageInfo(CoverageInfo), M(new llvm::Module(ModuleName, C)) {
+  CoverageInfo(CoverageInfo),
+  M(new llvm::Module(ExpandModuleName(ModuleName, CGO), C)) {
   C.setDiscardValueNames(CGO.DiscardValueNames);
 }
 
@@ -121,7 +129,7 @@
 llvm::Module *StartModule(llvm::StringRef ModuleName,
   llvm::LLVMContext ) {
   assert(!M && "Replacing existing Module?");
-  M.reset(new llvm::Module(ModuleName, C));
+  M.reset(new llvm::Module(ExpandModuleName(ModuleName, CodeGenOpts), C));
   Initialize(*Ctx);
   return M.get();
 }


Index: cfe/trunk/include/clang/Driver/CC1Options.td
===
--- cfe/trunk/include/clang/Driver/CC1Options.td
+++ cfe/trunk/include/clang/Driver/CC1Options.td
@@ -687,7 +687,7 @@
 def version : Flag<["-"], "version">,
   HelpText<"Print the compiler version">;
 def main_file_name : Separate<["-"], "main-file-name">,
-  HelpText<"Main file name to use for debug info">;
+  HelpText<"Main file name to use for debug info and source if missing">;
 def split_dwarf_output : Separate<["-"], "split-dwarf-output">,
   HelpText<"File name to use for split dwarf debug info output">;
 
Index: cfe/trunk/test/Frontend/stdin-input.c
===
--- cfe/trunk/test/Frontend/stdin-input.c
+++ cfe/trunk/test/Frontend/stdin-input.c
@@ -0,0 +1,7 @@
+// RUN: cat %s | %clang -emit-llvm -g -S \
+// RUN: -Xclang -main-file-name -Xclang test/foo.c -x c - -o - | FileCheck %s
+// CHECK: ; ModuleID = 'test/foo.c'
+// CHECK: source_filename = "test/foo.c"
+// CHECK: !1 = !DIFile(filename: "test/foo.c"
+
+int main() {}
Index: cfe/trunk/lib/CodeGen/ModuleBuilder.cpp
===
--- cfe/trunk/lib/CodeGen/ModuleBuilder.cpp
+++ cfe/trunk/lib/CodeGen/ModuleBuilder.cpp
@@ -65,6 +65,13 @@
   private:
 SmallVector DeferredInlineMemberFuncDefs;
 
+static llvm::StringRef ExpandModuleName(llvm::StringRef ModuleName,
+const CodeGenOptions ) {
+  if (ModuleName == "-" && !CGO.MainFileName.empty())
+return CGO.MainFileName;
+  return ModuleName;
+}
+
   public:
 CodeGeneratorImpl(DiagnosticsEngine , 

[PATCH] D68021: [IntrinsicEmitter] Add overloaded type VecOfBitcastsToInt for SVE intrinsics

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes accepted this revision.
c-rhodes added a comment.
This revision is now accepted and ready to land.

LGTM

Looks like a pretty straightforward change. Might be worth waiting a day or so 
before committing incase anyone else has any comments.


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

https://reviews.llvm.org/D68021



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

r373217 - [Clang] Use -main-file-name for source filename if not set

[Teresa Johnson via cfe-commits]

Author: tejohnson
Date: Mon Sep 30 08:05:35 2019
New Revision: 373217

URL: http://llvm.org/viewvc/llvm-project?rev=373217=rev
Log:
[Clang] Use -main-file-name for source filename if not set

-main-file-name is currently used to set the source name used in debug
information.

If the source filename is "-" and -main-file-name is set, then use the
filename also for source_filename and ModuleID of the output.

The argument is generally used outside the internal clang calls when
running clang in a wrapper like icecc which gives the source via stdin
but still wants to get a object file with the original source filename
both in debug info and IR code.

Patch by: the_jk (Joel Klinghed)

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

Added:
cfe/trunk/test/Frontend/stdin-input.c
Modified:
cfe/trunk/include/clang/Driver/CC1Options.td
cfe/trunk/lib/CodeGen/ModuleBuilder.cpp

Modified: cfe/trunk/include/clang/Driver/CC1Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/CC1Options.td?rev=373217=373216=373217=diff
==
--- cfe/trunk/include/clang/Driver/CC1Options.td (original)
+++ cfe/trunk/include/clang/Driver/CC1Options.td Mon Sep 30 08:05:35 2019
@@ -687,7 +687,7 @@ let Flags = [CC1Option, CC1AsOption, NoD
 def version : Flag<["-"], "version">,
   HelpText<"Print the compiler version">;
 def main_file_name : Separate<["-"], "main-file-name">,
-  HelpText<"Main file name to use for debug info">;
+  HelpText<"Main file name to use for debug info and source if missing">;
 def split_dwarf_output : Separate<["-"], "split-dwarf-output">,
   HelpText<"File name to use for split dwarf debug info output">;
 

Modified: cfe/trunk/lib/CodeGen/ModuleBuilder.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/ModuleBuilder.cpp?rev=373217=373216=373217=diff
==
--- cfe/trunk/lib/CodeGen/ModuleBuilder.cpp (original)
+++ cfe/trunk/lib/CodeGen/ModuleBuilder.cpp Mon Sep 30 08:05:35 2019
@@ -65,6 +65,13 @@ namespace {
   private:
 SmallVector DeferredInlineMemberFuncDefs;
 
+static llvm::StringRef ExpandModuleName(llvm::StringRef ModuleName,
+const CodeGenOptions ) {
+  if (ModuleName == "-" && !CGO.MainFileName.empty())
+return CGO.MainFileName;
+  return ModuleName;
+}
+
   public:
 CodeGeneratorImpl(DiagnosticsEngine , llvm::StringRef ModuleName,
   const HeaderSearchOptions ,
@@ -73,7 +80,8 @@ namespace {
   CoverageSourceInfo *CoverageInfo = nullptr)
 : Diags(diags), Ctx(nullptr), HeaderSearchOpts(HSO),
   PreprocessorOpts(PPO), CodeGenOpts(CGO), HandlingTopLevelDecls(0),
-  CoverageInfo(CoverageInfo), M(new llvm::Module(ModuleName, C)) {
+  CoverageInfo(CoverageInfo),
+  M(new llvm::Module(ExpandModuleName(ModuleName, CGO), C)) {
   C.setDiscardValueNames(CGO.DiscardValueNames);
 }
 
@@ -121,7 +129,7 @@ namespace {
 llvm::Module *StartModule(llvm::StringRef ModuleName,
   llvm::LLVMContext ) {
   assert(!M && "Replacing existing Module?");
-  M.reset(new llvm::Module(ModuleName, C));
+  M.reset(new llvm::Module(ExpandModuleName(ModuleName, CodeGenOpts), C));
   Initialize(*Ctx);
   return M.get();
 }

Added: cfe/trunk/test/Frontend/stdin-input.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Frontend/stdin-input.c?rev=373217=auto
==
--- cfe/trunk/test/Frontend/stdin-input.c (added)
+++ cfe/trunk/test/Frontend/stdin-input.c Mon Sep 30 08:05:35 2019
@@ -0,0 +1,7 @@
+// RUN: cat %s | %clang -emit-llvm -g -S \
+// RUN: -Xclang -main-file-name -Xclang test/foo.c -x c - -o - | FileCheck %s
+// CHECK: ; ModuleID = 'test/foo.c'
+// CHECK: source_filename = "test/foo.c"
+// CHECK: !1 = !DIFile(filename: "test/foo.c"
+
+int main() {}


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

[PATCH] D68211: [clangd] Use the index-based API to do the header-source switch.

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

some nits, thanks!




Comment at: clang-tools-extra/clangd/ClangdServer.cpp:456
+  //  1) use the file-only heuristic, it requires some IO but it is much
+  //faster than building AST, it works when .h/.cc files are in the same
+  //directory.

AST, it works -> AST, but it only works



Comment at: clang-tools-extra/clangd/ClangdServer.cpp:456
+  //  1) use the file-only heuristic, it requires some IO but it is much
+  //faster than building AST, it works when .h/.cc files are in the same
+  //directory.

kadircet wrote:
> AST, it works -> AST, but it only works
indent one more column



Comment at: clang-tools-extra/clangd/ClangdServer.cpp:458
+  //directory.
+  //  2) if 1) fails, we use the AST approach, it is slower but works for
+  // different code layout.

s/works for/supports


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68211



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

[PATCH] D68024: [clangd] Implement GetEligiblePoints

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 222434.
kadircet marked 7 inline comments as done.
kadircet added a comment.

- Address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68024

Files:
  clang-tools-extra/clangd/SourceCode.cpp
  clang-tools-extra/clangd/SourceCode.h
  clang-tools-extra/clangd/unittests/SourceCodeTests.cpp

Index: clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
===
--- clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
+++ clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
@@ -19,6 +19,7 @@
 #include "llvm/Testing/Support/Error.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
+#include 
 
 namespace clang {
 namespace clangd {
@@ -603,6 +604,66 @@
 Test.llvm::Annotations::point("bar"));
 }
 
+TEST(SourceCodeTests, GetEligiblePoints) {
+  constexpr std::tuple
+  Cases[] = {
+  {R"cpp(// FIXME: We should also mark positions before and after
+ //declarations/definitions as eligible.
+  namespace ns1 {
+  namespace a { namespace ns2 {} }
+  namespace ns2 {^
+  void foo();
+  namespace {}
+  void bar() {}
+  namespace ns3 {}
+  class T {};
+  ^}
+  using namespace ns2;
+  })cpp",
+   "ns1::ns2::symbol", "ns1::ns2::"},
+  {R"cpp(
+  namespace ns1 {^
+  namespace a { namespace ns2 {} }
+  namespace b {}
+  namespace ns {}
+  ^})cpp",
+   "ns1::ns2::symbol", "ns1::"},
+  {R"cpp(
+  namespace x {
+  namespace a { namespace ns2 {} }
+  namespace b {}
+  namespace ns {}
+  }^)cpp",
+   "ns1::ns2::symbol", ""},
+  {R"cpp(
+  namespace ns1 {
+  namespace ns2 {^^}
+  namespace b {}
+  namespace ns2 {^^}
+  }
+  namespace ns1 {namespace ns2 {^^}})cpp",
+   "ns1::ns2::symbol", "ns1::ns2::"},
+  {R"cpp(
+  namespace ns1 {^
+  namespace ns {}
+  namespace b {}
+  namespace ns {}
+  ^}
+  namespace ns1 {^namespace ns {}^})cpp",
+   "ns1::ns2::symbol", "ns1::"},
+  };
+  for (auto Case : Cases) {
+Annotations Test(std::get<0>(Case));
+
+auto Res = getEligiblePoints(Test.code(), std::get<1>(Case),
+ format::getLLVMStyle());
+EXPECT_THAT(Res.EligiblePoints, testing::ElementsAreArray(Test.points()))
+<< Test.code();
+EXPECT_EQ(Res.EnclosingNamespace, std::get<2>(Case)) << Test.code();
+  }
+}
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/SourceCode.h
===
--- clang-tools-extra/clangd/SourceCode.h
+++ clang-tools-extra/clangd/SourceCode.h
@@ -262,6 +262,27 @@
 std::vector visibleNamespaces(llvm::StringRef Code,
const format::FormatStyle );
 
+/// Represents locations that can accept a definition.
+struct EligibleRegion {
+  /// Namespace that owns all of the EligiblePoints, e.g.
+  /// namespace a{ namespace b {^ void foo();^} }
+  /// It will be “a::b” for both carrot locations.
+  std::string EnclosingNamespace;
+  /// Offsets into the code marking eligible points to insert a function
+  /// definition.
+  std::vector EligiblePoints;
+};
+
+/// Returns most eligible region to insert a definition for \p
+/// FullyQualifiedName in the \p Code.
+/// Pseudo parses \pCode under the hood to determine namespace decls and
+/// possible insertion points. Choses the region that matches the longest prefix
+/// of \p FullyQualifiedName. Returns EOF if there are no shared namespaces.
+/// \p FullyQualifiedName should not contain anonymous namespaces.
+EligibleRegion getEligiblePoints(llvm::StringRef Code,
+ llvm::StringRef FullyQualifiedName,
+ const format::FormatStyle );
+
 struct DefinedMacro {
   llvm::StringRef Name;
   const MacroInfo *Info;
Index: clang-tools-extra/clangd/SourceCode.cpp
===
--- clang-tools-extra/clangd/SourceCode.cpp
+++ clang-tools-extra/clangd/SourceCode.cpp
@@ -20,9 +20,11 @@
 #include "clang/Format/Format.h"
 #include "clang/Lex/Lexer.h"
 #include "clang/Lex/Preprocessor.h"
+#include "clang/Lex/Token.h"
 #include "clang/Tooling/Core/Replacement.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/None.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
@@ -37,6 +39,9 @@
 #include 

[PATCH] D68024: [clangd] Implement GetEligiblePoints

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: clang-tools-extra/clangd/SourceCode.h:270
+  /// It will be “a::b” for both carrot locations.
+  std::string CurrentNamespace;
+  /// Offsets into the code marking eligible points to insert a function

ilya-biryukov wrote:
> Do we ever name anonymous namespace here? How do we represent them?
> Do we ever name anonymous namespace here? How do we represent them?

No we don't, represents anonymous namespaces. I also updated docs to mention 
that `FullyQualifiedName` should not contain anon namespaces.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68024



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

[PATCH] D68211: [clangd] Use the index-based API to do the header-source switch.

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 222432.
hokein marked 4 inline comments as done.
hokein added a comment.

address comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68211

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/unittests/HeaderSourceSwitchTests.cpp
  clang-tools-extra/clangd/unittests/SyncAPI.cpp
  clang-tools-extra/clangd/unittests/SyncAPI.h

Index: clang-tools-extra/clangd/unittests/SyncAPI.h
===
--- clang-tools-extra/clangd/unittests/SyncAPI.h
+++ clang-tools-extra/clangd/unittests/SyncAPI.h
@@ -56,6 +56,9 @@
 llvm::Expected>
 runSemanticRanges(ClangdServer , PathRef File, Position Pos);
 
+llvm::Expected>
+runSwitchHeaderSource(ClangdServer , PathRef File);
+
 } // namespace clangd
 } // namespace clang
 
Index: clang-tools-extra/clangd/unittests/SyncAPI.cpp
===
--- clang-tools-extra/clangd/unittests/SyncAPI.cpp
+++ clang-tools-extra/clangd/unittests/SyncAPI.cpp
@@ -152,5 +152,12 @@
   return std::move(*Result);
 }
 
+llvm::Expected>
+runSwitchHeaderSource(ClangdServer , PathRef File) {
+  llvm::Optional>> Result;
+  Server.switchSourceHeader(File, capture(Result));
+  return std::move(*Result);
+}
+
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/unittests/HeaderSourceSwitchTests.cpp
===
--- clang-tools-extra/clangd/unittests/HeaderSourceSwitchTests.cpp
+++ clang-tools-extra/clangd/unittests/HeaderSourceSwitchTests.cpp
@@ -8,6 +8,7 @@
 
 #include "HeaderSourceSwitch.h"
 
+#include "SyncAPI.h"
 #include "TestFS.h"
 #include "TestTU.h"
 #include "index/MemIndex.h"
@@ -240,6 +241,32 @@
   }
 }
 
+TEST(HeaderSourceSwitchTest, ClangdServerIntegration) {
+  class IgnoreDiagnostics : public DiagnosticsConsumer {
+void onDiagnosticsReady(PathRef File,
+std::vector Diagnostics) override {}
+  } DiagConsumer;
+  MockCompilationDatabase CDB;
+  CDB.ExtraClangFlags = {"-I" +
+ testPath("src/include")}; // add search directory.
+  MockFSProvider FS;
+  // File heuristic fails here, we rely on the index to find the .h file.
+  std::string CppPath = testPath("src/lib/test.cpp");
+  std::string HeaderPath = testPath("src/include/test.h");
+  FS.Files[HeaderPath] = "void foo();";
+  const std::string FileContent = R"cpp(
+#include "test.h"
+void foo() {};
+  )cpp";
+  FS.Files[CppPath] = FileContent;
+  auto Options = ClangdServer::optsForTest();
+  Options.BuildDynamicSymbolIndex = true;
+  ClangdServer Server(CDB, FS, DiagConsumer, Options);
+  runAddDocument(Server, CppPath, FileContent);
+  EXPECT_EQ(HeaderPath,
+*llvm::cantFail(runSwitchHeaderSource(Server, CppPath)));
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/ClangdServer.h
===
--- clang-tools-extra/clangd/ClangdServer.h
+++ clang-tools-extra/clangd/ClangdServer.h
@@ -192,9 +192,10 @@
   void locateSymbolAt(PathRef File, Position Pos,
   Callback> CB);
 
-  /// Helper function that returns a path to the corresponding source file when
-  /// given a header file and vice versa.
-  llvm::Optional switchSourceHeader(PathRef Path);
+  /// Switch to a corresponding source file when given a header file, and vice
+  /// versa.
+  void switchSourceHeader(PathRef Path,
+  Callback> CB);
 
   /// Get document highlights for a given position.
   void findDocumentHighlights(PathRef File, Position Pos,
Index: clang-tools-extra/clangd/ClangdServer.cpp
===
--- clang-tools-extra/clangd/ClangdServer.cpp
+++ clang-tools-extra/clangd/ClangdServer.cpp
@@ -449,8 +449,24 @@
   WorkScheduler.runWithAST("Definitions", File, std::move(Action));
 }
 
-llvm::Optional ClangdServer::switchSourceHeader(PathRef Path) {
-  return getCorrespondingHeaderOrSource(Path, FSProvider.getFileSystem());
+void ClangdServer::switchSourceHeader(
+PathRef Path, Callback> CB) {
+  // We want to return the result as fast as possible, stragety is:
+  //  1) use the file-only heuristic, it requires some IO but it is much
+  //faster than building AST, it works when .h/.cc files are in the same
+  //directory.
+  //  2) if 1) fails, we use the AST approach, it is slower but works for
+  // different code layout.
+  if (auto CorrespondingFile =
+  getCorrespondingHeaderOrSource(Path, FSProvider.getFileSystem()))
+return CB(std::move(CorrespondingFile));
+  auto Action = [Path, CB = std::move(CB),
+ this](llvm::Expected InpAST) 

r373213 - Correct function declarations; NFC.

[Aaron Ballman via cfe-commits]

Author: aaronballman
Date: Mon Sep 30 07:43:52 2019
New Revision: 373213

URL: http://llvm.org/viewvc/llvm-project?rev=373213=rev
Log:
Correct function declarations; NFC.

This header is included by C code so the functions need to have a prototype. 
Also, fix the function definitions so that they have C linkage rather than C++ 
linkage.

Modified:
cfe/trunk/include/clang-c/FatalErrorHandler.h
cfe/trunk/tools/libclang/FatalErrorHandler.cpp

Modified: cfe/trunk/include/clang-c/FatalErrorHandler.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang-c/FatalErrorHandler.h?rev=373213=373212=373213=diff
==
--- cfe/trunk/include/clang-c/FatalErrorHandler.h (original)
+++ cfe/trunk/include/clang-c/FatalErrorHandler.h Mon Sep 30 07:43:52 2019
@@ -18,14 +18,14 @@ extern "C" {
  * Installs error handler that prints error message to stderr and calls 
abort().
  * Replaces currently installed error handler (if any).
  */
-void clang_install_aborting_llvm_fatal_error_handler();
+void clang_install_aborting_llvm_fatal_error_handler(void);
 
 /**
  * Removes currently installed error handler (if any).
  * If no error handler is intalled, the default strategy is to print error
  * message to stderr and call exit(1).
  */
-void clang_uninstall_llvm_fatal_error_handler();
+void clang_uninstall_llvm_fatal_error_handler(void);
 
 #ifdef __cplusplus
 }

Modified: cfe/trunk/tools/libclang/FatalErrorHandler.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/libclang/FatalErrorHandler.cpp?rev=373213=373212=373213=diff
==
--- cfe/trunk/tools/libclang/FatalErrorHandler.cpp (original)
+++ cfe/trunk/tools/libclang/FatalErrorHandler.cpp Mon Sep 30 07:43:52 2019
@@ -18,11 +18,13 @@ static void aborting_fatal_error_handler
   ::abort();
 }
 
-void clang_install_aborting_llvm_fatal_error_handler() {
+extern "C" {
+void clang_install_aborting_llvm_fatal_error_handler(void) {
   llvm::remove_fatal_error_handler();
   llvm::install_fatal_error_handler(aborting_fatal_error_handler, nullptr);
 }
 
-void clang_uninstall_llvm_fatal_error_handler() {
+void clang_uninstall_llvm_fatal_error_handler(void) {
   llvm::remove_fatal_error_handler();
 }
+}
\ No newline at end of file


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

[PATCH] D68211: [clangd] Use the index-based API to do the header-source switch.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

thanks, mostly LG




Comment at: clang-tools-extra/clangd/ClangdServer.cpp:453
+void ClangdServer::switchSourceHeader(
+PathRef Path, Callback> CB) {
+  if (auto CorrespondingFile =

could you add some comments explaining, why we first use file-only 
version(speed) and why try with an ast afterwards(making use of 
declarations)



Comment at: clang-tools-extra/clangd/ClangdServer.h:195
 
   /// Helper function that returns a path to the corresponding source file when
   /// given a header file and vice versa.

i don't think it is a helper function anymore



Comment at: clang-tools-extra/clangd/unittests/HeaderSourceSwitchTests.cpp:250
+  MockCompilationDatabase CDB;
+  CDB.ExtraClangFlags = {"-I../include"}; // add search directory.
+  MockFSProvider FS;

could we rather provide `testPath("src/include")` to make sure this also works 
on windows?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68211



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

[PATCH] D68024: [clangd] Implement GetEligiblePoints

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



Comment at: clang-tools-extra/clangd/SourceCode.h:270
+  /// It will be “a::b” for both carrot locations.
+  std::string CurrentNamespace;
+  /// Offsets into the code marking eligible points to insert a function

kadircet wrote:
> hokein wrote:
> > `Current` seems not clear enough, how about "MatchNamespace"?
> what about enclosing?
sounds good.



Comment at: clang-tools-extra/clangd/SourceCode.h:273
+  /// definition.
+  std::vector EligiblePoints;
+};

kadircet wrote:
> hokein wrote:
> > I'm curious how the caller use this, it seems that the caller has no/little 
> > information to distinguish all the positions, even for a simple case where 
> > we add a definition at the last eligible point (I assume just just using 
> > `EligiblePoints.back()`).
> for example a caller with access to AST and Index, could first look at the 
> decls surrounding the target(fully qualified name), then check for their 
> definition locations using index and find an eligible point between these two.
Fair enough, thanks for the explanation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68024



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

[PATCH] D67592: [Clang] Use -main-file-name for source filename if not set

[Joel Klinghed via Phabricator via cfe-commits]

the_jk added a comment.

Great, thanks again.


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

https://reviews.llvm.org/D67592



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

[PATCH] D68141: [Alignment][NFC] Remove AllocaInst::setAlignment(unsigned)

[Guillaume Chatelet via Phabricator via cfe-commits]

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



Comment at: llvm/trunk/lib/Target/AArch64/AArch64StackTagging.cpp:65
 
-static constexpr unsigned kTagGranuleSize = 16;
+static const Align kTagGranuleSize = Align(16);
 

arichardson wrote:
> Can't the Align ctor be constexpr? Will this result in a `Log2` call at 
> runtime?
The implementation of Log2 is quite complex and depends on the implementation 
of [[ 
https://github.com/llvm/llvm-project/blob/02ada9bd2b41d850876a483bede59715e7550c1e/llvm/include/llvm/Support/MathExtras.h#L127
 | countLeadingZeros ]].
GCC, clang, ICC can compute the value at compile time but there may be some 
compiler that can't.
https://godbolt.org/z/ytwWHn

I could solve this issue by introducing a `LogAlign()` function returning an 
`Align`. This function could be `constexpr`. What do you think? Is it worth the 
additional complexity?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D68141



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

[PATCH] D67592: [Clang] Use -main-file-name for source filename if not set

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson added a comment.

In D67592#1686898 , @the_jk wrote:

> I don't have commit access so I'd need someone to push this, thanks.


I can commit for you today.


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

https://reviews.llvm.org/D67592



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

[PATCH] D67695: [clangd] Implement getBeginning for overloaded operators.

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



Comment at: clang-tools-extra/clangd/SourceCode.cpp:242
+
+enum TokenKind { Identifier, Operator, Whitespace, Other };
+

ilya-biryukov wrote:
> `TokenKind` has the same name as `tok::TokenKind`. Could we use a different 
> name here to avoid possible confusions?
> E.g. `TokenGroup` or `TokenFlavor`.
renamed to `TokenFlavor`.



Comment at: clang-tools-extra/clangd/SourceCode.cpp:258
+
+TokenKind getTokenKind(SourceLocation Loc, const SourceManager ,
+   const LangOptions ) {

ilya-biryukov wrote:
> NIT: add `static` for consistency with the rest of the function.
I think the static is redundant here, as the function is in the anonymous 
namespace, I removed the `static` on the function above.



Comment at: clang-tools-extra/clangd/SourceCode.cpp:326
+
+  // Handle case 1 and 3. Note that the cursor could be at the token boundary,
+  // e.g. "Before^Current", we prefer identifiers to other tokens.

ilya-biryukov wrote:
> `could be at the token boundary` should be `is at the token boundary`, right?
> Whenever it's not the case we'll bail out on `BeforeTokBeginning == 
> CurrentTokBeginning`.
yes, you are right. I was thinking of the single token case `int ^foo;`, which 
should be counted into the boundary case as we consider whitespace as a token 
kind.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67695



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

[PATCH] D67695: [clangd] Implement getBeginning for overloaded operators.

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 222423.
hokein marked 8 inline comments as done.
hokein added a comment.

address comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67695

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

Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -441,6 +441,15 @@
   auto x = m^akeX();
 }
   )cpp",
+
+  R"cpp(
+struct X {
+  X& [[operator]]++() {}
+};
+void foo(X& x) {
+  +^+x;
+}
+  )cpp",
   };
   for (const char *Test : Tests) {
 Annotations T(Test);
Index: clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
===
--- clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
+++ clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
@@ -319,14 +319,28 @@
 Bar* bar;
   )cpp";
   // First ^ is the expected beginning, last is the search position.
-  for (std::string Text : std::vector{
+  for (const std::string  : std::vector{
"int ^f^oo();", // inside identifier
"int ^foo();",  // beginning of identifier
"int ^foo^();", // end of identifier
"int foo(^);",  // non-identifier
"^int foo();",  // beginning of file (can't back up)
"int ^f0^0();", // after a digit (lexing at N-1 is wrong)
-   "int ^λλ^λ();", // UTF-8 handled properly when backing up
+   "/^/ comments", // non-interesting token
+   "void f(int abc) { abc ^ ++; }",// whitespace
+   "void f(int abc) { ^abc^++; }", // range of identifier
+   "void f(int abc) { ++^abc^; }", // range of identifier
+   "void f(int abc) { ^+^+abc; }", // range of operator
+   "void f(int abc) { ^abc^ ++; }",// range of identifier
+   "void f(int abc) { abc ^++^; }",// range of operator
+   "void f(int abc) { ^++^ abc; }",// range of operator
+   "void f(int abc) { ++ ^abc^; }",// range of identifier
+   "void f(int abc) { ^++^/**/abc; }", // range of operator
+   "void f(int abc) { ++/**/^abc; }",  // range of identifier
+   "void f(int abc) { ^abc^/**/++; }", // range of identifier
+   "void f(int abc) { abc/**/^++; }",  // range of operator
+   "void f() {^ }", // outside of identifier and operator
+   "int ^λλ^λ();",  // UTF-8 handled properly when backing up
 
// identifier in macro arg
"MACRO(bar->^func())",  // beginning of identifier
Index: clang-tools-extra/clangd/SourceCode.cpp
===
--- clang-tools-extra/clangd/SourceCode.cpp
+++ clang-tools-extra/clangd/SourceCode.cpp
@@ -237,6 +237,45 @@
   return halfOpenToRange(SM, CharSourceRange::getCharRange(TokLoc, End));
 }
 
+namespace {
+
+enum TokenFlavor { Identifier, Operator, Whitespace, Other };
+
+bool isOverloadedOperator(const Token ) {
+  switch (Tok.getKind()) {
+#define OVERLOADED_OPERATOR(Name, Spelling, Token, Unary, Binary, MemOnly) \
+  case tok::Token:
+#define OVERLOADED_OPERATOR_MULTI(Name, Spelling, Unary, Binary, MemOnly)
+#include "clang/Basic/OperatorKinds.def"
+return true;
+
+  default:
+break;
+  }
+  return false;
+}
+
+TokenFlavor getTokenFlavor(SourceLocation Loc, const SourceManager ,
+   const LangOptions ) {
+  Token Tok;
+  Tok.setKind(tok::NUM_TOKENS);
+  if (Lexer::getRawToken(Loc, Tok, SM, LangOpts,
+ /*IgnoreWhiteSpace*/ false))
+return Other;
+
+  // getRawToken will return false without setting Tok when the token is
+  // whitespace, so if the flag is not set, we are sure this is a whitespace.
+  if (Tok.is(tok::TokenKind::NUM_TOKENS))
+return Whitespace;
+  if (Tok.is(tok::TokenKind::raw_identifier))
+return Identifier;
+  if (isOverloadedOperator(Tok))
+return Operator;
+  return Other;
+}
+
+} // namespace
+
 SourceLocation getBeginningOfIdentifier(const Position ,
 const SourceManager ,
 const LangOptions ) {
@@ -247,27 +286,57 @@
 return SourceLocation();
   }
 
-  // GetBeginningOfToken(pos) is almost what we want, but does the wrong thing
-  // if the cursor is at the end of the identifier.
-  // Instead, we lex at GetBeginningOfToken(pos - 1). The cases are:
-  //  1) at the beginning of an identifier, we'll be looking at something
-  //  that isn't an identifier.
-  //  2) at the middle or end of an identifier, we get the identifier.
-  //  3) 

r373210 - [OPENMP] Fix comment, NFC.

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Mon Sep 30 07:05:26 2019
New Revision: 373210

URL: http://llvm.org/viewvc/llvm-project?rev=373210=rev
Log:
[OPENMP] Fix comment, NFC.

Modified:
cfe/trunk/lib/Sema/SemaOpenMP.cpp

Modified: cfe/trunk/lib/Sema/SemaOpenMP.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOpenMP.cpp?rev=373210=373209=373210=diff
==
--- cfe/trunk/lib/Sema/SemaOpenMP.cpp (original)
+++ cfe/trunk/lib/Sema/SemaOpenMP.cpp Mon Sep 30 07:05:26 2019
@@ -170,7 +170,7 @@ private:
   OpenMPClauseKind ClauseKindMode = OMPC_unknown;
   Sema 
   bool ForceCapturing = false;
-  /// true if all the vaiables in the target executable directives must be
+  /// true if all the variables in the target executable directives must be
   /// captured by reference.
   bool ForceCaptureByReferenceInTargetExecutable = false;
   CriticalsWithHintsTy Criticals;


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

[PATCH] D68141: [Alignment][NFC] Remove AllocaInst::setAlignment(unsigned)

[Alexander Richardson via Phabricator via cfe-commits]

arichardson added inline comments.



Comment at: llvm/trunk/lib/Target/AArch64/AArch64StackTagging.cpp:65
 
-static constexpr unsigned kTagGranuleSize = 16;
+static const Align kTagGranuleSize = Align(16);
 

Can't the Align ctor be constexpr? Will this result in a `Log2` call at runtime?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D68141



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

[PATCH] D68141: [Alignment][NFC] Remove AllocaInst::setAlignment(unsigned)

[Guillaume Chatelet via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL373207: [Alignment][NFC] Remove 
AllocaInst::setAlignment(unsigned) (authored by gchatelet, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D68141?vs=222400=222409#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D68141

Files:
  cfe/trunk/lib/CodeGen/CGBuiltin.cpp
  cfe/trunk/lib/CodeGen/CGCall.cpp
  cfe/trunk/lib/CodeGen/CGExpr.cpp
  cfe/trunk/lib/CodeGen/TargetInfo.cpp
  llvm/trunk/include/llvm/IR/Instructions.h
  llvm/trunk/lib/CodeGen/AtomicExpandPass.cpp
  llvm/trunk/lib/CodeGen/CodeGenPrepare.cpp
  llvm/trunk/lib/IR/Core.cpp
  llvm/trunk/lib/IR/Instructions.cpp
  llvm/trunk/lib/Target/AArch64/AArch64StackTagging.cpp
  llvm/trunk/lib/Target/AMDGPU/AMDGPULibCalls.cpp
  llvm/trunk/lib/Target/NVPTX/NVPTXLowerArgs.cpp
  llvm/trunk/lib/Transforms/Coroutines/CoroFrame.cpp
  llvm/trunk/lib/Transforms/IPO/Inliner.cpp
  llvm/trunk/lib/Transforms/InstCombine/InstCombineCasts.cpp
  llvm/trunk/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
  llvm/trunk/lib/Transforms/Instrumentation/AddressSanitizer.cpp
  llvm/trunk/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
  llvm/trunk/lib/Transforms/Scalar/GVNHoist.cpp
  llvm/trunk/lib/Transforms/Scalar/MemCpyOptimizer.cpp
  llvm/trunk/lib/Transforms/Utils/Local.cpp
  polly/trunk/lib/CodeGen/IslNodeBuilder.cpp

Index: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
===
--- cfe/trunk/lib/CodeGen/CGBuiltin.cpp
+++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp
@@ -2366,7 +2366,7 @@
 .toCharUnitsFromBits(TI.getSuitableAlign())
 .getQuantity();
 AllocaInst *AI = Builder.CreateAlloca(Builder.getInt8Ty(), Size);
-AI->setAlignment(SuitableAlignmentInBytes);
+AI->setAlignment(MaybeAlign(SuitableAlignmentInBytes));
 initializeAlloca(*this, AI, Size, SuitableAlignmentInBytes);
 return RValue::get(AI);
   }
@@ -2379,7 +2379,7 @@
 unsigned AlignmentInBytes =
 CGM.getContext().toCharUnitsFromBits(AlignmentInBits).getQuantity();
 AllocaInst *AI = Builder.CreateAlloca(Builder.getInt8Ty(), Size);
-AI->setAlignment(AlignmentInBytes);
+AI->setAlignment(MaybeAlign(AlignmentInBytes));
 initializeAlloca(*this, AI, Size, AlignmentInBytes);
 return RValue::get(AI);
   }
Index: cfe/trunk/lib/CodeGen/CGExpr.cpp
===
--- cfe/trunk/lib/CodeGen/CGExpr.cpp
+++ cfe/trunk/lib/CodeGen/CGExpr.cpp
@@ -66,7 +66,7 @@
  const Twine ,
  llvm::Value *ArraySize) {
   auto Alloca = CreateTempAlloca(Ty, Name, ArraySize);
-  Alloca->setAlignment(Align.getQuantity());
+  Alloca->setAlignment(llvm::MaybeAlign(Align.getQuantity()));
   return Address(Alloca, Align);
 }
 
Index: cfe/trunk/lib/CodeGen/CGCall.cpp
===
--- cfe/trunk/lib/CodeGen/CGCall.cpp
+++ cfe/trunk/lib/CodeGen/CGCall.cpp
@@ -3841,7 +3841,7 @@
   AI = CreateTempAlloca(ArgStruct, "argmem");
 }
 auto Align = CallInfo.getArgStructAlignment();
-AI->setAlignment(Align.getQuantity());
+AI->setAlignment(llvm::MaybeAlign(Align.getQuantity()));
 AI->setUsedWithInAlloca(true);
 assert(AI->isUsedWithInAlloca() && !AI->isStaticAlloca());
 ArgMemory = Address(AI, Align);
Index: cfe/trunk/lib/CodeGen/TargetInfo.cpp
===
--- cfe/trunk/lib/CodeGen/TargetInfo.cpp
+++ cfe/trunk/lib/CodeGen/TargetInfo.cpp
@@ -9954,7 +9954,7 @@
   Builder.SetInsertPoint(BB);
   unsigned BlockAlign = CGF.CGM.getDataLayout().getPrefTypeAlignment(BlockTy);
   auto *BlockPtr = Builder.CreateAlloca(BlockTy, nullptr);
-  BlockPtr->setAlignment(BlockAlign);
+  BlockPtr->setAlignment(llvm::MaybeAlign(BlockAlign));
   Builder.CreateAlignedStore(F->arg_begin(), BlockPtr, BlockAlign);
   auto *Cast = Builder.CreatePointerCast(BlockPtr, InvokeFT->getParamType(0));
   llvm::SmallVector Args;
Index: llvm/trunk/include/llvm/IR/Instructions.h
===
--- llvm/trunk/include/llvm/IR/Instructions.h
+++ llvm/trunk/include/llvm/IR/Instructions.h
@@ -114,8 +114,6 @@
   return MA->value();
 return 0;
   }
-  // FIXME: Remove once migration to Align is over.
-  void setAlignment(unsigned Align);
   void setAlignment(MaybeAlign Align);
 
   /// Return true if this alloca is in the entry block of the function and is a
Index: llvm/trunk/lib/Target/AArch64/AArch64StackTagging.cpp
===
--- llvm/trunk/lib/Target/AArch64/AArch64StackTagging.cpp
+++ llvm/trunk/lib/Target/AArch64/AArch64StackTagging.cpp
@@ -62,7 +62,7 @@
 static cl::opt 

[PATCH] D65543: [Windows] Autolink with basenames and add libdir to libpath

[Russell Gallop via Phabricator via cfe-commits]

russell.gallop added a comment.

> The other thing worth checking is the clang PGO self-host on Windows.
>  This has the potential to break that, and the fix would be to add a linker 
> flag in LLVM's cmake.

This does indeed break PGO self-host with lld-link (applied on top of r373200):

  <...>\bin\lld-link.exe /nologo utils\not\CMakeFiles\not.dir\not.cpp.obj 
utils\not\CMakeFiles\not.dir\__\__\resources\windows_version_resource.rc.res 
/out:bin\not.exe /implib:lib\not.lib /pdb:bin\not.pdb /version:0.0 /machine:x64 
-fuse-ld=lld /STACK:1000 /INCREMENTAL:NO /subsystem:console 
lib\LLVMSupport.lib psapi.lib shell32.lib ole32.lib uuid.lib advapi32.lib 
delayimp.lib -delayload:shell32.dll -delayload:ole32.dll lib\LLVMDemangle.lib 
kernel32.lib user32.lib gdi32.lib winspool.lib shell32.lib ole32.lib 
oleaut32.lib uuid.lib comdlg32.lib advapi32.lib /MANIFEST 
/MANIFESTFILE:bin\not.exe.manifest
  lld-link: warning: ignoring unknown argument '-fuse-ld=lld'
  lld-link: error: could not open 'clang_rt.profile-x86_64.lib': no such file 
or directory

So at the very least this change would need that path adding.

I agree with Nico that having to add a path dependent on the LLVM version 
sounds like a pain.

Is it possible for the compiler to embed a /libpath as well as the dependent 
lib? That goes back to having a path embedded, though you could override it if 
required so could be an improvement over things at the moment.

It sounds to me like:

> ...folks have to explicitly pass 
> /libpath:\path\to\clang\lib\clang\$changing_version\lib\windows to the 
> linker, which to me is a pretty poor experience

and

> I'd like to get away from having paths embedded in the object if possible.

are hard to reconcile.

It may be possible to have the Windows installer add the path to LIB 
environment variable but that would rely on having run the installer, and could 
cause problems if you have multiple versions of LLVM around.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65543



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

r373207 - [Alignment][NFC] Remove AllocaInst::setAlignment(unsigned)

[Guillaume Chatelet via cfe-commits]

Author: gchatelet
Date: Mon Sep 30 06:34:44 2019
New Revision: 373207

URL: http://llvm.org/viewvc/llvm-project?rev=373207=rev
Log:
[Alignment][NFC] Remove AllocaInst::setAlignment(unsigned)

Summary:
This is patch is part of a series to introduce an Alignment type.
See this thread for context: 
http://lists.llvm.org/pipermail/llvm-dev/2019-July/133851.html
See this patch for the introduction of the type: https://reviews.llvm.org/D64790

Reviewers: courbet

Subscribers: jholewinski, arsenm, jvesely, nhaehnle, eraman, hiraditya, 
cfe-commits, llvm-commits

Tags: #clang, #llvm

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

Modified:
cfe/trunk/lib/CodeGen/CGBuiltin.cpp
cfe/trunk/lib/CodeGen/CGCall.cpp
cfe/trunk/lib/CodeGen/CGExpr.cpp
cfe/trunk/lib/CodeGen/TargetInfo.cpp

Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=373207=373206=373207=diff
==
--- cfe/trunk/lib/CodeGen/CGBuiltin.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp Mon Sep 30 06:34:44 2019
@@ -2366,7 +2366,7 @@ RValue CodeGenFunction::EmitBuiltinExpr(
 .toCharUnitsFromBits(TI.getSuitableAlign())
 .getQuantity();
 AllocaInst *AI = Builder.CreateAlloca(Builder.getInt8Ty(), Size);
-AI->setAlignment(SuitableAlignmentInBytes);
+AI->setAlignment(MaybeAlign(SuitableAlignmentInBytes));
 initializeAlloca(*this, AI, Size, SuitableAlignmentInBytes);
 return RValue::get(AI);
   }
@@ -2379,7 +2379,7 @@ RValue CodeGenFunction::EmitBuiltinExpr(
 unsigned AlignmentInBytes =
 CGM.getContext().toCharUnitsFromBits(AlignmentInBits).getQuantity();
 AllocaInst *AI = Builder.CreateAlloca(Builder.getInt8Ty(), Size);
-AI->setAlignment(AlignmentInBytes);
+AI->setAlignment(MaybeAlign(AlignmentInBytes));
 initializeAlloca(*this, AI, Size, AlignmentInBytes);
 return RValue::get(AI);
   }

Modified: cfe/trunk/lib/CodeGen/CGCall.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCall.cpp?rev=373207=373206=373207=diff
==
--- cfe/trunk/lib/CodeGen/CGCall.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGCall.cpp Mon Sep 30 06:34:44 2019
@@ -3841,7 +3841,7 @@ RValue CodeGenFunction::EmitCall(const C
   AI = CreateTempAlloca(ArgStruct, "argmem");
 }
 auto Align = CallInfo.getArgStructAlignment();
-AI->setAlignment(Align.getQuantity());
+AI->setAlignment(llvm::MaybeAlign(Align.getQuantity()));
 AI->setUsedWithInAlloca(true);
 assert(AI->isUsedWithInAlloca() && !AI->isStaticAlloca());
 ArgMemory = Address(AI, Align);

Modified: cfe/trunk/lib/CodeGen/CGExpr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExpr.cpp?rev=373207=373206=373207=diff
==
--- cfe/trunk/lib/CodeGen/CGExpr.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGExpr.cpp Mon Sep 30 06:34:44 2019
@@ -66,7 +66,7 @@ Address CodeGenFunction::CreateTempAlloc
  const Twine ,
  llvm::Value *ArraySize) {
   auto Alloca = CreateTempAlloca(Ty, Name, ArraySize);
-  Alloca->setAlignment(Align.getQuantity());
+  Alloca->setAlignment(llvm::MaybeAlign(Align.getQuantity()));
   return Address(Alloca, Align);
 }
 

Modified: cfe/trunk/lib/CodeGen/TargetInfo.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/TargetInfo.cpp?rev=373207=373206=373207=diff
==
--- cfe/trunk/lib/CodeGen/TargetInfo.cpp (original)
+++ cfe/trunk/lib/CodeGen/TargetInfo.cpp Mon Sep 30 06:34:44 2019
@@ -9954,7 +9954,7 @@ llvm::Function *AMDGPUTargetCodeGenInfo:
   Builder.SetInsertPoint(BB);
   unsigned BlockAlign = CGF.CGM.getDataLayout().getPrefTypeAlignment(BlockTy);
   auto *BlockPtr = Builder.CreateAlloca(BlockTy, nullptr);
-  BlockPtr->setAlignment(BlockAlign);
+  BlockPtr->setAlignment(llvm::MaybeAlign(BlockAlign));
   Builder.CreateAlignedStore(F->arg_begin(), BlockPtr, BlockAlign);
   auto *Cast = Builder.CreatePointerCast(BlockPtr, InvokeFT->getParamType(0));
   llvm::SmallVector Args;


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

[PATCH] D68213: [LTO] Support for embedding bitcode section during LTO

[Josef Eisl via Phabricator via cfe-commits]

zapster marked 5 inline comments as done.
zapster added a comment.

Added inline remarks.




Comment at: clang/lib/CodeGen/BackendUtil.cpp:1547
 }
 
-static const char* getSectionNameForBitcode(const Triple ) {

moved to `llvm/lib/Bitcode/Writer/BitcodeWriter.cpp`



Comment at: clang/test/Frontend/x86-embed-bitcode.ll:1
+; REQUIRES: x86-registered-target
+; check .ll input

This duplicates the `embed-bitcode.ll` test (which only runs on ARM) for x86.



Comment at: llvm/include/llvm/Bitcode/BitcodeWriter.h:158
+bool EmbedMarker,
+const std::vector *CmdArgs);
+

`BitcodeWriter.h` seems like a natural place for this functionality. However, 
suggestions for a better location are more than appreciated. 



Comment at: llvm/lib/Bitcode/Writer/BitcodeWriter.cpp:4670
 }
+
+static const char *getSectionNameForBitcode(const Triple ) {

moved from `clang/lib/CodeGen/BackendUtil.cpp`



Comment at: llvm/lib/LTO/LTOBackend.cpp:327
+
+static EmbedBitcodeKind getEmbedBitcode(Config ) {
+  if (EmbedBitcode.empty())

This options parsing logic is duplicated from clang. We might want move this to 
a shared place, but I failed to find a good candidate. 
`include/llvm/Support/CodeGen.h` came to mind, but it currently only contains 
types, no code. Any suggestions?


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

https://reviews.llvm.org/D68213



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

[PATCH] D68213: [LTO] Support for embedding bitcode section during LTO

[Josef Eisl via Phabricator via cfe-commits]

zapster created this revision.
zapster added reviewers: LLVM, clang, rsmith, pcc.
Herald added subscribers: llvm-commits, cfe-commits, dang, dexonsmith, 
steven_wu, aheejin, hiraditya, inglorion, mehdi_amini.
Herald added a reviewer: alexshap.
Herald added projects: clang, LLVM.
zapster updated this revision to Diff 222401.
zapster added a comment.

re-ran clang format


This adds support for embedding bitcode in a binary during LTO. The libLTO 
gains supports the `-lto-embed-bitcode` option which accepts `off`, `all`, 
`bitcode`, and `marker`. The semantics are the same as for clangs 
`-fembed-bitcode`. The option allows users of the LTO library to embed a 
bitcode section. For example, LLD can pass the option via `ld.lld 
-mllvm=-lto-embed-bitcode=all`.

This feature allows doing something comparable to `clang -c -fembed-bitcode`, 
but on the (LTO) linker level. Having bitcode alongside native code has many 
use-cases. To give an example, the MacOS linker can create a `-bitcode_bundle` 
section containing bitcode. Also, having this feature built into LLVM is an 
alternative to 3rd party tools such as wllvm 
 or gllvm 
. As with these tools, this feature 
simplifies creating "whole-program" llvm bitcode files, but in contrast to 
wllvm/gllvm it does not rely on a specific llvm frontend/driver.

I originally proposed this feature as an addition to LLD 
. It turned 
out, however, that doing this purely on LLVM/LTO side is more general and might 
be useful to a broader audience.

The implementation is quite straight forward. The embedding logic moved from 
clang to llvm/lib/Bitcode and llvm/lib/LTO gained the `-lto-embed-bitcode` 
option. Most code just moved.


https://reviews.llvm.org/D68213

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/test/Frontend/x86-embed-bitcode.ll
  llvm/include/llvm/Bitcode/BitcodeWriter.h
  llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
  llvm/lib/LTO/LTOBackend.cpp
  llvm/test/LTO/X86/Inputs/start-lib1.ll
  llvm/test/LTO/X86/Inputs/start-lib2.ll
  llvm/test/LTO/X86/embed-bitcode.ll

Index: llvm/test/LTO/X86/embed-bitcode.ll
===
--- /dev/null
+++ llvm/test/LTO/X86/embed-bitcode.ll
@@ -0,0 +1,33 @@
+; RUN: llvm-as %s -o %t1.o
+; RUN: llvm-as %p/Inputs/start-lib1.ll -o %t2.o
+; RUN: llvm-as %p/Inputs/start-lib2.ll -o %t3.o
+
+; RUN: llvm-lto2 run -r %t1.o,_start,px -r %t2.o,foo,px -r %t3.o,bar,px -r %t2.o,bar,px -lto-embed-bitcode=off -o %t3 %t1.o %t2.o %t3.o
+; RUN: llvm-readelf -S %t3.0 | FileCheck %s --implicit-check-not=.llvmbc
+
+; RUN: llvm-lto2 run -r %t1.o,_start,px -r %t2.o,foo,px -r %t3.o,bar,px -r %t2.o,bar,px -lto-embed-bitcode=marker -o %t3 %t1.o %t2.o %t3.o
+; RUN: llvm-readelf -S %t3.0 | FileCheck %s --check-prefix=CHECK-ELF
+
+; RUN: llvm-lto2 run -r %t1.o,_start,px -r %t2.o,foo,px -r %t3.o,bar,px -r %t2.o,bar,px -lto-embed-bitcode=bitcode -o %t3 %t1.o %t2.o %t3.o
+; RUN: llvm-readelf -S %t3.0 | FileCheck %s --check-prefix=CHECK-ELF
+; RUN: llvm-objcopy -O binary -j .llvmbc %t3.0 %t-embedded.bc
+; RUN: llvm-dis %t-embedded.bc -o - | FileCheck %s --check-prefix=CHECK-LL
+
+; RUN: llvm-lto2 run -r %t1.o,_start,px -r %t2.o,foo,px -r %t3.o,bar,px -r %t2.o,bar,px -lto-embed-bitcode=all -o %t3 %t1.o %t2.o %t3.o
+; RUN: llvm-readelf -S %t3.0 | FileCheck %s --check-prefix=CHECK-ELF
+; RUN: llvm-objcopy -O binary -j .llvmbc %t3.0 %t-embedded.bc
+; RUN: llvm-dis %t-embedded.bc -o - | FileCheck %s --check-prefix=CHECK-LL
+
+; CHECK-ELF: .text
+; CHECK-ELF: .llvmbc
+
+; CHECK-LL: @_start
+; CHECK-LL: @foo
+; CHECK-LL: @bar
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define void @_start() {
+  ret void
+}
Index: llvm/test/LTO/X86/Inputs/start-lib2.ll
===
--- /dev/null
+++ llvm/test/LTO/X86/Inputs/start-lib2.ll
@@ -0,0 +1,6 @@
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define void @bar() {
+  ret void
+}
Index: llvm/test/LTO/X86/Inputs/start-lib1.ll
===
--- /dev/null
+++ llvm/test/LTO/X86/Inputs/start-lib1.ll
@@ -0,0 +1,8 @@
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+declare void @bar()
+
+define void @foo() {
+  ret void
+}
Index: llvm/lib/LTO/LTOBackend.cpp
===
--- llvm/lib/LTO/LTOBackend.cpp
+++ llvm/lib/LTO/LTOBackend.cpp
@@ -34,6 +34,7 @@
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Program.h"
+#include "llvm/Support/SmallVectorMemoryBuffer.h"
 #include "llvm/Support/TargetRegistry.h"
 #include "llvm/Support/ThreadPool.h"
 #include 

[PATCH] D68213: [LTO] Support for embedding bitcode section during LTO

[Josef Eisl via Phabricator via cfe-commits]

zapster updated this revision to Diff 222401.
zapster added a comment.

re-ran clang format


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

https://reviews.llvm.org/D68213

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/test/Frontend/x86-embed-bitcode.ll
  llvm/include/llvm/Bitcode/BitcodeWriter.h
  llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
  llvm/lib/LTO/LTOBackend.cpp
  llvm/test/LTO/X86/Inputs/start-lib1.ll
  llvm/test/LTO/X86/Inputs/start-lib2.ll
  llvm/test/LTO/X86/embed-bitcode.ll

Index: llvm/test/LTO/X86/embed-bitcode.ll
===
--- /dev/null
+++ llvm/test/LTO/X86/embed-bitcode.ll
@@ -0,0 +1,33 @@
+; RUN: llvm-as %s -o %t1.o
+; RUN: llvm-as %p/Inputs/start-lib1.ll -o %t2.o
+; RUN: llvm-as %p/Inputs/start-lib2.ll -o %t3.o
+
+; RUN: llvm-lto2 run -r %t1.o,_start,px -r %t2.o,foo,px -r %t3.o,bar,px -r %t2.o,bar,px -lto-embed-bitcode=off -o %t3 %t1.o %t2.o %t3.o
+; RUN: llvm-readelf -S %t3.0 | FileCheck %s --implicit-check-not=.llvmbc
+
+; RUN: llvm-lto2 run -r %t1.o,_start,px -r %t2.o,foo,px -r %t3.o,bar,px -r %t2.o,bar,px -lto-embed-bitcode=marker -o %t3 %t1.o %t2.o %t3.o
+; RUN: llvm-readelf -S %t3.0 | FileCheck %s --check-prefix=CHECK-ELF
+
+; RUN: llvm-lto2 run -r %t1.o,_start,px -r %t2.o,foo,px -r %t3.o,bar,px -r %t2.o,bar,px -lto-embed-bitcode=bitcode -o %t3 %t1.o %t2.o %t3.o
+; RUN: llvm-readelf -S %t3.0 | FileCheck %s --check-prefix=CHECK-ELF
+; RUN: llvm-objcopy -O binary -j .llvmbc %t3.0 %t-embedded.bc
+; RUN: llvm-dis %t-embedded.bc -o - | FileCheck %s --check-prefix=CHECK-LL
+
+; RUN: llvm-lto2 run -r %t1.o,_start,px -r %t2.o,foo,px -r %t3.o,bar,px -r %t2.o,bar,px -lto-embed-bitcode=all -o %t3 %t1.o %t2.o %t3.o
+; RUN: llvm-readelf -S %t3.0 | FileCheck %s --check-prefix=CHECK-ELF
+; RUN: llvm-objcopy -O binary -j .llvmbc %t3.0 %t-embedded.bc
+; RUN: llvm-dis %t-embedded.bc -o - | FileCheck %s --check-prefix=CHECK-LL
+
+; CHECK-ELF: .text
+; CHECK-ELF: .llvmbc
+
+; CHECK-LL: @_start
+; CHECK-LL: @foo
+; CHECK-LL: @bar
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define void @_start() {
+  ret void
+}
Index: llvm/test/LTO/X86/Inputs/start-lib2.ll
===
--- /dev/null
+++ llvm/test/LTO/X86/Inputs/start-lib2.ll
@@ -0,0 +1,6 @@
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define void @bar() {
+  ret void
+}
Index: llvm/test/LTO/X86/Inputs/start-lib1.ll
===
--- /dev/null
+++ llvm/test/LTO/X86/Inputs/start-lib1.ll
@@ -0,0 +1,8 @@
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+declare void @bar()
+
+define void @foo() {
+  ret void
+}
Index: llvm/lib/LTO/LTOBackend.cpp
===
--- llvm/lib/LTO/LTOBackend.cpp
+++ llvm/lib/LTO/LTOBackend.cpp
@@ -34,6 +34,7 @@
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Program.h"
+#include "llvm/Support/SmallVectorMemoryBuffer.h"
 #include "llvm/Support/TargetRegistry.h"
 #include "llvm/Support/ThreadPool.h"
 #include "llvm/Support/raw_ostream.h"
@@ -312,11 +313,56 @@
   return !Conf.PostOptModuleHook || Conf.PostOptModuleHook(Task, Mod);
 }
 
+cl::opt
+EmbedBitcode("lto-embed-bitcode", cl::desc("Embed LLVM bitcode"),
+ cl::value_desc("option: off, all, bitcode, marker"));
+
+enum EmbedBitcodeKind {
+  Embed_Off, // No embedded bitcode.
+  Embed_All, // Embed bitcode and commandline in the output.
+  Embed_Bitcode, // Embed only bitcode in the output.
+  Embed_Marker   // Embed a marker as a placeholder for bitcode.
+};
+
+static EmbedBitcodeKind getEmbedBitcode(Config ) {
+  if (EmbedBitcode.empty())
+return EmbedBitcodeKind::Embed_Off;
+  StringRef value = EmbedBitcode;
+  unsigned Model = llvm::StringSwitch(value)
+   .Case("off", EmbedBitcodeKind::Embed_Off)
+   .Case("all", EmbedBitcodeKind::Embed_All)
+   .Case("bitcode", EmbedBitcodeKind::Embed_Bitcode)
+   .Case("marker", EmbedBitcodeKind::Embed_Marker)
+   .Default(~0U);
+  if (Model == ~0U) {
+report_fatal_error("invalid value '" + value + "' for 'lto-embed-bitcode'");
+  }
+  return static_cast(Model);
+}
+
+static void EmitBitcodeSection(Module , Config ) {
+  const EmbedBitcodeKind embedBitcode = getEmbedBitcode(Conf);
+  if (embedBitcode == Embed_Off)
+return;
+  SmallVector Buffer;
+  raw_svector_ostream OS(Buffer);
+  WriteBitcodeToFile(M, OS);
+
+  std::unique_ptr Buf(
+  new SmallVectorMemoryBuffer(std::move(Buffer)));
+  std::vector CmdArgs;
+  llvm::EmbedBitcodeInModule(
+  M, Buf->getMemBufferRef(), embedBitcode != EmbedBitcodeKind::Embed_Marker,
+   

[PATCH] D68141: [Alignment][NFC] Remove AllocaInst::setAlignment(unsigned)

[Guillaume Chatelet via Phabricator via cfe-commits]

gchatelet updated this revision to Diff 222400.
gchatelet marked an inline comment as done.
gchatelet added a comment.
Herald added a reviewer: bollu.

- Address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68141

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  llvm/include/llvm/IR/Instructions.h
  llvm/lib/CodeGen/AtomicExpandPass.cpp
  llvm/lib/CodeGen/CodeGenPrepare.cpp
  llvm/lib/IR/Core.cpp
  llvm/lib/IR/Instructions.cpp
  llvm/lib/Target/AArch64/AArch64StackTagging.cpp
  llvm/lib/Target/AMDGPU/AMDGPULibCalls.cpp
  llvm/lib/Target/NVPTX/NVPTXLowerArgs.cpp
  llvm/lib/Transforms/Coroutines/CoroFrame.cpp
  llvm/lib/Transforms/IPO/Inliner.cpp
  llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp
  llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
  llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
  llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
  llvm/lib/Transforms/Scalar/GVNHoist.cpp
  llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp
  llvm/lib/Transforms/Utils/Local.cpp
  polly/lib/CodeGen/IslNodeBuilder.cpp

Index: polly/lib/CodeGen/IslNodeBuilder.cpp
===
--- polly/lib/CodeGen/IslNodeBuilder.cpp
+++ polly/lib/CodeGen/IslNodeBuilder.cpp
@@ -1497,7 +1497,8 @@
 
   auto *CreatedArray = new AllocaInst(NewArrayType, DL.getAllocaAddrSpace(),
   SAI->getName(), &*InstIt);
-  CreatedArray->setAlignment(PollyTargetFirstLevelCacheLineSize);
+  CreatedArray->setAlignment(
+  MaybeAlign(PollyTargetFirstLevelCacheLineSize));
   SAI->setBasePtr(CreatedArray);
 }
   }
Index: llvm/lib/Transforms/Utils/Local.cpp
===
--- llvm/lib/Transforms/Utils/Local.cpp
+++ llvm/lib/Transforms/Utils/Local.cpp
@@ -1154,7 +1154,7 @@
 // then don't round up. This avoids dynamic stack realignment.
 if (DL.exceedsNaturalStackAlignment(Align(PrefAlign)))
   return Alignment;
-AI->setAlignment(PrefAlign);
+AI->setAlignment(MaybeAlign(PrefAlign));
 return PrefAlign;
   }
 
Index: llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp
===
--- llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp
+++ llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp
@@ -895,7 +895,7 @@
   // If the destination wasn't sufficiently aligned then increase its alignment.
   if (!isDestSufficientlyAligned) {
 assert(isa(cpyDest) && "Can only increase alloca alignment!");
-cast(cpyDest)->setAlignment(srcAlign);
+cast(cpyDest)->setAlignment(MaybeAlign(srcAlign));
   }
 
   // Drop any cached information about the call, because we may have changed
Index: llvm/lib/Transforms/Scalar/GVNHoist.cpp
===
--- llvm/lib/Transforms/Scalar/GVNHoist.cpp
+++ llvm/lib/Transforms/Scalar/GVNHoist.cpp
@@ -899,8 +899,8 @@
   ++NumStoresRemoved;
 } else if (auto *ReplacementAlloca = dyn_cast(Repl)) {
   ReplacementAlloca->setAlignment(
-  std::max(ReplacementAlloca->getAlignment(),
-   cast(I)->getAlignment()));
+  MaybeAlign(std::max(ReplacementAlloca->getAlignment(),
+  cast(I)->getAlignment(;
 } else if (isa(Repl)) {
   ++NumCallsRemoved;
 }
Index: llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
@@ -1184,7 +1184,7 @@
 uint64_t Size = getAllocaSizeInBytes(*AI);
 uint64_t AlignedSize = alignTo(Size, Mapping.getObjectAlignment());
 AI->setAlignment(
-std::max(AI->getAlignment(), Mapping.getObjectAlignment()));
+MaybeAlign(std::max(AI->getAlignment(), Mapping.getObjectAlignment(;
 if (Size != AlignedSize) {
   Type *AllocatedType = AI->getAllocatedType();
   if (AI->isArrayAllocation()) {
@@ -1197,7 +1197,7 @@
   auto *NewAI = new AllocaInst(
   TypeWithPadding, AI->getType()->getAddressSpace(), nullptr, "", AI);
   NewAI->takeName(AI);
-  NewAI->setAlignment(AI->getAlignment());
+  NewAI->setAlignment(MaybeAlign(AI->getAlignment()));
   NewAI->setUsedWithInAlloca(AI->isUsedWithInAlloca());
   NewAI->setSwiftError(AI->isSwiftError());
   NewAI->copyMetadata(*AI);
Index: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -2899,18 +2899,19 @@
   for (Argument  : F.args()) {
   

[PATCH] D68211: [clangd] Use the index-based API to do the header-source switch.

[Haojian Wu via Phabricator via cfe-commits]

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

If the file heuristic fails, we try to use the index to do the
header/source inference.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D68211

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/unittests/HeaderSourceSwitchTests.cpp
  clang-tools-extra/clangd/unittests/SyncAPI.cpp
  clang-tools-extra/clangd/unittests/SyncAPI.h

Index: clang-tools-extra/clangd/unittests/SyncAPI.h
===
--- clang-tools-extra/clangd/unittests/SyncAPI.h
+++ clang-tools-extra/clangd/unittests/SyncAPI.h
@@ -56,6 +56,9 @@
 llvm::Expected>
 runSemanticRanges(ClangdServer , PathRef File, Position Pos);
 
+llvm::Expected>
+runSwitchHeaderSource(ClangdServer , PathRef File);
+
 } // namespace clangd
 } // namespace clang
 
Index: clang-tools-extra/clangd/unittests/SyncAPI.cpp
===
--- clang-tools-extra/clangd/unittests/SyncAPI.cpp
+++ clang-tools-extra/clangd/unittests/SyncAPI.cpp
@@ -152,5 +152,12 @@
   return std::move(*Result);
 }
 
+llvm::Expected>
+runSwitchHeaderSource(ClangdServer , PathRef File) {
+  llvm::Optional>> Result;
+  Server.switchSourceHeader(File, capture(Result));
+  return std::move(*Result);
+}
+
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/unittests/HeaderSourceSwitchTests.cpp
===
--- clang-tools-extra/clangd/unittests/HeaderSourceSwitchTests.cpp
+++ clang-tools-extra/clangd/unittests/HeaderSourceSwitchTests.cpp
@@ -8,6 +8,7 @@
 
 #include "HeaderSourceSwitch.h"
 
+#include "SyncAPI.h"
 #include "TestFS.h"
 #include "TestTU.h"
 #include "index/MemIndex.h"
@@ -240,6 +241,31 @@
   }
 }
 
+TEST(HeaderSourceSwitchTest, ClangdServerIntegration) {
+  class IgnoreDiagnostics : public DiagnosticsConsumer {
+void onDiagnosticsReady(PathRef File,
+std::vector Diagnostics) override {}
+  } DiagConsumer;
+  MockCompilationDatabase CDB;
+  CDB.ExtraClangFlags = {"-I../include"}; // add search directory.
+  MockFSProvider FS;
+  // File heuristic fails here, we rely on the index to find the .h file.
+  std::string CppPath = testPath("src/lib/test.cpp");
+  std::string HeaderPath = testPath("src/include/test.h");
+  FS.Files[HeaderPath] = "void foo();";
+  const std::string FileContent = R"cpp(
+#include "test.h"
+void foo() {};
+  )cpp";
+  FS.Files[CppPath] = FileContent;
+  auto Options = ClangdServer::optsForTest();
+  Options.BuildDynamicSymbolIndex = true;
+  ClangdServer Server(CDB, FS, DiagConsumer, Options);
+  runAddDocument(Server, CppPath, FileContent);
+  EXPECT_EQ(HeaderPath,
+*llvm::cantFail(runSwitchHeaderSource(Server, CppPath)));
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/ClangdServer.h
===
--- clang-tools-extra/clangd/ClangdServer.h
+++ clang-tools-extra/clangd/ClangdServer.h
@@ -194,7 +194,8 @@
 
   /// Helper function that returns a path to the corresponding source file when
   /// given a header file and vice versa.
-  llvm::Optional switchSourceHeader(PathRef Path);
+  void switchSourceHeader(PathRef Path,
+  Callback> CB);
 
   /// Get document highlights for a given position.
   void findDocumentHighlights(PathRef File, Position Pos,
Index: clang-tools-extra/clangd/ClangdServer.cpp
===
--- clang-tools-extra/clangd/ClangdServer.cpp
+++ clang-tools-extra/clangd/ClangdServer.cpp
@@ -449,8 +449,18 @@
   WorkScheduler.runWithAST("Definitions", File, std::move(Action));
 }
 
-llvm::Optional ClangdServer::switchSourceHeader(PathRef Path) {
-  return getCorrespondingHeaderOrSource(Path, FSProvider.getFileSystem());
+void ClangdServer::switchSourceHeader(
+PathRef Path, Callback> CB) {
+  if (auto CorrespondingFile =
+  getCorrespondingHeaderOrSource(Path, FSProvider.getFileSystem()))
+return CB(std::move(CorrespondingFile));
+  auto Action = [Path, CB = std::move(CB),
+ this](llvm::Expected InpAST) mutable {
+if (!InpAST)
+  return CB(InpAST.takeError());
+CB(getCorrespondingHeaderOrSource(Path, InpAST->AST, Index));
+  };
+  WorkScheduler.runWithAST("SwitchHeaderSource", Path, std::move(Action));
 }
 
 llvm::Expected
Index: clang-tools-extra/clangd/ClangdLSPServer.cpp
===
--- clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -1038,10 +1038,16 @@
 void 

[PATCH] D67695: [clangd] Implement getBeginning for overloaded operators.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Mostly NITs, except the naming of the new `TokenKind` enum.
I think it's better to pick something that's not clashing with 
`clang::tok::TokenKind`, even if the enum is in a different namespace.




Comment at: clang-tools-extra/clangd/SourceCode.cpp:242
+
+enum TokenKind { Identifier, Operator, Whitespace, Other };
+

`TokenKind` has the same name as `tok::TokenKind`. Could we use a different 
name here to avoid possible confusions?
E.g. `TokenGroup` or `TokenFlavor`.



Comment at: clang-tools-extra/clangd/SourceCode.cpp:258
+
+TokenKind getTokenKind(SourceLocation Loc, const SourceManager ,
+   const LangOptions ) {

NIT: add `static` for consistency with the rest of the function.



Comment at: clang-tools-extra/clangd/SourceCode.cpp:261
+  Token Tok;
+  Tok.setKind(tok::TokenKind::NUM_TOKENS);
+  if (Lexer::getRawToken(Loc, Tok, SM, LangOpts,

NIT: can this just be `tok::NUM_TOKENS`? same for other kinds in the same 
function



Comment at: clang-tools-extra/clangd/SourceCode.cpp:311
+
+  // Case 2.
+  if (BeforeTokBeginning == CurrentTokBeginning) {

NIT: Could you please duplicate what `case 2` is to avoid the need to go back 
to the comment at the top.



Comment at: clang-tools-extra/clangd/SourceCode.cpp:326
+
+  // Handle case 1 and 3. Note that the cursor could be at the token boundary,
+  // e.g. "Before^Current", we prefer identifiers to other tokens.

`could be at the token boundary` should be `is at the token boundary`, right?
Whenever it's not the case we'll bail out on `BeforeTokBeginning == 
CurrentTokBeginning`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67695



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

[PATCH] D68137: [clangd] Handle template arguments in findExplicitReferences

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: clang-tools-extra/clangd/FindTarget.cpp:556
 
+  // We re-define Traverse*, since there's no corresponding Visit*.
+  bool TraverseTemplateArgumentLoc(TemplateArgumentLoc A) {

... and we need it because, template template decls are visited through it ?



Comment at: clang-tools-extra/clangd/FindTarget.cpp:567
+  break;
+case TemplateArgument::Declaration:
+  break; // FIXME: can this actually happen in TemplateArgumentLoc?

I suppose these corresponds to non-type template paramters, don't they? (which 
should make it similar to integral case)



Comment at: clang-tools-extra/clangd/FindTarget.cpp:618
-if (Ref->NameLoc.isInvalid()) {
-  dlog("invalid location at node {0}", nodeToString(N));
   return;

can we keep dlog ?



Comment at: clang-tools-extra/clangd/unittests/FindTargetTests.cpp:751
+int func();
+template  struct wrapper {};
+

can you also name the template param, add a reference in wrapper and check for 
it ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68137



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