[PATCH] D76098: [WIP] [clangd] Do not trigger go-to-def textual fallback inside string literals

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: clang-tools-extra/clangd/XRefs.cpp:380
 
+  TokenFlavor Flavor = getTokenFlavor(Loc, SM, AST.getLangOpts());
+  // Only consider comment and (raw) identifier tokens.

you can rather use `AST.getTokens().spelledTokenAt(Loc)` to get preprocessed 
token, and pass that into getTokenFlavor rather than a SourceLocation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76098



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

[PATCH] D75745: [clang-tidy] Added AllowMissingMoveFunctionsWhenCopyIsDeleted flag to cppcoreguidelines-special-member-functions

[Paweł Barań via Phabricator via cfe-commits]

pbaran added a comment.

Thank you! Could you please integrate it for me? I do not have access rights


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75745



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

[PATCH] D76084: [Sema][SVE] Reject subscripts on pointers to sizeless types

[Richard Sandiford via Phabricator via cfe-commits]

rsandifo-arm added a comment.

In D76084#1920103 , @efriedma wrote:

> I'm not sure we actually want to reject this; let's discuss on the review for 
> D76086 , since subscripting is sort of a 
> special case of arithmetic.


A silly corner case for this is:

  template
  constexpr __SIZE_TYPE__ f(T *x) { return &x[1] - x; }
  typedef int arr1[f((int *)0) - 1];
  typedef int arr2[f((__SVInt8_t *)0) - 1];

which gives:

  a.cpp:2:48: warning: subtraction of pointers to type '__SVInt8_t' of zero 
size has undefined behavior [-Wpointer-arith]
  constexpr __SIZE_TYPE__ f(T *x) { return &x[1] - x; }
   ~ ^ ~
  a.cpp:4:18: note: in instantiation of function template specialization 
'f<__SVInt8_t>' requested here
  typedef int arr2[f((__SVInt8_t *)0) - 1];
   ^
  a.cpp:4:13: error: variable length array declaration not allowed at file scope
  typedef int arr2[f((__SVInt8_t *)0) - 1];
  ^

No real code would do anything like this of course.  But I think it shows that 
it's going to be difficult to divide cases that measure `sizeof` only during 
CodeGen from those that measure it during Sema.  With more and more things 
being accepted as constexprs, I think it's safer to treat any measure of 
`sizeof` as being at least potentially computable at compile time.

There's also the argument that this is what the spec says.  I realise that's a 
fairly weak argument though, since the spec could always be changed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76084



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

[PATCH] D75682: [Analyzer][StreamChecker] Introduction of stream error handling.

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

balazske marked 6 inline comments as done.
balazske added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:88
 struct FnDescription;
 using FnCheck = std::function;

NoQ wrote:
> `llvm::function_ref`?
`function_ref`'s documentation says:
> This class does not own the callable, so it is not in general safe to store a 
> function_ref.
The `FnDescription` stores these functions so it is not safe to use 
`llvm::function_ref`?






Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:366
+
+  State = State->set(StreamSym, StreamState::getOpened());
+  C.addTransition(State);

NoQ wrote:
> So what exactly happens when you're calling `clearerr` on a closed stream? 
> Does it actually become opened? Also what happens when you're calling it on 
> an open-failed stream?
The stream gets always in opened and error-free state. The `preDefault` checks 
that the stream is opened (not closed and not open-failed) and creates error 
node if not. If this create fails that is a problem case, the stream will be 
opened after `clearerr`. Should check in the eval functions for closed stream 
even if the pre-functions (in `preCall` callback) have normally checked this?



Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:379-381
+  const CallExpr *CE = dyn_cast_or_null(Call.getOriginExpr());
+  if (!CE)
+return;

NoQ wrote:
> Maybe we should add a helper function for this, 
> `CallEvent::getOriginCallExpr()`. That's an otherwise ugly function given 
> that `CallEvent ` is a class hierarchy and the superclass doesn't need to 
> know about subclasses, but most users do in fact only care about 
> call-expressions and concise checker API is important. Same with 
> `dyn_cast_or_null(Call.getDecl())` that could be written as 
> `Call.getFunctionDecl()`.
Can be done in another revision together with changing every use of 
`dyn_cast_or_null(Call.getOriginExpr())` to the new function.



Comment at: clang/test/Analysis/stream-error.c:33-34
+  int ch = fputc('a', F);
+  if (ch == EOF) {
+// FIXME: fputc not handled by checker yet, should expect TRUE
+clang_analyzer_eval(feof(F) || ferror(F)); // expected-warning {{FALSE}}

NoQ wrote:
> Szelethus wrote:
> > Sure, we don't, but the bigger issue here is that we wouldn't mark `F` to 
> > be in EOF even if we did handle it, we would also need to understand that 
> > `ch == EOF` is the telling sign. But that also ins't in the scope of this 
> > patch :)
> Yeah, that's gonna be fun.
If there is a `fputc` call (for example) that is recognized (has recognized 
stream and no functions "fail") there is already an error and non-error branch 
created for it. On the error branch the return value of `fputc` would be 
**EOF** and the stream state is set to error. (The `fputc` should be later 
modeled by this checker, not done yet.)

If the `ch == EOF` is found then to figure out if this `ch` comes from an 
`fputc` that is called on a stream that is not recognized (it may be function 
parameter) and then make a "tracked" stream with error state for it, this is 
another thing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75682



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

[PATCH] D76218: [Sema][SVE] Reject "new" with sizeless types

[Richard Sandiford via Phabricator via cfe-commits]

rsandifo-arm created this revision.
rsandifo-arm added reviewers: sdesmalen, efriedma, rovka, rjmccall.
Herald added subscribers: cfe-commits, psnobl, rkruppe, tschuett.
Herald added a reviewer: rengolin.
Herald added a project: clang.
rsandifo-arm added a parent revision: D76090: [Sema][SVE] Don't allow sizeless 
types to be caught.
rsandifo-arm added a child revision: D76219: [Sema][SVE] Reject "delete" with 
sizeless types.

new-expressions for a type T require sizeof(T) to be computable,
so the SVE ACLE does not allow them for sizeless types.  At the moment:

  auto f() { return new __SVInt8_t; }

creates a call to operator new with a zero size:

  %call = call noalias nonnull i8* @_Znwm(i64 0)

This patch reports an appropriate error instead.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D76218

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/SemaCXX/sizeless-1.cpp


Index: clang/test/SemaCXX/sizeless-1.cpp
===
--- clang/test/SemaCXX/sizeless-1.cpp
+++ clang/test/SemaCXX/sizeless-1.cpp
@@ -411,6 +411,15 @@
   } catch (svint8_t &) { // expected-error {{cannot catch reference to 
sizeless type 'svint8_t'}}
   }
 
+  new svint8_t; // expected-error {{allocation of sizeless type 
'svint8_t'}}
+  new svint8_t();   // expected-error {{allocation of sizeless type 
'svint8_t'}}
+  new svint8_t[10]; // expected-error {{allocation of sizeless type 
'svint8_t'}}
+  new svint8_t *;
+
+  new (global_int8_ptr) svint8_t; // expected-error {{allocation of 
sizeless type 'svint8_t'}}
+  new (global_int8_ptr) svint8_t();   // expected-error {{allocation of 
sizeless type 'svint8_t'}}
+  new (global_int8_ptr) svint8_t[10]; // expected-error {{allocation of 
sizeless type 'svint8_t'}}
+
   local_int8.~__SVInt8_t(); // expected-error {{object expression of 
non-scalar type 'svint8_t' (aka '__SVInt8_t') cannot be used in a 
pseudo-destructor expression}}
 
   (void)svint8_t();
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -2340,7 +2340,8 @@
 return Diag(Loc, diag::err_bad_new_type)
   << AllocType << 1 << R;
   else if (!AllocType->isDependentType() &&
-   RequireCompleteType(Loc, AllocType, 
diag::err_new_incomplete_type,R))
+   RequireCompleteSizedType(
+   Loc, AllocType, diag::err_new_incomplete_or_sizeless_type, R))
 return true;
   else if (RequireNonAbstractType(Loc, AllocType,
   diag::err_allocation_of_abstract_type))
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6897,8 +6897,8 @@
   "array size must be specified in new expression with no initializer">;
 def err_bad_new_type : Error<
   "cannot allocate %select{function|reference}1 type %0 with new">;
-def err_new_incomplete_type : Error<
-  "allocation of incomplete type %0">;
+def err_new_incomplete_or_sizeless_type : Error<
+  "allocation of %select{incomplete|sizeless}0 type %1">;
 def err_new_array_nonconst : Error<
   "only the first dimension of an allocated array may have dynamic size">;
 def err_new_array_size_unknown_from_init : Error<


Index: clang/test/SemaCXX/sizeless-1.cpp
===
--- clang/test/SemaCXX/sizeless-1.cpp
+++ clang/test/SemaCXX/sizeless-1.cpp
@@ -411,6 +411,15 @@
   } catch (svint8_t &) { // expected-error {{cannot catch reference to sizeless type 'svint8_t'}}
   }
 
+  new svint8_t; // expected-error {{allocation of sizeless type 'svint8_t'}}
+  new svint8_t();   // expected-error {{allocation of sizeless type 'svint8_t'}}
+  new svint8_t[10]; // expected-error {{allocation of sizeless type 'svint8_t'}}
+  new svint8_t *;
+
+  new (global_int8_ptr) svint8_t; // expected-error {{allocation of sizeless type 'svint8_t'}}
+  new (global_int8_ptr) svint8_t();   // expected-error {{allocation of sizeless type 'svint8_t'}}
+  new (global_int8_ptr) svint8_t[10]; // expected-error {{allocation of sizeless type 'svint8_t'}}
+
   local_int8.~__SVInt8_t(); // expected-error {{object expression of non-scalar type 'svint8_t' (aka '__SVInt8_t') cannot be used in a pseudo-destructor expression}}
 
   (void)svint8_t();
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -2340,7 +2340,8 @@
 return Diag(Loc, diag::err_bad_new_type)
   << AllocType << 1 << R;
   else if (!AllocType->isDependentType() &&
-   RequireCompleteType(Loc, AllocType, diag::err_new_incomplete_type,R))
+   RequireCompleteSizedType(
+   

[PATCH] D57497: [RISCV] Passing small data limitation value to RISCV backend

[Sam Elliott via Phabricator via cfe-commits]

lenary accepted this revision.
lenary added a comment.

In D57497#1923615 , @shiva0217 wrote:

> In D57497#1921497 , @lenary wrote:
>
> > How hard would it be to use the `-msmall-data-threshold` flag work if a 
> > `-msmall-data-limit` flag is not provided?
>
>
> I think it won't be hard, users just need to find the same semantic flags 
> when switching between LLVM and GCC. If we use the same flag for the same 
> functionality, we could avoid the effort. Does it sound reasonable?


I'm not suggesting "don't support `-msmall-data-limit=`". I'm suggesting "use 
`-msmall-data-threshold=` if `-msmall-data-limit=` is not specified".

Actually, we can always add this support in later, it shouldn't block this 
patch. I'm happy for this patch to be merged - @jrtc27 is too, so let's wait 
for @apazos's final thoughts.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57497



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

[PATCH] D76219: [Sema][SVE] Reject "delete" with sizeless types

[Richard Sandiford via Phabricator via cfe-commits]

rsandifo-arm created this revision.
rsandifo-arm added reviewers: sdesmalen, efriedma, rovka, rjmccall.
Herald added subscribers: cfe-commits, psnobl, rkruppe, tschuett.
Herald added a reviewer: rengolin.
Herald added a project: clang.
rsandifo-arm added a parent revision: D76218: [Sema][SVE] Reject "new" with 
sizeless types.

Sizeless types can't be used with "new", so it doesn't make sense
to use them with "delete" either.  The SVE ACLE therefore doesn't
allow that.

This is slightly stronger than for normal incomplete types, since:

  struct S;
  void f(S *s) { delete s; }

is (by necessity) just a default-on warning rather than an error.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D76219

Files:
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/SemaCXX/sizeless-1.cpp


Index: clang/test/SemaCXX/sizeless-1.cpp
===
--- clang/test/SemaCXX/sizeless-1.cpp
+++ clang/test/SemaCXX/sizeless-1.cpp
@@ -420,6 +420,9 @@
   new (global_int8_ptr) svint8_t();   // expected-error {{allocation of 
sizeless type 'svint8_t'}}
   new (global_int8_ptr) svint8_t[10]; // expected-error {{allocation of 
sizeless type 'svint8_t'}}
 
+  delete global_int8_ptr;   // expected-error {{cannot delete expression of 
type 'svint8_t *'}}
+  delete[] global_int8_ptr; // expected-error {{cannot delete expression of 
type 'svint8_t *'}}
+
   local_int8.~__SVInt8_t(); // expected-error {{object expression of 
non-scalar type 'svint8_t' (aka '__SVInt8_t') cannot be used in a 
pseudo-destructor expression}}
 
   (void)svint8_t();
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -3467,7 +3467,8 @@
   // this, so we treat it as a warning unless we're in a SFINAE context.
   Diag(StartLoc, diag::ext_delete_void_ptr_operand)
 << Type << Ex.get()->getSourceRange();
-} else if (Pointee->isFunctionType() || Pointee->isVoidType()) {
+} else if (Pointee->isFunctionType() || Pointee->isVoidType() ||
+   Pointee->isSizelessType()) {
   return ExprError(Diag(StartLoc, diag::err_delete_operand)
 << Type << Ex.get()->getSourceRange());
 } else if (!Pointee->isDependentType()) {


Index: clang/test/SemaCXX/sizeless-1.cpp
===
--- clang/test/SemaCXX/sizeless-1.cpp
+++ clang/test/SemaCXX/sizeless-1.cpp
@@ -420,6 +420,9 @@
   new (global_int8_ptr) svint8_t();   // expected-error {{allocation of sizeless type 'svint8_t'}}
   new (global_int8_ptr) svint8_t[10]; // expected-error {{allocation of sizeless type 'svint8_t'}}
 
+  delete global_int8_ptr;   // expected-error {{cannot delete expression of type 'svint8_t *'}}
+  delete[] global_int8_ptr; // expected-error {{cannot delete expression of type 'svint8_t *'}}
+
   local_int8.~__SVInt8_t(); // expected-error {{object expression of non-scalar type 'svint8_t' (aka '__SVInt8_t') cannot be used in a pseudo-destructor expression}}
 
   (void)svint8_t();
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -3467,7 +3467,8 @@
   // this, so we treat it as a warning unless we're in a SFINAE context.
   Diag(StartLoc, diag::ext_delete_void_ptr_operand)
 << Type << Ex.get()->getSourceRange();
-} else if (Pointee->isFunctionType() || Pointee->isVoidType()) {
+} else if (Pointee->isFunctionType() || Pointee->isVoidType() ||
+   Pointee->isSizelessType()) {
   return ExprError(Diag(StartLoc, diag::err_delete_operand)
 << Type << Ex.get()->getSourceRange());
 } else if (!Pointee->isDependentType()) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 8b409ea - [SVE] Auto-generate builtins and header for svld1.

[Sander de Smalen via cfe-commits]

Author: Sander de Smalen
Date: 2020-03-16T10:52:37Z
New Revision: 8b409eabaf755c88a7d652fe99d3ad858a4fe82a

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

LOG: [SVE] Auto-generate builtins and header for svld1.

This is a first patch in a series for the SveEmitter to generate the arm_sve.h
header file and builtins.

I've tried my best to strip down this patch as best as I could, but there
are still a few changes that are not necessarily exercised by the load 
intrinsics
in this patch, mostly around the SVEType class which has some common logic to
represent types from a type and prototype string. I thought it didn't make
much sense to remove that from this patch and split it up.

Reviewers: efriedma, rovka, SjoerdMeijer, rsandifo-arm, rengolin

Reviewed By: SjoerdMeijer

Tags: #clang

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

Added: 
clang/include/clang/Basic/AArch64SVETypeFlags.h
clang/include/clang/Basic/BuiltinsSVE.def

Modified: 
clang/include/clang/Basic/BuiltinsAArch64.def
clang/include/clang/Basic/CMakeLists.txt
clang/include/clang/Basic/TargetBuiltins.h
clang/include/clang/Basic/arm_sve.td
clang/lib/Basic/Targets/AArch64.cpp
clang/lib/CodeGen/CGBuiltin.cpp
clang/lib/CodeGen/CodeGenFunction.h
clang/utils/TableGen/SveEmitter.cpp
clang/utils/TableGen/TableGen.cpp
clang/utils/TableGen/TableGenBackends.h

Removed: 




diff  --git a/clang/include/clang/Basic/AArch64SVETypeFlags.h 
b/clang/include/clang/Basic/AArch64SVETypeFlags.h
new file mode 100644
index ..2b11fe6f9b2b
--- /dev/null
+++ b/clang/include/clang/Basic/AArch64SVETypeFlags.h
@@ -0,0 +1,67 @@
+//===- AArch64SVETypeFlags.h - Flags used to generate ACLE builtins- C++ 
-*-===//
+//
+//  Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+//  See https://llvm.org/LICENSE.txt for license information.
+//  SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_BASIC_AARCH64SVETYPEFLAGS_H
+#define LLVM_CLANG_BASIC_AARCH64SVETYPEFLAGS_H
+
+#include 
+
+namespace clang {
+
+/// Flags to identify the types for overloaded SVE builtins.
+class SVETypeFlags {
+  uint64_t Flags;
+
+public:
+  /// These must be kept in sync with the flags in
+  /// include/clang/Basic/arm_sve.td.
+  static const uint64_t MemEltTypeOffset = 4; // Bit offset of MemEltTypeMask
+  static const uint64_t EltTypeMask  = 0x000f;
+  static const uint64_t MemEltTypeMask   = 0x0070;
+  static const uint64_t IsLoad   = 0x0080;
+
+  enum EltType {
+Invalid,
+Int8,
+Int16,
+Int32,
+Int64,
+Float16,
+Float32,
+Float64,
+Bool8,
+Bool16,
+Bool32,
+Bool64
+  };
+
+  enum MemEltTy {
+MemEltTyDefault,
+MemEltTyInt8,
+MemEltTyInt16,
+MemEltTyInt32,
+MemEltTyInt64
+  };
+
+  SVETypeFlags(uint64_t F) : Flags(F) { }
+  SVETypeFlags(EltType ET, bool IsUnsigned) : Flags(ET) { }
+
+  EltType getEltType() const { return (EltType)(Flags & EltTypeMask); }
+  MemEltTy getMemEltType() const {
+return (MemEltTy)((Flags & MemEltTypeMask) >> MemEltTypeOffset);
+  }
+
+  bool isLoad() const { return Flags & IsLoad; }
+
+  uint64_t getBits() const { return Flags; }
+  bool isFlagSet(uint64_t Flag) const { return Flags & Flag; }
+};
+
+} // end namespace clang
+
+#endif

diff  --git a/clang/include/clang/Basic/BuiltinsAArch64.def 
b/clang/include/clang/Basic/BuiltinsAArch64.def
index 8f3a24c2e1f6..f07c567053de 100644
--- a/clang/include/clang/Basic/BuiltinsAArch64.def
+++ b/clang/include/clang/Basic/BuiltinsAArch64.def
@@ -99,19 +99,6 @@ BUILTIN(__builtin_arm_tcommit, "v", "n")
 BUILTIN(__builtin_arm_tcancel, "vWUIi", "n")
 BUILTIN(__builtin_arm_ttest, "WUi", "nc")
 
-// SVE
-BUILTIN(__builtin_sve_svld1_s16, "q8sq16bSsC*", "n")
-BUILTIN(__builtin_sve_svld1_s32, "q4iq16bSiC*", "n")
-BUILTIN(__builtin_sve_svld1_s64, "q2Wiq16bSWiC*", "n")
-BUILTIN(__builtin_sve_svld1_s8, "q16Scq16bScC*", "n")
-BUILTIN(__builtin_sve_svld1_u16, "q8Usq16bUsC*", "n")
-BUILTIN(__builtin_sve_svld1_u32, "q4Uiq16bUiC*", "n")
-BUILTIN(__builtin_sve_svld1_u64, "q2UWiq16bUWiC*", "n")
-BUILTIN(__builtin_sve_svld1_u8, "q16Ucq16bUcC*", "n")
-BUILTIN(__builtin_sve_svld1_f64, "q2dq16bdC*", "n")
-BUILTIN(__builtin_sve_svld1_f32, "q4fq16bfC*", "n")
-BUILTIN(__builtin_sve_svld1_f16, "q8hq16bhC*", "n")
-
 TARGET_HEADER_BUILTIN(_BitScanForward, "UcUNi*UNi", "nh", "intrin.h", 
ALL_MS_LANGUAGES, "")
 TARGET_HEADER_BUILTIN(_BitScanReverse, "UcUNi*UNi", "nh", "intrin.h", 
ALL_MS_LANGUAGES, "")
 TARGET_HEADER_BUILTIN(_BitScanForward64, "UcUNi*ULLi", "nh", "intrin.h", 
ALL_MS_LANGUAGES, "")

diff  --git a/clang/include/cl

[PATCH] D75514: [Analyzer] Only add container note tags to the operations of the affected container

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

Szelethus accepted this revision.
Szelethus added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/DebugContainerModeling.cpp:32
 
   template 
   void analyzerContainerDataField(const CallExpr *CE, CheckerContext &C,

NoQ wrote:
> `llvm::function_ref`?
Is that a thing? Nice.


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

https://reviews.llvm.org/D75514



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

[PATCH] D72089: [Syntax] Build declarator nodes

[Marcel Hlopko via Phabricator via cfe-commits]

hlopko added a comment.

I'm taking over of this patch (with a kind permission from Ilya :) I've replied 
to comments here, but let's continue the review over at 
https://reviews.llvm.org/D76220. Thanks :)




Comment at: clang/include/clang/Tooling/Syntax/Nodes.h:395
   }
+  /// FIXME: use custom iterator instead of 'vector'.
+  std::vector declarators();

gribozavr2 wrote:
> s/iterator/container/ ?
> 
> Also unclear why a custom one should be used.
> 
> I also think it should be an implementation comment (in the .cc file).
Waiting for reply from Ilya offline, in the meantime I'm keeping the comment as 
is.



Comment at: clang/include/clang/Tooling/Syntax/Nodes.h:491
+/// Array size specified inside a declarator.
+/// E.g. `[10]` in `int a[10]`.
+class ArraySubscript final : public Tree {

gribozavr2 wrote:
> Also `[static 10]` in `void f(int xs[static 10]);`
Added example into the comment + added a test verifying it's handled correctly.



Comment at: clang/include/clang/Tooling/Syntax/Nodes.h:500
+  syntax::Expression *sizeExpression();
+  syntax::Leaf *rbracket();
+};

gribozavr2 wrote:
> + TODO: add an accessor for the "static" keyword.
Added a TODO.



Comment at: clang/include/clang/Tooling/Syntax/Nodes.h:503
+
+/// Trailing return type inside parameter list, starting from the arrow token.
+/// E.g. `-> int***`.

gribozavr2 wrote:
> s/inside parameter list/after the parameter list/ ?
> 
> s/starting from/starting with/ or "including the arrow token" to emphasize 
> that the arrow is included.
Done.



Comment at: clang/include/clang/Tooling/Syntax/Nodes.h:512
+  syntax::Leaf *arrow();
+  syntax::SimpleDeclarator *declarator();
+};

gribozavr2 wrote:
> + TODO: add accessors for specifiers.
> 
> Or we could add a syntax node for the "type-id" construct -- it seems like it 
> will be useful in other places that require exactly one type, like the 
> argument of sizeof, type argument of static_cast etc. (Also could be a TODO, 
> this patch is pretty long.)
Added a TODO (also mentioning type-id node)



Comment at: clang/include/clang/Tooling/Syntax/Nodes.h:515
+
+/// Parameter list for a function type.
+class ParametersAndQualifiers final : public Tree {

gribozavr2 wrote:
> Would be great to show an example of what sort of qualifiers we're talking 
> about here (trailing "const", "volatile", "&", "&&", right?) What about 
> "noexcept"? Would be also good to say that "override" does not belong here.
Added a comment (and added tests where there was no coverage):

+/// E.g.:
+///  `(volatile int a)` in `int foo(volatile int a);`
+///  `(int&& a)` in `int foo(int&& a);`
+///  `() -> int` in `auto foo() -> int;`
+///  `() const` in `int foo() const;`
+///  `() noexcept` in `int foo() noexcept;`
+///  `() throw()` in `int foo() throw();`
+///
+/// (!) override doesn't belong here.



Comment at: clang/include/clang/Tooling/Syntax/Nodes.h:524
+  /// FIXME: use custom iterator instead of 'vector'.
+  std::vector parameters();
+  syntax::Leaf *rparen();

gribozavr2 wrote:
> Similarly, I think it should be an implementation comment.
Will handle once I hear from Ilya.



Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:53
+namespace {
+struct GetStartLoc : TypeLocVisitor {
+  SourceLocation VisitParenTypeLoc(ParenTypeLoc T) {

gribozavr2 wrote:
> A brief comment about why this is necessary (and why it works) would be 
> appreciated. Just remind the reader that type locs are stored inside-out, and 
> that the start location in the source order would be on the innermost node.
Added:

53 /// Get start location of the Decl from the TypeLoc. 



 54 /// E.g.:   


 
 55 ///   loc of `(` in `int (a)`   


 
 56 ///   loc of `*` in `int *(a)`  


 
 57 ///   loc of the first `(` in `int (*a)(int)`   

[PATCH] D76220: Build declarator nodes

[Marcel Hlopko via Phabricator via cfe-commits]

hlopko created this revision.
hlopko added a reviewer: gribozavr2.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Copy of https://reviews.llvm.org/D72089 with Ilya's permission. See
https://reviews.llvm.org/D72089 for the first batch of comments.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D76220

Files:
  clang/include/clang/Tooling/Syntax/Nodes.h
  clang/lib/Tooling/Syntax/BuildTree.cpp
  clang/lib/Tooling/Syntax/Nodes.cpp
  clang/unittests/Tooling/Syntax/TreeTest.cpp

Index: clang/unittests/Tooling/Syntax/TreeTest.cpp
===
--- clang/unittests/Tooling/Syntax/TreeTest.cpp
+++ clang/unittests/Tooling/Syntax/TreeTest.cpp
@@ -174,17 +174,21 @@
 *: TranslationUnit
 |-SimpleDeclaration
 | |-int
-| |-main
-| |-(
-| |-)
+| |-SimpleDeclarator
+| | |-main
+| | `-ParametersAndQualifiers
+| |   |-(
+| |   `-)
 | `-CompoundStatement
 |   |-{
 |   `-}
 `-SimpleDeclaration
   |-void
-  |-foo
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-foo
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 `-}
@@ -201,9 +205,11 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-int
-  |-main
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-main
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 |-IfStatement
@@ -246,9 +252,11 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-void
-  |-test
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-test
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 |-ForStatement
@@ -268,18 +276,21 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-void
-  |-test
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-test
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 |-DeclarationStatement
 | |-SimpleDeclaration
 | | |-int
-| | |-a
-| | |-=
-| | `-UnknownExpression
-| |   `-10
+| | `-SimpleDeclarator
+| |   |-a
+| |   |-=
+| |   `-UnknownExpression
+| | `-10
 | `-;
 `-}
 )txt"},
@@ -287,9 +298,11 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-void
-  |-test
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-test
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 |-EmptyStatement
@@ -309,9 +322,11 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-void
-  |-test
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-test
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 |-SwitchStatement
@@ -345,9 +360,11 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-void
-  |-test
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-test
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 |-WhileStatement
@@ -375,9 +392,11 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-int
-  |-test
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-test
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 |-ReturnStatement
@@ -398,26 +417,31 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-void
-  |-test
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-test
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 |-DeclarationStatement
 | |-SimpleDeclaration
 | | |-int
-| | |-a
-| | |-[
-| | |-UnknownExpression
-| | | `-3
-| | `-]
+| | `-SimpleDeclarator
+| |   |-a
+| |   `-ArraySubscript
+| | |-[
+| | |-UnknownExpression
+| | | `-3
+| | `-]
 | `-;
 |-RangeBasedForStatement
 | |-for
 | |-(
 | |-SimpleDeclaration
 | | |-int
-| | |-x
+| | |-SimpleDeclarator
+| | | `-x
 | | `-:
 | |-UnknownExpression
 | | `-a
@@ -433,9 +457,11 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-void
-  |-main
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-main
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 |-UnknownStatement
@@ -460,9 +486,11 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-void
-  |-test
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-test
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 |-ExpressionStatement
@@ -500,10 +528,12 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-int
-  |-*
-  |-a
+  |-SimpleDeclarator
+  | |-*
+  | `-a
   |-,
-  |-b
+  |-SimpleDeclarator
+  | `-b
   `-;
   )txt"},
   {R"cpp(
@@ -514,10 +544,12 @@
 `-SimpleDeclaration
   |-typedef
   |-int
-  |-*
-  |-a
+  |-SimpleDeclarator
+  | |-*
+  | `-a
   |-,
-  |-b
+  |-SimpleDeclarator
+  | `-b
   `-;
   )txt"},
   // Multiple declarators inside a statement.
@@ -531,27 +563,33 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-void
-  |-foo
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-foo
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 |-DeclarationStatement
 | |-SimpleDeclaration
 | | |-int
- 

[PATCH] D76221: [Sema][SVE] Flip default RequireCompleteType behavior for sizeless types

[Richard Sandiford via Phabricator via cfe-commits]

rsandifo-arm created this revision.
rsandifo-arm added reviewers: sdesmalen, efriedma, rovka, rjmccall.
Herald added subscribers: cfe-commits, danielkiss, psnobl, jfb, rkruppe, 
kristof.beyls, tschuett.
Herald added a reviewer: rengolin.
Herald added a project: clang.
rsandifo-arm added a parent revision: D76219: [Sema][SVE] Reject "delete" with 
sizeless types.

Sizeless types are defined to be a form of incomplete type, but with
more relaxed rules in certain cases.  Previous patches in the series
took the following approach:

- Add tests for things that are explicitly supposed to be valid for sizeless 
types and that clang already handled correctly.

- Add diagnostics for things that are explicitly supposed to be invalid for 
sizeless types, in many cases inheriting the same rules as for incomplete types.

For any other cases, we should conservatively treat sizeless types as
in the same way as incomplete types.  This patch therefore flips the
default behavior for RequireCompleteType and co.  Callers that want
to accept sizeless types must then explicitly pass AcceptSizeless.

The Sema::BuildObjCEncodeExpression change is needed to keep
clang/test/CodeGenObjC/aarch64-sve-types.m passing.  If @encoding a
sizeless type will never be useful, we could enforce that here instead.

For reference, here's a list of every other function that now passes
AcceptSizeless, along with one example line from Sema/sizeless-1.c
or SemaCXX/sizeless-1.cpp that would regress without the change.

- CastOperation::CheckCStyleCast

  local_int8 = (svint8_t)0; // expected-error {{...arithmetic or pointer type 
is required...}}

- Sema::CheckParmsForFunctionDef

  int vararg_receiver(int count, svint8_t first, ...) {

- Sema::AddInitializerToDecl

  svint8_t init_int8 = local_int8;

- Sema::ActOnInitializerError

  svint8_t bad_init_int8 = for; // expected-error {{expected expression}}

- Sema::ActOnUninitializedDecl

  svint8_t local_int8;

- Sema::ActOnStartOfFunctionDef

  svint8_t ret_bad_conv() { return explicit_conv(); }

- Sema::SetParamDefaultArgument

  void with_default(svint8_t = svint8_t());

- Sema::BuildExceptionDeclaration

  } catch (svint8_t) { // expected-error {{...sizeless type...}}

- Sema::CheckSpecifiedExceptionType

  void throwing_func() throw(svint8_t); // expected-error {{...sizeless...}}

- Sema::DefaultVariadicArgumentPromotion

  varargs(1, local_int8, local_int16);

- Sema::GatherArgumentsForCall

  pass_int8(local_int8);

- Sema::BuildResolvedCallExpr

  noproto(local_int8);

- CheckCommaOperands

  0, local_int8;

- Sema::BuildVAArgExpr

  __builtin_va_arg(va, svint8_t);

- Sema::CheckCXXThrowOperand

  throw local_int8; // expected-error {{...sizeless type...}}

- Sema::BuildCXXTypeConstructExpr

  void with_default(svint8_t = svint8_t());

- CheckUnaryTypeTraitTypeCompleteness

  _Static_assert(__is_trivially_destructible(svint8_t), "");

- evaluateTypeTrait

  _Static_assert(__is_constructible(svint8_t), "");

- EvaluateBinaryTypeTrait

  _Static_assert(__is_convertible_to(svint8_t, svint8_t), ""); 
_Static_assert(!__is_assignable(svint8_t, svint8_t), "");

- Sema::IgnoredValueConversions

  (void)local_int8;

- CopyObject

  widget w(1);

- InitializationSequence::Diagnose

  svint8_t ret_bad_conv() { return explicit_conv(); }

- Sema::buildLambdaScope

  local_int8 = ([]() { return svint8_t(); })();

- TryListConversion

  template_fn_direct({wrapper()});

- Sema::AddConversionCandidate

  local_int8 = wrapper();

- Sema::BuildCXXForRangeStmt

  for (auto x : local_int8) {

- Sema::BuildAtomicType

  _Atomic svint8_t atomic_int8; // expected-error {{...sizeless...}}


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D76221

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaCast.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaExceptionSpec.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaExprObjC.cpp
  clang/lib/Sema/SemaInit.cpp
  clang/lib/Sema/SemaLambda.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaType.cpp

Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -7924,7 +7924,7 @@
 
 bool Sema::RequireCompleteExprType(Expr *E, unsigned DiagID) {
   BoundTypeDiagnoser<> Diagnoser(DiagID);
-  return RequireCompleteExprType(E, CompleteTypeKind::Default, Diagnoser);
+  return RequireCompleteExprType(E, CompleteTypeKind::Normal, Diagnoser);
 }
 
 /// Ensure that the type T is a complete type.
@@ -8555,7 +8555,10 @@
   if (!T->isDependentType()) {
 // FIXME: It isn't entirely clear whether incomplete atomic types
 // are allowed or not; for simplicity, ban them for the moment.
-if (RequireCompleteType(Loc, T, diag::err_atomic_specifier_bad_type, 0))
+//
+// Fall through to the code belo

[PATCH] D75360: [analyzer][NFC] Tie CheckerRegistry to CheckerManager, allow CheckerManager to be constructed for non-analysis purposes

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

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



Comment at: clang/include/clang/StaticAnalyzer/Frontend/AnalyzerHelpFlags.h:1-10
+//===-- CheckerRegistration.h - Checker Registration Function ---*- C++ 
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//

This is all incorrect.



Comment at: clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp:33
 #include "llvm/Support/ErrorHandling.h"
+#include 
 using namespace clang;

NoQ wrote:
> Why?
clangd >.< Nice catch, I'll remove it.


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

https://reviews.llvm.org/D75360



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

[PATCH] D75470: [SVE] Auto-generate builtins and header for svld1.

[Sander de Smalen via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG8b409eabaf75: [SVE] Auto-generate builtins and header for 
svld1. (authored by sdesmalen).

Changed prior to commit:
  https://reviews.llvm.org/D75470?vs=249312&id=250509#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75470

Files:
  clang/include/clang/Basic/AArch64SVETypeFlags.h
  clang/include/clang/Basic/BuiltinsAArch64.def
  clang/include/clang/Basic/BuiltinsSVE.def
  clang/include/clang/Basic/CMakeLists.txt
  clang/include/clang/Basic/TargetBuiltins.h
  clang/include/clang/Basic/arm_sve.td
  clang/lib/Basic/Targets/AArch64.cpp
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/utils/TableGen/SveEmitter.cpp
  clang/utils/TableGen/TableGen.cpp
  clang/utils/TableGen/TableGenBackends.h

Index: clang/utils/TableGen/TableGenBackends.h
===
--- clang/utils/TableGen/TableGenBackends.h
+++ clang/utils/TableGen/TableGenBackends.h
@@ -92,6 +92,8 @@
 void EmitNeonTest2(llvm::RecordKeeper &Records, llvm::raw_ostream &OS);
 
 void EmitSveHeader(llvm::RecordKeeper &Records, llvm::raw_ostream &OS);
+void EmitSveBuiltins(llvm::RecordKeeper &Records, llvm::raw_ostream &OS);
+void EmitSveCodeGenMap(llvm::RecordKeeper &Records, llvm::raw_ostream &OS);
 
 void EmitMveHeader(llvm::RecordKeeper &Records, llvm::raw_ostream &OS);
 void EmitMveBuiltinDef(llvm::RecordKeeper &Records, llvm::raw_ostream &OS);
Index: clang/utils/TableGen/TableGen.cpp
===
--- clang/utils/TableGen/TableGen.cpp
+++ clang/utils/TableGen/TableGen.cpp
@@ -71,6 +71,8 @@
   GenArmMveBuiltinCG,
   GenArmMveBuiltinAliases,
   GenArmSveHeader,
+  GenArmSveBuiltins,
+  GenArmSveCodeGenMap,
   GenArmCdeHeader,
   GenArmCdeBuiltinDef,
   GenArmCdeBuiltinSema,
@@ -188,6 +190,10 @@
"Generate ARM NEON tests for clang"),
 clEnumValN(GenArmSveHeader, "gen-arm-sve-header",
"Generate arm_sve.h for clang"),
+clEnumValN(GenArmSveBuiltins, "gen-arm-sve-builtins",
+   "Generate arm_sve_builtins.inc for clang"),
+clEnumValN(GenArmSveCodeGenMap, "gen-arm-sve-codegenmap",
+   "Generate arm_sve_codegenmap.inc for clang"),
 clEnumValN(GenArmMveHeader, "gen-arm-mve-header",
"Generate arm_mve.h for clang"),
 clEnumValN(GenArmMveBuiltinDef, "gen-arm-mve-builtin-def",
@@ -372,6 +378,12 @@
   case GenArmSveHeader:
 EmitSveHeader(Records, OS);
 break;
+  case GenArmSveBuiltins:
+EmitSveBuiltins(Records, OS);
+break;
+  case GenArmSveCodeGenMap:
+EmitSveCodeGenMap(Records, OS);
+break;
   case GenArmCdeHeader:
 EmitCdeHeader(Records, OS);
 break;
Index: clang/utils/TableGen/SveEmitter.cpp
===
--- clang/utils/TableGen/SveEmitter.cpp
+++ clang/utils/TableGen/SveEmitter.cpp
@@ -29,6 +29,7 @@
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/TableGen/Record.h"
 #include "llvm/TableGen/Error.h"
+#include "clang/Basic/AArch64SVETypeFlags.h"
 #include 
 #include 
 #include 
@@ -36,26 +37,535 @@
 
 using namespace llvm;
 
-//===--===//
-// SVEEmitter
-//===--===//
+enum ClassKind {
+  ClassNone,
+  ClassS, // signed/unsigned, e.g., "_s8", "_u8" suffix
+  ClassG, // Overloaded name without type suffix
+};
+
+using TypeSpec = std::string;
+using SVETypeFlags = clang::SVETypeFlags;
 
 namespace {
 
+class SVEType {
+  TypeSpec TS;
+  bool Float, Signed, Immediate, Void, Constant, Pointer;
+  bool DefaultType, IsScalable, Predicate, PredicatePattern, PrefetchOp;
+  unsigned Bitwidth, ElementBitwidth, NumVectors;
+
+public:
+  SVEType() : SVEType(TypeSpec(), 'v') {}
+
+  SVEType(TypeSpec TS, char CharMod)
+  : TS(TS), Float(false), Signed(true), Immediate(false), Void(false),
+Constant(false), Pointer(false), DefaultType(false), IsScalable(true),
+Predicate(false), PredicatePattern(false), PrefetchOp(false),
+Bitwidth(128), ElementBitwidth(~0U), NumVectors(1) {
+if (!TS.empty())
+  applyTypespec();
+applyModifier(CharMod);
+  }
+
+  /// Return the value in SVETypeFlags for this type.
+  unsigned getTypeFlags() const;
+
+  bool isPointer() const { return Pointer; }
+  bool isVoidPointer() const { return Pointer && Void; }
+  bool isSigned() const { return Signed; }
+  bool isImmediate() const { return Immediate; }
+  bool isScalar() const { return NumVectors == 0; }
+  bool isVector() const { return NumVectors > 0; }
+  bool isScalableVector() const { return isVector() && IsScalable; }
+  bool isChar() const { return ElementBitwidth == 8; }
+  bool isVoid() 

[PATCH] D76098: [WIP] [clangd] Do not trigger go-to-def textual fallback inside string literals

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clang-tools-extra/clangd/XRefs.cpp:360
+
+TokenFlavor getTokenFlavor(SourceLocation Loc, const SourceManager &SM,
+   const LangOptions &LangOpts) {

I don't think we need this function, which just maps one enum onto another - 
just use the token kind directly?



Comment at: clang-tools-extra/clangd/XRefs.cpp:380
 
+  TokenFlavor Flavor = getTokenFlavor(Loc, SM, AST.getLangOpts());
+  // Only consider comment and (raw) identifier tokens.

kadircet wrote:
> you can rather use `AST.getTokens().spelledTokenAt(Loc)` to get preprocessed 
> token, and pass that into getTokenFlavor rather than a SourceLocation.
for a bit more context: running Lexer::getRawToken runs a raw lex that only 
sees the piece of text you point at. If you're inside a huge comment, it won't 
know that.

Using AST.getTokens() uses the results of an earlier raw lex of the whole file.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76098



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

[PATCH] D73638: [AST] Move dependence computations into a separate file

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



Comment at: clang/include/clang/AST/Expr.h:4081
-: Expr(ConvertVectorExprClass, DstType, VK, OK,
-   DstType->isDependentType(),
-   DstType->isDependentType() || SrcExpr->isValueDependent(),

sammccall wrote:
> hokein wrote:
> > !  behavior change change, now the `ConvertVectorExpr::TypeDependent = 
> > DstType->isDependentType() | SrcExpr->isDependentType()`.
> this looks incorrect to me. A conversion's type doesn't depend on the input 
> type.
reverted to the old behavior.



Comment at: clang/include/clang/AST/Expr.h:4250
 SourceLocation RPLoc, QualType t, bool IsMS)
-  : Expr(VAArgExprClass, t, VK_RValue, OK_Ordinary, t->isDependentType(),
- false, (TInfo->getType()->isInstantiationDependentType() ||

sammccall wrote:
> hokein wrote:
> > ! the implementation in the original patch doesn't seem to correct to me, I 
> > rewrote it, need another review..
> Are you saying the previous version of this patch didn't match the spec, or 
> didn't match the old behavior, or both?
> 
> The new  one seems pretty different to head.
> e.g. parameter packsand instantiation dependence are propagated from type, 
> type dependence is propagated from the  subexpr and the writtentypeinfo's 
> type.
> 
> Do these not make a difference for practical cases?
> (If this is a bug fix, I'd prefer to land it separately)
ah, you are right, I intended to make the new implementation match the old 
behavior (not sure what I thought before), updated, it should match the old 
behavior now.



Comment at: clang/include/clang/AST/Expr.h:5031
- CommonInit->isValueDependent() || ElementInit->isValueDependent(),
- T->isInstantiationDependentType(),
- CommonInit->containsUnexpandedParameterPack() ||

sammccall wrote:
> hokein wrote:
> > ! behavior change here, now `ArrayInitLoopExpr::Instantiation =  
> > T->isInstantiationDependentType() |  
> > CommonInit->isInstantiationDependentType() |  
> > ElementInit->isInstantiationDependentType()`.
> Is this a bugfix? Consider leaving a FIXME instead?
reverted to old behavior.



Comment at: clang/include/clang/AST/Expr.h:5650
-: Expr(AsTypeExprClass, DstType, VK, OK,
-   DstType->isDependentType(),
-   DstType->isDependentType() || SrcExpr->isValueDependent(),

sammccall wrote:
> hokein wrote:
> > ! behavior change here, now `Expr::TypeDependent = 
> > DstType->isDependentType() | SrcExpr->isTypeDependent()`.
> This seems incorrect to me. Like a new-expr, the type of the result doesn't 
> depend on the type of the inputs.
sounds fair, reverted to old behavior.



Comment at: clang/include/clang/AST/ExprCXX.h:4102
 std::uninitialized_copy(PartialArgs.begin(), PartialArgs.end(), Args);
+setDependence(Length ? ExprDependence::None
+ : ExprDependence::ValueInstantiation);

sammccall wrote:
> Why not have a computeDependence function for this?
> In particular we'll end up missing haserrors propagation without it, right?
this looks like a very straight-forward and simple `Dependence` initialization, 
and it doesn't depend on other elements

For this particular case, I don't see big difference of having a 
`computeDependence` function. If we introduce the error bit, we probably just 
set it to false' but for `CXXFoldExpr` example, it makes sense to have a 
`computeDependence` function, we should populate the errorbit from its 
subexprs. 

and this patches has other places where we directly initialize the dependence, 
we should have `computeDependence` for all of them? 





Comment at: clang/include/clang/AST/TemplateBase.h:672
   TemplateArgumentLoc *OutArgArray);
+  // FIXME: cleanup the unsued Deps.
   void initializeFrom(SourceLocation TemplateKWLoc,

sammccall wrote:
> I don't understand this comment, can you expand it?
The parameter `Deps` is the result populated by this method, the caller doesn't 
need it since we have the `computeDependencies`.



Comment at: clang/lib/AST/ComputeDependence.cpp:13
+#include "clang/AST/DeclarationName.h"
+#include "clang/AST/DependencyFlags.h"
+#include "clang/AST/Expr.h"

sammccall wrote:
> doh, we forgot to fix dependencyflags -> dependenceflags in the previous patch
will rename it in a separate patch.



Comment at: clang/lib/AST/ComputeDependence.cpp:24
+
+// In Expr.h
+ExprDependence clang::computeDependence(FullExpr *E) {

sammccall wrote:
> I'm not sure who these comments are for.
These were for code reviews, functions below were in the `Expr.h` before this 
patch. I will remove them when landing this patch.



Comment at: clang/lib/AST/ComputeDependence.cpp:54
+  auto ArgDeps = E->getArgumentExpr()->getD

[PATCH] D71524: [analyzer] Support tainted objects in GenericTaintChecker

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

Szelethus added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:139
 
+  /// Add taint sources for extraction operator on pre-visit.
+  bool addOverloadedOpPre(const CallExpr *CE, CheckerContext &C) const;

steakhal wrote:
> boga95 wrote:
> > Szelethus wrote:
> > > Extraction operator? Is that a thing?
> > I can call it `operator>>` if you think that is better.
> I think `extraction operator` is the right term for this.
> It is used in the standard: 
> http://eel.is/c++draft/input.streams#istream.extractors
> 
> 
Well, I consider myself proven wrong :)


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

https://reviews.llvm.org/D71524



___
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 250517.
Szelethus edited the summary of this revision.
Szelethus added a comment.

Upload final version. This revision stays here for archaeological purposes, as 
I'll split this up into manageable pieces.


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-local-invalidation.cpp
  clang/test/Analysis/dump-definitions-nontrivial-expressions.cpp
  clang/test/Analysis/dump-definitions-records.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,402 @@
+// 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]) 
+// CHECK-NEXT: 0 OUT (ptr, [3, 3]) 
+// CHECK-NEXT: 0 OUT (ptr, [4, 3]) 
+// CHECK-NEXT: 1 IN (global_var, [4, 6]) 
+// CHECK-NEXT: 1 IN (ptr, [3, 3]) 
+// CHECK-NEXT: 1 IN (ptr, [4, 3]) 
+// CHECK-NEXT: 1 OUT (global_var, [4, 6]) 
+// CHECK-NEXT: 1 OUT (ptr, [3, 3]) 
+// CHECK-NEXT: 1 OUT (ptr, [4, 3]) 
+// CHECK-NEXT: 2 IN (global_var, [4, 6]) 
+// CHECK-NEXT: 2 IN (ptr, [3, 3]) 
+// CHECK-NEXT: 2 IN (ptr, [4, 3]) 
+// CHECK-NEXT: 2 OUT (global_var, [4, 6]) 
+// CHECK-NEXT: 2 OUT (ptr, [3, 3]) 
+// CHECK-NEXT: 2 OUT (ptr, [4, 3]) 
+// CHECK-NEXT: 3 IN (global_var, [4, 6]) 
+// CHECK-NEXT: 3 IN (ptr, [4, 3]) 
+// CHECK-NEXT: 3 OUT (global_var, [4, 6]) 
+// CHECK-NEXT: 3 OUT (ptr, [3, 3]) 
+// CHECK-NEXT: 4 OUT

[clang-tools-extra] 2f20417 - Add AllowMissingMoveFunctionsWhenCopyIsDeleted flag to cppcoreguidelines-special-member-functions

[Aaron Ballman via cfe-commits]

Author: Paweł Barań
Date: 2020-03-16T08:14:48-04:00
New Revision: 2f20417ef04781cd5019b9a99b85df4790bdf525

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

LOG: Add AllowMissingMoveFunctionsWhenCopyIsDeleted flag to 
cppcoreguidelines-special-member-functions

The flag allows classes to don't define move operations when copy operations
are explicitly deleted. This flag is related to Google C++ Style Guide
https://google.github.io/styleguide/cppguide.html#Copyable_Movable_Types

Added: 

clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-special-member-functions-allow-missing-move-when-copy-is-deleted.cpp

Modified: 

clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h

clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-special-member-functions.rst

Removed: 




diff  --git 
a/clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
 
b/clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
index 6ff60495a72b..b7f5eb6e1e33 100644
--- 
a/clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
+++ 
b/clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
@@ -25,12 +25,16 @@ SpecialMemberFunctionsCheck::SpecialMemberFunctionsCheck(
 StringRef Name, ClangTidyContext *Context)
 : ClangTidyCheck(Name, Context),
   AllowMissingMoveFunctions(Options.get("AllowMissingMoveFunctions", 0)),
-  AllowSoleDefaultDtor(Options.get("AllowSoleDefaultDtor", 0)) {}
+  AllowSoleDefaultDtor(Options.get("AllowSoleDefaultDtor", 0)),
+  AllowMissingMoveFunctionsWhenCopyIsDeleted(
+  Options.get("AllowMissingMoveFunctionsWhenCopyIsDeleted", 0)) {}
 
 void SpecialMemberFunctionsCheck::storeOptions(
 ClangTidyOptions::OptionMap &Opts) {
   Options.store(Opts, "AllowMissingMoveFunctions", AllowMissingMoveFunctions);
   Options.store(Opts, "AllowSoleDefaultDtor", AllowSoleDefaultDtor);
+  Options.store(Opts, "AllowMissingMoveFunctionsWhenCopyIsDeleted",
+AllowMissingMoveFunctionsWhenCopyIsDeleted);
 }
 
 void SpecialMemberFunctionsCheck::registerMatchers(MatchFinder *Finder) {
@@ -103,17 +107,18 @@ void SpecialMemberFunctionsCheck::check(
 
   ClassDefId ID(MatchedDecl->getLocation(), 
std::string(MatchedDecl->getName()));
 
-  auto StoreMember = [this, &ID](SpecialMemberFunctionKind Kind) {
-llvm::SmallVectorImpl &Members =
+  auto StoreMember = [this, &ID](SpecialMemberFunctionData data) {
+llvm::SmallVectorImpl &Members =
 ClassWithSpecialMembers[ID];
-if (!llvm::is_contained(Members, Kind))
-  Members.push_back(Kind);
+if (!llvm::is_contained(Members, data))
+  Members.push_back(std::move(data));
   };
 
   if (const auto *Dtor = Result.Nodes.getNodeAs("dtor")) {
-StoreMember(Dtor->isDefaulted()
-? SpecialMemberFunctionKind::DefaultDestructor
-: SpecialMemberFunctionKind::NonDefaultDestructor);
+StoreMember({Dtor->isDefaulted()
+ ? SpecialMemberFunctionKind::DefaultDestructor
+ : SpecialMemberFunctionKind::NonDefaultDestructor,
+ Dtor->isDeleted()});
   }
 
   std::initializer_list>
@@ -123,8 +128,9 @@ void SpecialMemberFunctionsCheck::check(
   {"move-assign", SpecialMemberFunctionKind::MoveAssignment}};
 
   for (const auto &KV : Matchers)
-if (Result.Nodes.getNodeAs(KV.first)) {
-  StoreMember(KV.second);
+if (const auto *MethodDecl =
+Result.Nodes.getNodeAs(KV.first)) {
+  StoreMember({KV.second, MethodDecl->isDeleted()});
 }
 }
 
@@ -136,11 +142,19 @@ void 
SpecialMemberFunctionsCheck::onEndOfTranslationUnit() {
 
 void SpecialMemberFunctionsCheck::checkForMissingMembers(
 const ClassDefId &ID,
-llvm::ArrayRef DefinedMembers) {
+llvm::ArrayRef DefinedMembers) {
   llvm::SmallVector MissingMembers;
 
   auto HasMember = [&](SpecialMemberFunctionKind Kind) {
-return llvm::is_contained(DefinedMembers, Kind);
+return llvm::any_of(DefinedMembers, [Kind](const auto &data) {
+  return data.FunctionKind == Kind;
+});
+  };
+
+  auto IsDeleted = [&](SpecialMemberFunctionKind Kind) {
+return llvm::any_of(DefinedMembers, [Kind](const auto &data) {
+  return data.FunctionKind == Kind && data.IsDeleted;
+});
   };
 
   auto RequireMember = [&](SpecialMemberFunctionKind Kind) {
@@ -171,16 +185,23 @@ void SpecialMemberFunctionsCheck::checkForMissingMembers(
 RequireMember(SpecialMemberFunctionKind::CopyAssignment);
   }
 
-  if (RequireFive) {
+  if (RequireFive &&
+  !(AllowMissingMoveFunctionsWhenCopyIsDeleted &&

[PATCH] D75745: [clang-tidy] Added AllowMissingMoveFunctionsWhenCopyIsDeleted flag to cppcoreguidelines-special-member-functions

[Aaron Ballman via Phabricator via cfe-commits]

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

In D75745#1923913 , @pbaran wrote:

> Thank you! Could you please integrate it for me? I do not have access rights


Happy to do so; I've commit in 2f20417ef04781cd5019b9a99b85df4790bdf525 
, thank 
you for the patch!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75745



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

[PATCH] D75184: [clang-tidy] Optional inheritance of file configs from parent directories

[Dmitry Polukhin via Phabricator via cfe-commits]

DmitryPolukhin added a comment.

@alexfh could you please take another look to the diff?
All comments resolved.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75184



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

[PATCH] D75068: libclang: Add static build support for Windows

[Cristian Adam via Phabricator via cfe-commits]

cristian.adam added a comment.

Ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75068



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

[clang] 67d2591 - [AST] rename DependencyFlags.h => DependenceFlags.h, NFC

[Haojian Wu via cfe-commits]

Author: Haojian Wu
Date: 2020-03-16T13:54:21+01:00
New Revision: 67d25914b2a42cef88124fe267c7a89cce9e7987

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

LOG: [AST] rename DependencyFlags.h => DependenceFlags.h, NFC

We forgot to fix in the previous patch.

Added: 
clang/include/clang/AST/DependenceFlags.h

Modified: 
clang/include/clang/AST/Expr.h
clang/include/clang/AST/NestedNameSpecifier.h
clang/include/clang/AST/Stmt.h
clang/include/clang/AST/TemplateBase.h
clang/include/clang/AST/TemplateName.h
clang/include/clang/AST/Type.h
clang/lib/AST/Expr.cpp
clang/lib/AST/ExprCXX.cpp
clang/lib/AST/ExprConcepts.cpp
clang/lib/AST/ExprObjC.cpp
clang/lib/AST/NestedNameSpecifier.cpp
clang/lib/AST/TemplateBase.cpp
clang/lib/AST/TemplateName.cpp
clang/lib/Sema/SemaOverload.cpp
clang/lib/Serialization/ASTReaderStmt.cpp

Removed: 
clang/include/clang/AST/DependencyFlags.h



diff  --git a/clang/include/clang/AST/DependencyFlags.h 
b/clang/include/clang/AST/DependenceFlags.h
similarity index 97%
rename from clang/include/clang/AST/DependencyFlags.h
rename to clang/include/clang/AST/DependenceFlags.h
index c27016aa9aec..8aeb828b8e8c 100644
--- a/clang/include/clang/AST/DependencyFlags.h
+++ b/clang/include/clang/AST/DependenceFlags.h
@@ -1,12 +1,12 @@
-//===--- DependencyFlags.h 
===//
+//===--- DependenceFlags.h 
===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 
//===--===//
-#ifndef LLVM_CLANG_AST_DEPENDENCYFLAGS_H
-#define LLVM_CLANG_AST_DEPENDENCYFLAGS_H
+#ifndef LLVM_CLANG_AST_DEPENDENCEFLAGS_H
+#define LLVM_CLANG_AST_DEPENDENCEFLAGS_H
 
 #include "clang/Basic/BitmaskEnum.h"
 #include "llvm/ADT/BitmaskEnum.h"

diff  --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h
index b1f17631c892..372d64763c4b 100644
--- a/clang/include/clang/AST/Expr.h
+++ b/clang/include/clang/AST/Expr.h
@@ -17,7 +17,7 @@
 #include "clang/AST/ASTVector.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclAccessPair.h"
-#include "clang/AST/DependencyFlags.h"
+#include "clang/AST/DependenceFlags.h"
 #include "clang/AST/OperationKinds.h"
 #include "clang/AST/Stmt.h"
 #include "clang/AST/TemplateBase.h"

diff  --git a/clang/include/clang/AST/NestedNameSpecifier.h 
b/clang/include/clang/AST/NestedNameSpecifier.h
index e465c33e2cc5..47f5268c938b 100644
--- a/clang/include/clang/AST/NestedNameSpecifier.h
+++ b/clang/include/clang/AST/NestedNameSpecifier.h
@@ -14,7 +14,7 @@
 #ifndef LLVM_CLANG_AST_NESTEDNAMESPECIFIER_H
 #define LLVM_CLANG_AST_NESTEDNAMESPECIFIER_H
 
-#include "clang/AST/DependencyFlags.h"
+#include "clang/AST/DependenceFlags.h"
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/SourceLocation.h"
 #include "llvm/ADT/FoldingSet.h"

diff  --git a/clang/include/clang/AST/Stmt.h b/clang/include/clang/AST/Stmt.h
index d0efad03e42f..87e1d7f26a16 100644
--- a/clang/include/clang/AST/Stmt.h
+++ b/clang/include/clang/AST/Stmt.h
@@ -14,7 +14,7 @@
 #define LLVM_CLANG_AST_STMT_H
 
 #include "clang/AST/DeclGroup.h"
-#include "clang/AST/DependencyFlags.h"
+#include "clang/AST/DependenceFlags.h"
 #include "clang/AST/StmtIterator.h"
 #include "clang/Basic/CapturedStmt.h"
 #include "clang/Basic/IdentifierTable.h"

diff  --git a/clang/include/clang/AST/TemplateBase.h 
b/clang/include/clang/AST/TemplateBase.h
index e5fd1ca9dd46..1875e3eda039 100644
--- a/clang/include/clang/AST/TemplateBase.h
+++ b/clang/include/clang/AST/TemplateBase.h
@@ -14,7 +14,7 @@
 #ifndef LLVM_CLANG_AST_TEMPLATEBASE_H
 #define LLVM_CLANG_AST_TEMPLATEBASE_H
 
-#include "clang/AST/DependencyFlags.h"
+#include "clang/AST/DependenceFlags.h"
 #include "clang/AST/NestedNameSpecifier.h"
 #include "clang/AST/TemplateName.h"
 #include "clang/AST/Type.h"

diff  --git a/clang/include/clang/AST/TemplateName.h 
b/clang/include/clang/AST/TemplateName.h
index bf917788477f..9bcf2838dcf1 100644
--- a/clang/include/clang/AST/TemplateName.h
+++ b/clang/include/clang/AST/TemplateName.h
@@ -13,7 +13,7 @@
 #ifndef LLVM_CLANG_AST_TEMPLATENAME_H
 #define LLVM_CLANG_AST_TEMPLATENAME_H
 
-#include "clang/AST/DependencyFlags.h"
+#include "clang/AST/DependenceFlags.h"
 #include "clang/AST/NestedNameSpecifier.h"
 #include "clang/Basic/LLVM.h"
 #include "llvm/ADT/FoldingSet.h"

diff  --git a/clang/include/clang/AST/Type.h b/clang/include/clang/AST/Type.h
index f9a269e12d2d..e72b44fbb31f 100644
--- a/clang/include/clang/AST/Type.h
+++ b/clang/include/clang/AST/Ty

[PATCH] D76229: [clang-tidy] Added PlacementNewStorageCheck

[Karasev Nikita via Phabricator via cfe-commits]

f00kat created this revision.
f00kat added reviewers: aaron.ballman, alexfh.
f00kat added projects: clang, clang-tools-extra.
Herald added subscribers: cfe-commits, xazax.hun, mgorny.

Added new checker 'cert-mem54-cpp' that checks for CERT rule MEM54-CPP.

I have troubles writing English descriptions for the release notes, so I need 
all the help I can get :)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D76229

Files:
  clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
  clang-tools-extra/clang-tidy/cert/CMakeLists.txt
  clang-tools-extra/clang-tidy/cert/PlacementNewStorageCheck.cpp
  clang-tools-extra/clang-tidy/cert/PlacementNewStorageCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/cert-mem54-cpp.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/cert-mem54-cpp.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cert-mem54-cpp.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cert-mem54-cpp.cpp
@@ -0,0 +1,185 @@
+// RUN: %check_clang_tidy %s cert-mem54-cpp %t
+
+inline void *operator new(size_t s, void *p) noexcept {
+  (void)s;
+  return p;
+}
+
+inline void *operator new[](size_t s, void *p) noexcept {
+  (void)s;
+  return p;
+}
+
+void test1()
+{
+// bad (short is aligned to 2).
+short s1;
+::new (&s1) long;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 's1' is aligned to 2 bytes but allocated type 'long' is aligned to 4 bytes [cert-mem54-cpp]
+
+// ok (same alignment because types are the same).
+long s2;
+::new (&s2) long;
+
+// ok (long long is greater then long).
+long long s3;
+::new (&s3) long;
+
+// bad (alias for type 'short' which is aligned to 2).
+using s4t = short;
+s4t s4;
+::new (&s4) long;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 's4' is aligned to 2 bytes but allocated type 'long' is aligned to 4 bytes [cert-mem54-cpp]
+
+// ok (skip void pointer).
+void* s5 = nullptr;
+::new (s5) long;
+
+// bad (short is aligned to 2).
+short* s6 = nullptr;
+::new (s6) long;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 's6' is aligned to 2 bytes but allocated type 'long' is aligned to 4 bytes [cert-mem54-cpp]
+
+// bad (just check for pointer to pointer).
+short **s7;
+::new (*s7) long;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 's7' is aligned to 2 bytes but allocated type 'long' is aligned to 4 bytes [cert-mem54-cpp]
+}
+
+void test2()
+{
+  // ok.
+  alignas(long) unsigned char buffer1[sizeof(long)];
+  ::new (buffer1) long;
+
+  // bad (buffer is aligned to 'short' instead of 'long').
+  alignas(short) unsigned char buffer2[sizeof(long)];
+  ::new (buffer2) long;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'buffer2' is aligned to 2 bytes but allocated type 'long' is aligned to 4 bytes [cert-mem54-cpp]
+
+  // bad (not enough space).
+  alignas(long) unsigned char buffer3[sizeof(short)];
+  ::new (buffer3) long;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 2 bytes are not enough for allocation type 'long' which requires 4 bytes [cert-mem54-cpp]
+
+  // bad (still not enough space).
+  alignas(long) unsigned char buffer4[sizeof(short) + 1];
+  ::new (buffer4) long;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 3 bytes are not enough for allocation type 'long' which requires 4 bytes [cert-mem54-cpp]
+}
+
+void test3()
+{
+// ok (100 % 4 == 0).
+::new ((size_t*)100) long;
+
+// bad (100 % 4 == 2).
+::new ((size_t*)102) long;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: address is aligned to 2 bytes instead of 4 bytes [cert-mem54-cpp]
+
+::new (reinterpret_cast(100)) long;
+
+::new (reinterpret_cast(102)) long;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: address is aligned to 2 bytes instead of 4 bytes [cert-mem54-cpp]
+
+::new ((size_t*)(100 + 0)) long;
+
+::new ((size_t*)(100 + 2)) long;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: address is aligned to 2 bytes instead of 4 bytes [cert-mem54-cpp]
+}
+
+short* test4_f1()
+{
+return nullptr;
+}
+
+short& test4_f2()
+{
+static short v;
+return v;
+}
+
+void test4()
+{
+::new (test4_f1()) long;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'test4_f1' is aligned to 2 bytes but allocated type 'long' is aligned to 4 bytes [cert-mem54-cpp]
+
+::new (&test4_f2()) long;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'test4_f2' is aligned to 2 bytes but allocated type 'long' is aligned to 4 bytes [cert-mem54-cpp]
+
+short* (*p1)();
+p1 = test4_f1;
+::new (p1()) long;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'p1' is aligned to 2 bytes but allocated type 'long' is aligned to 4 bytes [cert-mem54-cpp]
+}
+
+void test5()
+{
+  struct S
+  {
+  short a;
+  };
+
+  // bad (not enough space).
+  const unsigned N = 32;
+  alignas(S) unsigned char bu

[PATCH] D76083: [clang-tidy] Expand the list of functions in bugprone-unused-return-value

[Joe Ranieri via Phabricator via cfe-commits]

jranieri-grammatech added inline comments.



Comment at: clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp:57
+   "::std::map::lower_bound;"
+   "::std::move;"
+   "::std::multimap::equal_range;"

alexfh wrote:
> This will also affect "the other std::move" 
> (https://en.cppreference.com/w/cpp/algorithm/move).
The ambiguity here is unfortunate, but I'll remove it from the list.



Comment at: clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp:98
+   "::access;"
+   "::bind;"
+   "::connect;"

alexfh wrote:
> bind has a side effect and returns a success status. Thus, the result being 
> unused isn't necessarily a bug. Same for `connect`. And probably for `setjmp` 
> as well.
In terms of bind, connect, and setjmp: while I personally would say that code 
not using the return value is bugprone, the data suggests that the vast 
majority of developers are using these functions in the intended manner and the 
false-positive rate should be low.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76083



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

[PATCH] D76083: [clang-tidy] Expand the list of functions in bugprone-unused-return-value

[Joe Ranieri via Phabricator via cfe-commits]

jranieri-grammatech updated this revision to Diff 250545.
jranieri-grammatech marked an inline comment as done.
jranieri-grammatech added a comment.

Removing std::move.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76083

Files:
  clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp


Index: clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
@@ -43,7 +43,90 @@
"::std::unique;"
"::std::unique_ptr::release;"
"::std::basic_string::empty;"
-   "::std::vector::empty")) {}
+   "::std::vector::empty;"
+   "::std::back_inserter;"
+   "::std::distance;"
+   "::std::find;"
+   "::std::find_if;"
+   "::std::inserter;"
+   "::std::lower_bound;"
+   "::std::make_pair;"
+   "::std::map::count;"
+   "::std::map::find;"
+   "::std::map::lower_bound;"
+   "::std::multimap::equal_range;"
+   "::std::multimap::upper_bound;"
+   "::std::set::count;"
+   "::std::set::find;"
+   "::std::setfill;"
+   "::std::setprecision;"
+   "::std::setw;"
+   "::std::upper_bound;"
+   "::std::vector::at;"
+   // C standard library
+   "::bsearch;"
+   "::ferror;"
+   "::feof;"
+   "::isalnum;"
+   "::isalpha;"
+   "::isblank;"
+   "::iscntrl;"
+   "::isdigit;"
+   "::isgraph;"
+   "::islower;"
+   "::isprint;"
+   "::ispunct;"
+   "::isspace;"
+   "::isupper;"
+   "::iswalnum;"
+   "::iswprint;"
+   "::iswspace;"
+   "::isxdigit;"
+   "::memchr;"
+   "::memcmp;"
+   "::strcmp;"
+   "::strcoll;"
+   "::strncmp;"
+   "::strpbrk;"
+   "::strrchr;"
+   "::strspn;"
+   "::strstr;"
+   "::wcscmp;"
+   // POSIX
+   "::access;"
+   "::bind;"
+   "::connect;"
+   "::difftime;"
+   "::dlsym;"
+   "::fnmatch;"
+   "::getaddrinfo;"
+   "::getopt;"
+   "::htonl;"
+   "::htons;"
+   "::iconv_open;"
+   "::inet_addr;"
+   "::isascii;"
+   "::isatty;"
+   "::mmap;"
+   "::newlocale;"
+   "::openat;"
+   "::pathconf;"
+   "::pthread_equal;"
+   "::pthread_getspecific;"
+   "::pthread_mutex_trylock;"
+   "::readdir;"
+   "::readlink;"
+   "::recvmsg;"
+   "::regexec;"
+   "::scandir;"
+   "::semget;"
+   "::setjmp;"
+   "::shm_op

[PATCH] D76196: [ASTMatchers] Extend hasReturnValue to GNU StmtExpr

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D76196#1923597 , @njames93 wrote:

> In D76196#1923524 , @aaron.ballman 
> wrote:
>
> > I don't think this is a natural fit for the functionality. A statement 
> > expression doesn't have a return value so much as it is a value expression 
> > that can contain multiple statements. To me, at least, this is a bit like 
> > saying the comma operator has a return value because you can do `int i; i = 
> > 1, 2, 3;` and it "returns" 3.
> >
> > Can you explain a bit about why you need this matcher functionality?
>
>
> I wasn't sure where to best place this and I was well aware of the comma 
> operator acting similarly. The idea behind it is the first `N-1` statements 
> in a `StmtExpr` with `N` aren't as relevant as the final `Stmt` if its an 
> `Expr` Maybe hasFinalExpression(InnerMatcher) would be a better way 
> WDYT?


I'm still not understanding your use case, so it's a bit hard for me to tell 
whether this approach is good or not. Do you have a situation where you cannot 
match on the expression itself, you also have to know its position? I'm asking 
because we don't currently have support for querying positional statements (for 
instance, you cannot ask what the final statement in a `CompoundStmt`) and 
statement expressions are a bit strange. For instance, what would the final 
"expression" be in this legal-but-bizarre statement expression: `({int j = 12; 
j; int x;});`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76196



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

[PATCH] D75685: Add MS Mangling for OpenCL Pipe types, add mangling test.

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

Ping!


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

https://reviews.llvm.org/D75685



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

[PATCH] D72089: [Syntax] Build declarator nodes

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr2 added inline comments.



Comment at: clang/unittests/Tooling/Syntax/TreeTest.cpp:1210
+  `-;
+   )txt"},
   };

hlopko wrote:
> gribozavr2 wrote:
> > A few complex tests that combine multiple declarators would be nice 
> > (especially to test that the delayed fold logic composes correctly).
> > 
> > ```
> > char (*(*x[10])(short a, int b))[20];
> > char (*(*x[10][20])(short a, int b))[30][40];
> > void x(char a, short (*b)(int));
> > void x(char a, short (*b)(int), long (**c)(long long));
> > void (*x(char a, short (*b)(int)))(long);
> > ```
> > 
> > Also qualifiers -- or are they not implemented yet?
> > 
> > ```
> > int * const * volatile x;
> > ```
> > 
> Some of these tests actually crash, so I'll debug/fix in a separate patch.
BTW, would be also nice to have trailing return types not at the top level:

`auto x(char a, auto (*b)(int) -> short) -> void;`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72089



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

[PATCH] D75470: [SVE] Auto-generate builtins and header for svld1.

[Nico Weber via Phabricator via cfe-commits]

thakis added inline comments.



Comment at: clang/include/clang/Basic/CMakeLists.txt:44
 
 # ARM NEON and MVE
 clang_tablegen(arm_neon.inc -gen-arm-neon-sema

Update comment to also say "SVE" and "CDE" (or just say "# ARM builtin headers")



Comment at: clang/utils/TableGen/TableGen.cpp:196
+clEnumValN(GenArmSveCodeGenMap, "gen-arm-sve-codegenmap",
+   "Generate arm_sve_codegenmap.inc for clang"),
 clEnumValN(GenArmMveHeader, "gen-arm-mve-header",

Any reason these aren't called `-gen-arm-sve-builtin-def` and 
`-gen-arm-sve-builtin-codegen` for consistency with CDE and MVE?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75470



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

[PATCH] D76196: [ASTMatchers] Extend hasReturnValue to GNU StmtExpr

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

In D76196#1924393 , @aaron.ballman 
wrote:

> I'm still not understanding your use case, so it's a bit hard for me to tell 
> whether this approach is good or not. Do you have a situation where you 
> cannot match on the expression itself, you also have to know its position? 
> I'm asking because we don't currently have support for querying positional 
> statements (for instance, you cannot ask what the final statement in a 
> `CompoundStmt`) and statement expressions are a bit strange. For instance, 
> what would the final "expression" be in this legal-but-bizarre statement 
> expression: `({int j = 12; j; int x;});`?


It would be the `int x;` but not matchable as its not an expression. The way 
`StmtExpr` works is it evaluates each `Stmt` in order and returns the result of 
the last `Stmt`, if it is an expression. otherwise it basically returns `void`. 
Possible use cases are if you want to match on say an if condition, but the 
condition is inside a `StmtExpr`. I feel like there could be a better way to 
get this behaviour but I'm not exactly sure what it is


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76196



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

[PATCH] D75470: [SVE] Auto-generate builtins and header for svld1.

[Nico Weber via Phabricator via cfe-commits]

thakis added inline comments.



Comment at: clang/utils/TableGen/SveEmitter.cpp:32
 #include "llvm/TableGen/Error.h"
+#include "clang/Basic/AArch64SVETypeFlags.h"
 #include 

Including stuff from `clang/Basic` in clang/utils/TableGen is conceptually a 
layering violation: clang-tblgen is used to generate headers included in 
clang/Basic. In this case it happens to work, but it's because you're lucky, 
and it could break for subtle reasons if the TypeFlags header starts including 
some other header in Basic that happens to include something generated.

Please restructure this so that the TableGen code doesn't need an include from 
Basic.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75470



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

[PATCH] D75682: [Analyzer][StreamChecker] Introduction of stream error handling.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:88
 struct FnDescription;
 using FnCheck = std::function;

balazske wrote:
> NoQ wrote:
> > `llvm::function_ref`?
> `function_ref`'s documentation says:
> > This class does not own the callable, so it is not in general safe to store 
> > a function_ref.
> The `FnDescription` stores these functions so it is not safe to use 
> `llvm::function_ref`?
> 
> 
> 
I think you're using it only for global function pointers, no?



Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:366
+
+  State = State->set(StreamSym, StreamState::getOpened());
+  C.addTransition(State);

balazske wrote:
> NoQ wrote:
> > So what exactly happens when you're calling `clearerr` on a closed stream? 
> > Does it actually become opened? Also what happens when you're calling it on 
> > an open-failed stream?
> The stream gets always in opened and error-free state. The `preDefault` 
> checks that the stream is opened (not closed and not open-failed) and creates 
> error node if not. If this create fails that is a problem case, the stream 
> will be opened after `clearerr`. Should check in the eval functions for 
> closed stream even if the pre-functions (in `preCall` callback) have normally 
> checked this?
Aha, ok, got it.
Let's assert it then?



Comment at: clang/test/Analysis/stream-error.c:33-34
+  int ch = fputc('a', F);
+  if (ch == EOF) {
+// FIXME: fputc not handled by checker yet, should expect TRUE
+clang_analyzer_eval(feof(F) || ferror(F)); // expected-warning {{FALSE}}

balazske wrote:
> NoQ wrote:
> > Szelethus wrote:
> > > Sure, we don't, but the bigger issue here is that we wouldn't mark `F` to 
> > > be in EOF even if we did handle it, we would also need to understand that 
> > > `ch == EOF` is the telling sign. But that also ins't in the scope of this 
> > > patch :)
> > Yeah, that's gonna be fun.
> If there is a `fputc` call (for example) that is recognized (has recognized 
> stream and no functions "fail") there is already an error and non-error 
> branch created for it. On the error branch the return value of `fputc` would 
> be **EOF** and the stream state is set to error. (The `fputc` should be later 
> modeled by this checker, not done yet.)
> 
> If the `ch == EOF` is found then to figure out if this `ch` comes from an 
> `fputc` that is called on a stream that is not recognized (it may be function 
> parameter) and then make a "tracked" stream with error state for it, this is 
> another thing.
Aha, fair enough, that's probably a well-justified state split.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75682



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

[PATCH] D76229: [clang-tidy] Added PlacementNewStorageCheck

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: clang-tools-extra/clang-tidy/cert/PlacementNewStorageCheck.cpp:19
+
+namespace {
+const ValueDecl *getDescendantValueDecl(const Stmt *TheStmt) {

Please use static instead of anonymous namespaces for functions. See LLVM 
Coding Guidelines.



Comment at: clang-tools-extra/clang-tidy/cert/PlacementNewStorageCheck.cpp:21
+const ValueDecl *getDescendantValueDecl(const Stmt *TheStmt) {
+  if (auto TheDeclRefExpr = dyn_cast(TheStmt))
+return TheDeclRefExpr->getDecl();

const auto *. Same in other places.



Comment at: clang-tools-extra/clang-tidy/cert/PlacementNewStorageCheck.cpp:47
+void PlacementNewStorageCheck::registerMatchers(MatchFinder *Finder) {
+  if (!getLangOpts().CPlusPlus)
+return;

Belongs to isLanguageVersionSupported() now.



Comment at: clang-tools-extra/clang-tidy/cert/PlacementNewStorageCheck.cpp:69
+
+if (auto TheFunctionProtoType = StorageT->getAs())
+  StorageT = TheFunctionProtoType->getReturnType();

const auto *. Same below.



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:95
+
+  Checks that placement new provided with properly aligned pointer to
+  sufficient storage capacity.

Please highlight new with double back-ticks.



Comment at: clang-tools-extra/docs/clang-tidy/checks/cert-mem54-cpp.rst:6
+
+Checks that placement new provided with properly aligned pointer to sufficient 
storage capacity.
+

Please highlight new with double back-ticks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76229



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

[PATCH] D75446: [clang][Syntax] Handle macro arguments in spelledForExpanded

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

I think per offline discussion, this was going to have some additional docs 
clarifying the contracts of this function, and maybe extra tests?

The conclusion IIRC was that this function is designed to find the text that 
you could replace in order to replace an AST node, so it's fairly conservative 
(but arg expansions are fair game assuming expanded once).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75446



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

[PATCH] D76229: [clang-tidy] Added PlacementNewStorageCheck

[Karasev Nikita via Phabricator via cfe-commits]

f00kat updated this revision to Diff 250556.
f00kat added a comment.

Fix 'const auto*' in function getDescendantValueDecl


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76229

Files:
  clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
  clang-tools-extra/clang-tidy/cert/CMakeLists.txt
  clang-tools-extra/clang-tidy/cert/PlacementNewStorageCheck.cpp
  clang-tools-extra/clang-tidy/cert/PlacementNewStorageCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/cert-mem54-cpp.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/cert-mem54-cpp.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cert-mem54-cpp.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cert-mem54-cpp.cpp
@@ -0,0 +1,185 @@
+// RUN: %check_clang_tidy %s cert-mem54-cpp %t
+
+inline void *operator new(size_t s, void *p) noexcept {
+  (void)s;
+  return p;
+}
+
+inline void *operator new[](size_t s, void *p) noexcept {
+  (void)s;
+  return p;
+}
+
+void test1()
+{
+// bad (short is aligned to 2).
+short s1;
+::new (&s1) long;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 's1' is aligned to 2 bytes but allocated type 'long' is aligned to 4 bytes [cert-mem54-cpp]
+
+// ok (same alignment because types are the same).
+long s2;
+::new (&s2) long;
+
+// ok (long long is greater then long).
+long long s3;
+::new (&s3) long;
+
+// bad (alias for type 'short' which is aligned to 2).
+using s4t = short;
+s4t s4;
+::new (&s4) long;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 's4' is aligned to 2 bytes but allocated type 'long' is aligned to 4 bytes [cert-mem54-cpp]
+
+// ok (skip void pointer).
+void* s5 = nullptr;
+::new (s5) long;
+
+// bad (short is aligned to 2).
+short* s6 = nullptr;
+::new (s6) long;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 's6' is aligned to 2 bytes but allocated type 'long' is aligned to 4 bytes [cert-mem54-cpp]
+
+// bad (just check for pointer to pointer).
+short **s7;
+::new (*s7) long;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 's7' is aligned to 2 bytes but allocated type 'long' is aligned to 4 bytes [cert-mem54-cpp]
+}
+
+void test2()
+{
+  // ok.
+  alignas(long) unsigned char buffer1[sizeof(long)];
+  ::new (buffer1) long;
+
+  // bad (buffer is aligned to 'short' instead of 'long').
+  alignas(short) unsigned char buffer2[sizeof(long)];
+  ::new (buffer2) long;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'buffer2' is aligned to 2 bytes but allocated type 'long' is aligned to 4 bytes [cert-mem54-cpp]
+
+  // bad (not enough space).
+  alignas(long) unsigned char buffer3[sizeof(short)];
+  ::new (buffer3) long;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 2 bytes are not enough for allocation type 'long' which requires 4 bytes [cert-mem54-cpp]
+
+  // bad (still not enough space).
+  alignas(long) unsigned char buffer4[sizeof(short) + 1];
+  ::new (buffer4) long;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 3 bytes are not enough for allocation type 'long' which requires 4 bytes [cert-mem54-cpp]
+}
+
+void test3()
+{
+// ok (100 % 4 == 0).
+::new ((size_t*)100) long;
+
+// bad (100 % 4 == 2).
+::new ((size_t*)102) long;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: address is aligned to 2 bytes instead of 4 bytes [cert-mem54-cpp]
+
+::new (reinterpret_cast(100)) long;
+
+::new (reinterpret_cast(102)) long;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: address is aligned to 2 bytes instead of 4 bytes [cert-mem54-cpp]
+
+::new ((size_t*)(100 + 0)) long;
+
+::new ((size_t*)(100 + 2)) long;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: address is aligned to 2 bytes instead of 4 bytes [cert-mem54-cpp]
+}
+
+short* test4_f1()
+{
+return nullptr;
+}
+
+short& test4_f2()
+{
+static short v;
+return v;
+}
+
+void test4()
+{
+::new (test4_f1()) long;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'test4_f1' is aligned to 2 bytes but allocated type 'long' is aligned to 4 bytes [cert-mem54-cpp]
+
+::new (&test4_f2()) long;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'test4_f2' is aligned to 2 bytes but allocated type 'long' is aligned to 4 bytes [cert-mem54-cpp]
+
+short* (*p1)();
+p1 = test4_f1;
+::new (p1()) long;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'p1' is aligned to 2 bytes but allocated type 'long' is aligned to 4 bytes [cert-mem54-cpp]
+}
+
+void test5()
+{
+  struct S
+  {
+  short a;
+  };
+
+  // bad (not enough space).
+  const unsigned N = 32;
+  alignas(S) unsigned char buffer1[sizeof(S) * N];
+  ::new (buffer1) S[N];
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 64 bytes are not enough for array allocation which requires 64 bytes [cert-mem54

[PATCH] D76229: [clang-tidy] Added PlacementNewStorageCheck

[Karasev Nikita via Phabricator via cfe-commits]

f00kat updated this revision to Diff 250555.
f00kat added a comment.

1. Static functions instead of anonymous namespaces.
2. const auto*.
3. Added double back-ticks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76229

Files:
  clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
  clang-tools-extra/clang-tidy/cert/CMakeLists.txt
  clang-tools-extra/clang-tidy/cert/PlacementNewStorageCheck.cpp
  clang-tools-extra/clang-tidy/cert/PlacementNewStorageCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/cert-mem54-cpp.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/cert-mem54-cpp.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cert-mem54-cpp.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cert-mem54-cpp.cpp
@@ -0,0 +1,185 @@
+// RUN: %check_clang_tidy %s cert-mem54-cpp %t
+
+inline void *operator new(size_t s, void *p) noexcept {
+  (void)s;
+  return p;
+}
+
+inline void *operator new[](size_t s, void *p) noexcept {
+  (void)s;
+  return p;
+}
+
+void test1()
+{
+// bad (short is aligned to 2).
+short s1;
+::new (&s1) long;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 's1' is aligned to 2 bytes but allocated type 'long' is aligned to 4 bytes [cert-mem54-cpp]
+
+// ok (same alignment because types are the same).
+long s2;
+::new (&s2) long;
+
+// ok (long long is greater then long).
+long long s3;
+::new (&s3) long;
+
+// bad (alias for type 'short' which is aligned to 2).
+using s4t = short;
+s4t s4;
+::new (&s4) long;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 's4' is aligned to 2 bytes but allocated type 'long' is aligned to 4 bytes [cert-mem54-cpp]
+
+// ok (skip void pointer).
+void* s5 = nullptr;
+::new (s5) long;
+
+// bad (short is aligned to 2).
+short* s6 = nullptr;
+::new (s6) long;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 's6' is aligned to 2 bytes but allocated type 'long' is aligned to 4 bytes [cert-mem54-cpp]
+
+// bad (just check for pointer to pointer).
+short **s7;
+::new (*s7) long;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 's7' is aligned to 2 bytes but allocated type 'long' is aligned to 4 bytes [cert-mem54-cpp]
+}
+
+void test2()
+{
+  // ok.
+  alignas(long) unsigned char buffer1[sizeof(long)];
+  ::new (buffer1) long;
+
+  // bad (buffer is aligned to 'short' instead of 'long').
+  alignas(short) unsigned char buffer2[sizeof(long)];
+  ::new (buffer2) long;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'buffer2' is aligned to 2 bytes but allocated type 'long' is aligned to 4 bytes [cert-mem54-cpp]
+
+  // bad (not enough space).
+  alignas(long) unsigned char buffer3[sizeof(short)];
+  ::new (buffer3) long;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 2 bytes are not enough for allocation type 'long' which requires 4 bytes [cert-mem54-cpp]
+
+  // bad (still not enough space).
+  alignas(long) unsigned char buffer4[sizeof(short) + 1];
+  ::new (buffer4) long;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 3 bytes are not enough for allocation type 'long' which requires 4 bytes [cert-mem54-cpp]
+}
+
+void test3()
+{
+// ok (100 % 4 == 0).
+::new ((size_t*)100) long;
+
+// bad (100 % 4 == 2).
+::new ((size_t*)102) long;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: address is aligned to 2 bytes instead of 4 bytes [cert-mem54-cpp]
+
+::new (reinterpret_cast(100)) long;
+
+::new (reinterpret_cast(102)) long;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: address is aligned to 2 bytes instead of 4 bytes [cert-mem54-cpp]
+
+::new ((size_t*)(100 + 0)) long;
+
+::new ((size_t*)(100 + 2)) long;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: address is aligned to 2 bytes instead of 4 bytes [cert-mem54-cpp]
+}
+
+short* test4_f1()
+{
+return nullptr;
+}
+
+short& test4_f2()
+{
+static short v;
+return v;
+}
+
+void test4()
+{
+::new (test4_f1()) long;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'test4_f1' is aligned to 2 bytes but allocated type 'long' is aligned to 4 bytes [cert-mem54-cpp]
+
+::new (&test4_f2()) long;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'test4_f2' is aligned to 2 bytes but allocated type 'long' is aligned to 4 bytes [cert-mem54-cpp]
+
+short* (*p1)();
+p1 = test4_f1;
+::new (p1()) long;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'p1' is aligned to 2 bytes but allocated type 'long' is aligned to 4 bytes [cert-mem54-cpp]
+}
+
+void test5()
+{
+  struct S
+  {
+  short a;
+  };
+
+  // bad (not enough space).
+  const unsigned N = 32;
+  alignas(S) unsigned char buffer1[sizeof(S) * N];
+  ::new (buffer1) S[N];
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 64 bytes are not enough for array al

[PATCH] D76094: [clangd] Change line break behaviour for hoverinfo

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

I understand the main goal here is preserving intended line breaks in 
documentation comments? Thanks for tackling this problem!

A design goal here is that the markup objects are the source of truth about the 
document structure. So if we've decided we want a comment to turn into multiple 
lines, we should create multiple Paragraph objects, this makes it easy to 
adjust the rendering strategy and isolate differences between text/markdown.

I think this means this splitting needs to happen on the way in instead of on 
the way out. In Hover.cpp:

  if (!Documentation.empty())
Output.addParagraph().appendText(Documentation);

would be come something like

  parseDocumentationComment(Output, Documentation);

which would ultimately add a paragraph to `Output` for each line detected in 
Documentation.

> Even though markdown doc comments in cpp are still rare, I removed the 
> markdown escaping

Please let's leave this out, at least from this patch, if it's not the primary 
goal. This is a complicated tradeoff, there's plenty of reasonably common 
patterns where we should be escaping, such as `vector`. Moreover if 
we're treating punctuation in the comments as formatting, we should likely also 
be doing so in plain-text mode.

FYI D75687  (less unneccesary escaping in 
markdown) is also in flight and likely to touch some of the same code.




Comment at: clang-tools-extra/clangd/FormattedString.cpp:103
+// converted to markdown linebreaks if \p Markdown is set.
+std::string convertLineBreaks(llvm::StringRef Input, bool Markdown = false) {
+  Input = Input.trim();

We can separate concerns more clearly by not having this function care about 
markdown.
Can it have a signature like:
 - `vector splitLines(StringRef Text)` (which wouldn't normalize 
whitespace within the lines)
 - `vector splitLines(StringRef Text)` which would
and then continue to have line rendering handled elsewhere?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76094



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

[PATCH] D71524: [analyzer] Support tainted objects in GenericTaintChecker

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

steakhal added a comment.

In D71524#1917251 , @Szelethus wrote:

> Are we sure this is what we want? If this is a heuristic, we should document 
> it well, and even then I'm not sure whether we want it. I'm also pretty sure 
> this would make the eventual change to `CallDescriptionMap` more difficult, 
> because the way taintedness is propagated around `std::basic_istream` not 
> really the property of the global `>>` operator and what its parameters are, 
> but rather the property of `std::basic_istream::operator>>`, 
> right? What do you think?


I think `CallDescription` can only identify objects/functions which has 
`IdefntifyerInfo` in them. AFAIK operators don't have such. Though somehow AST 
matchers of Clang Tidy were triggered with this: 
`functionDecl(hasName("operator>>"))`
I'm afraid it needs to be a different patch to replace with 
`CallDescriptionMap`, even though I agree with you.


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

https://reviews.llvm.org/D71524



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

[PATCH] D76122: [ARM,MVE] Add intrinsics and isel for MVE integer VMLA.

[Dave Green via Phabricator via cfe-commits]

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

LGTM




Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:14289-14292
   case Intrinsic::arm_neon_vqrshifts:
   case Intrinsic::arm_neon_vqrshiftu:
 // No immediate versions of these to check for.
 break;

I know it's not part of this patch, but what is this code for?



Comment at: llvm/lib/Target/ARM/ARMInstrMVE.td:5172
+if scalar_addend then {
+  def : Pat<(VTI.Vec (add (mul v1, v2), vs)), (VTI.Vec (Inst v1, v2, s))>;
+} else {

Can you make it so that patterns are:
  def : Pat<(VTI.Vec (add (mul v1, v2), vs)),
(VTI.Vec (Inst v1, v2, s))>;
So they are easier to read.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76122



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

[PATCH] D76229: [clang-tidy] Added PlacementNewStorageCheck

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: clang-tools-extra/clang-tidy/cert/PlacementNewStorageCheck.cpp:43
+}
+
+

Unnecessary empty line.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76229



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

[PATCH] D75470: [SVE] Auto-generate builtins and header for svld1.

[Sander de Smalen via Phabricator via cfe-commits]

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



Comment at: clang/utils/TableGen/SveEmitter.cpp:32
 #include "llvm/TableGen/Error.h"
+#include "clang/Basic/AArch64SVETypeFlags.h"
 #include 

thakis wrote:
> Including stuff from `clang/Basic` in clang/utils/TableGen is conceptually a 
> layering violation: clang-tblgen is used to generate headers included in 
> clang/Basic. In this case it happens to work, but it's because you're lucky, 
> and it could break for subtle reasons if the TypeFlags header starts 
> including some other header in Basic that happens to include something 
> generated.
> 
> Please restructure this so that the TableGen code doesn't need an include 
> from Basic.
Thanks for pointing out! The only directory that seems to have common includes 
between Clang TableGen/CodeGen is the llvm/Support directory, any objection to 
me moving the header there?



Comment at: clang/utils/TableGen/TableGen.cpp:196
+clEnumValN(GenArmSveCodeGenMap, "gen-arm-sve-codegenmap",
+   "Generate arm_sve_codegenmap.inc for clang"),
 clEnumValN(GenArmMveHeader, "gen-arm-mve-header",

thakis wrote:
> Any reason these aren't called `-gen-arm-sve-builtin-def` and 
> `-gen-arm-sve-builtin-codegen` for consistency with CDE and MVE?
Not really, I can change that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75470



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

[PATCH] D75470: [SVE] Auto-generate builtins and header for svld1.

[Raphael Isemann via Phabricator via cfe-commits]

teemperor added a comment.

This patch broke the Clang build with enabled modules (which is used by the 
LLDB bot, but every other bot is also dead: 
http://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/ )


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75470



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

[PATCH] D76220: [Syntax] Build declarator nodes

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr2 added inline comments.



Comment at: clang/include/clang/Tooling/Syntax/Nodes.h:519
+
+/// Parameter list for a function type.
+/// E.g.:

... and a trailing return type, if the function has one.



Comment at: clang/include/clang/Tooling/Syntax/Nodes.h:522
+///  `(volatile int a)` in `int foo(volatile int a);`
+///  `(int&& a)` in `int foo(int&& a);`
+///  `() -> int` in `auto foo() -> int;`

I meant:

`int foo() volatile;`
`int foo() &&;`

Of course parameters can be cv and ref-qualified, but that's not the 
interesting part here. What's interesting is function qualifiers.



Comment at: clang/include/clang/Tooling/Syntax/Nodes.h:526
+///  `() noexcept` in `int foo() noexcept;`
+///  `() throw()` in `int foo() throw();`
+///

Would you mind adding these examples to tests?



Comment at: clang/include/clang/Tooling/Syntax/Nodes.h:544
+/// E.g. `X::*` in `int X::* a = 0;`
+class MemberPointer final : public Tree {
+public:

Seems a bit weird that we have a separate node for member pointers, array 
subscripts, but not for regular pointers.

I think the rationale behind it is that since we are not building a tree 
structure out of declarators, the only token under a regular pointer declarator 
would be a star, so creating that node would not be very helpful.



Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:53
+namespace {
+/// Get start location of the Decl from the TypeLoc.
+/// E.g.:

s/of the Decl/of the declarator/



Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:61
+///
+///   (!) TypeLocs are stored inside out (in the example above `*volatile` is
+///   the TypeLoc returned by `Decl.getTypeSourceInfo()`, and `*const` is

I don't think LLVM does `(!)` type of ASCII art.

"""
It is non-trivial to get the start location because TypeLocs are stored inside 
out. In the example above, ...
"""



Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:106
+if (T.getTypePtr()->hasTrailingReturn())
+  return SourceLocation(); // avoid recursing into the suffix of 
declarator.
+return VisitTypeLoc(T);

I suspect there might be an issue here with nested function declarators because 
we will avoid recursing not only into the suffix (which I think means the 
trailing return type), but also into parameters.

If it was not necessary to recurse into parameters, why would the return 
statement below do that?



Comment at: clang/unittests/Tooling/Syntax/TreeTest.cpp:1282
+struct X {};
+int X::* a;
+)cpp",

Please also add tests with qualifiers.

`const int X::* b;`
`const int X::* const c;`

Also for member functions:

`int (X::*d)(int param);`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76220



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

[clang] 6ce537c - Revert "[SVE] Auto-generate builtins and header for svld1."

[Sander de Smalen via cfe-commits]

Author: Sander de Smalen
Date: 2020-03-16T15:22:15Z
New Revision: 6ce537ccfcfc9262ecb8472f7f3c86285b7198fb

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

LOG: Revert "[SVE] Auto-generate builtins and header for svld1."

This reverts commit 8b409eabaf755c88a7d652fe99d3ad858a4fe82a.

Reverting this patch for now because it breaks some buildbots.

Added: 


Modified: 
clang/include/clang/Basic/BuiltinsAArch64.def
clang/include/clang/Basic/CMakeLists.txt
clang/include/clang/Basic/TargetBuiltins.h
clang/include/clang/Basic/arm_sve.td
clang/lib/Basic/Targets/AArch64.cpp
clang/lib/CodeGen/CGBuiltin.cpp
clang/lib/CodeGen/CodeGenFunction.h
clang/utils/TableGen/SveEmitter.cpp
clang/utils/TableGen/TableGen.cpp
clang/utils/TableGen/TableGenBackends.h

Removed: 
clang/include/clang/Basic/AArch64SVETypeFlags.h
clang/include/clang/Basic/BuiltinsSVE.def



diff  --git a/clang/include/clang/Basic/AArch64SVETypeFlags.h 
b/clang/include/clang/Basic/AArch64SVETypeFlags.h
deleted file mode 100644
index 2b11fe6f9b2b..
--- a/clang/include/clang/Basic/AArch64SVETypeFlags.h
+++ /dev/null
@@ -1,67 +0,0 @@
-//===- AArch64SVETypeFlags.h - Flags used to generate ACLE builtins- C++ 
-*-===//
-//
-//  Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
-//  See https://llvm.org/LICENSE.txt for license information.
-//  SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===--===//
-
-#ifndef LLVM_CLANG_BASIC_AARCH64SVETYPEFLAGS_H
-#define LLVM_CLANG_BASIC_AARCH64SVETYPEFLAGS_H
-
-#include 
-
-namespace clang {
-
-/// Flags to identify the types for overloaded SVE builtins.
-class SVETypeFlags {
-  uint64_t Flags;
-
-public:
-  /// These must be kept in sync with the flags in
-  /// include/clang/Basic/arm_sve.td.
-  static const uint64_t MemEltTypeOffset = 4; // Bit offset of MemEltTypeMask
-  static const uint64_t EltTypeMask  = 0x000f;
-  static const uint64_t MemEltTypeMask   = 0x0070;
-  static const uint64_t IsLoad   = 0x0080;
-
-  enum EltType {
-Invalid,
-Int8,
-Int16,
-Int32,
-Int64,
-Float16,
-Float32,
-Float64,
-Bool8,
-Bool16,
-Bool32,
-Bool64
-  };
-
-  enum MemEltTy {
-MemEltTyDefault,
-MemEltTyInt8,
-MemEltTyInt16,
-MemEltTyInt32,
-MemEltTyInt64
-  };
-
-  SVETypeFlags(uint64_t F) : Flags(F) { }
-  SVETypeFlags(EltType ET, bool IsUnsigned) : Flags(ET) { }
-
-  EltType getEltType() const { return (EltType)(Flags & EltTypeMask); }
-  MemEltTy getMemEltType() const {
-return (MemEltTy)((Flags & MemEltTypeMask) >> MemEltTypeOffset);
-  }
-
-  bool isLoad() const { return Flags & IsLoad; }
-
-  uint64_t getBits() const { return Flags; }
-  bool isFlagSet(uint64_t Flag) const { return Flags & Flag; }
-};
-
-} // end namespace clang
-
-#endif

diff  --git a/clang/include/clang/Basic/BuiltinsAArch64.def 
b/clang/include/clang/Basic/BuiltinsAArch64.def
index f07c567053de..8f3a24c2e1f6 100644
--- a/clang/include/clang/Basic/BuiltinsAArch64.def
+++ b/clang/include/clang/Basic/BuiltinsAArch64.def
@@ -99,6 +99,19 @@ BUILTIN(__builtin_arm_tcommit, "v", "n")
 BUILTIN(__builtin_arm_tcancel, "vWUIi", "n")
 BUILTIN(__builtin_arm_ttest, "WUi", "nc")
 
+// SVE
+BUILTIN(__builtin_sve_svld1_s16, "q8sq16bSsC*", "n")
+BUILTIN(__builtin_sve_svld1_s32, "q4iq16bSiC*", "n")
+BUILTIN(__builtin_sve_svld1_s64, "q2Wiq16bSWiC*", "n")
+BUILTIN(__builtin_sve_svld1_s8, "q16Scq16bScC*", "n")
+BUILTIN(__builtin_sve_svld1_u16, "q8Usq16bUsC*", "n")
+BUILTIN(__builtin_sve_svld1_u32, "q4Uiq16bUiC*", "n")
+BUILTIN(__builtin_sve_svld1_u64, "q2UWiq16bUWiC*", "n")
+BUILTIN(__builtin_sve_svld1_u8, "q16Ucq16bUcC*", "n")
+BUILTIN(__builtin_sve_svld1_f64, "q2dq16bdC*", "n")
+BUILTIN(__builtin_sve_svld1_f32, "q4fq16bfC*", "n")
+BUILTIN(__builtin_sve_svld1_f16, "q8hq16bhC*", "n")
+
 TARGET_HEADER_BUILTIN(_BitScanForward, "UcUNi*UNi", "nh", "intrin.h", 
ALL_MS_LANGUAGES, "")
 TARGET_HEADER_BUILTIN(_BitScanReverse, "UcUNi*UNi", "nh", "intrin.h", 
ALL_MS_LANGUAGES, "")
 TARGET_HEADER_BUILTIN(_BitScanForward64, "UcUNi*ULLi", "nh", "intrin.h", 
ALL_MS_LANGUAGES, "")

diff  --git a/clang/include/clang/Basic/BuiltinsSVE.def 
b/clang/include/clang/Basic/BuiltinsSVE.def
deleted file mode 100644
index 2839ca992d98..
--- a/clang/include/clang/Basic/BuiltinsSVE.def
+++ /dev/null
@@ -1,20 +0,0 @@
-//===--- BuiltinsSVE.def - SVE Builtin function database *- C++ 
-*-===//
-//
-//  Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
-//  See https://llvm.org/LICENSE.txt for license information.
-//  SPDX-License-Identifier: Apache-2.0 WITH LLVM-except

[PATCH] D71018: [ASTImporter] Improved import of TypeSourceInfo (TypeLoc)

[Gabor Marton via Phabricator via cfe-commits]

martong updated this revision to Diff 250568.
martong added a comment.

- Remove redundant VisitQualifiedTypeLoc and VisitAttributedTypeLoc


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71018

Files:
  clang/lib/AST/ASTImporter.cpp

Index: clang/lib/AST/ASTImporter.cpp
===
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -8106,7 +8106,7 @@
 
 namespace clang {
 
-#define IMPORT_TYPE_LOC(NAME)  \
+#define IMPORT_TYPE_LOC_OR_RETURN_ERROR(NAME)  \
   if (auto ImpOrErr = Importer.Import(From.get##NAME()))   \
 To.set##NAME(*ImpOrErr);   \
   else \
@@ -8123,126 +8123,80 @@
   TypeLocImporter(ASTImporter &Importer, TypeLoc ToL)
   : Importer(Importer), ToL(ToL) {}
 
-  Error VisitTypeSpecTypeLoc(TypeSpecTypeLoc From) {
-auto To = ToL.castAs();
-IMPORT_TYPE_LOC(NameLoc);
+  Error VisitTypeLoc(TypeLoc From) {
+// The visitor does not call directly VisitTypeSpecTypeLoc, because that is
+// not a leaf node in the hierarchy (i.e. there is no such enum value in
+// TypeNodes.inc).
+if (auto FTS = From.getAs())
+  return VisitTypeSpecTypeLoc(FTS);
 return Error::success();
   }
 
-  Error VisitTypedefTypeLoc(TypedefTypeLoc From) {
-return VisitTypeSpecTypeLoc(From);
-  }
-
-  Error VisitInjectedClassNameTypeLoc(InjectedClassNameTypeLoc From) {
-return VisitTypeSpecTypeLoc(From);
-  }
-
-  Error VisitUnresolvedUsingTypeLoc(UnresolvedUsingTypeLoc From) {
-return VisitTypeSpecTypeLoc(From);
-  }
-
-  Error VisitRecordTypeLoc(RecordTypeLoc From) {
-return VisitTypeSpecTypeLoc(From);
-  }
-
-  Error VisitEnumTypeLoc(EnumTypeLoc From) {
-return VisitTypeSpecTypeLoc(From);
-  }
-
-  Error VisitTemplateTypeParmTypeLoc(TemplateTypeParmTypeLoc From) {
-return VisitTypeSpecTypeLoc(From);
-  }
-
-  Error VisitSubstTemplateTypeParmTypeLoc(SubstTemplateTypeParmTypeLoc From) {
-return VisitTypeSpecTypeLoc(From);
-  }
-
-  Error
-  VisitSubstTemplateTypeParmPackTypeLoc(SubstTemplateTypeParmPackTypeLoc From) {
-return VisitTypeSpecTypeLoc(From);
-  }
-
-  Error VisitVectorTypeLoc(VectorTypeLoc From) {
-return VisitTypeSpecTypeLoc(From);
-  }
-
-  Error VisitDependentVectorTypeLoc(DependentVectorTypeLoc From) {
-return VisitTypeSpecTypeLoc(From);
-  }
-
-  Error VisitExtVectorTypeLoc(ExtVectorTypeLoc From) {
-return VisitTypeSpecTypeLoc(From);
-  }
-
-  Error
-  VisitDependentSizedExtVectorTypeLoc(DependentSizedExtVectorTypeLoc From) {
-return VisitTypeSpecTypeLoc(From);
-  }
-
-  Error VisitComplexTypeLoc(ComplexTypeLoc From) {
-return VisitTypeSpecTypeLoc(From);
-  }
-
-  Error VisitDecltypeTypeLoc(DecltypeTypeLoc From) {
-return VisitTypeSpecTypeLoc(From);
-  }
-
-  Error VisitDeducedTypeLoc(DeducedTypeLoc From) {
-return VisitTypeSpecTypeLoc(From);
-  }
-
-  Error VisitAutoTypeLoc(AutoTypeLoc From) {
-return VisitTypeSpecTypeLoc(From);
+  Error VisitTypeSpecTypeLoc(TypeSpecTypeLoc From) {
+auto To = ToL.castAs();
+IMPORT_TYPE_LOC_OR_RETURN_ERROR(NameLoc);
+return Error::success();
   }
 
   Error VisitBuiltinTypeLoc(BuiltinTypeLoc From) {
 auto To = ToL.castAs();
-IMPORT_TYPE_LOC(BuiltinLoc);
-// FIXME: Import other attributes?
+IMPORT_TYPE_LOC_OR_RETURN_ERROR(BuiltinLoc);
 return Error::success();
   }
 
   Error VisitParenTypeLoc(ParenTypeLoc From) {
 auto To = ToL.castAs();
 
-IMPORT_TYPE_LOC(LParenLoc);
-IMPORT_TYPE_LOC(RParenLoc);
+IMPORT_TYPE_LOC_OR_RETURN_ERROR(LParenLoc);
+IMPORT_TYPE_LOC_OR_RETURN_ERROR(RParenLoc);
+
+return Error::success();
+  }
 
+  Error VisitPointerTypeLoc(PointerTypeLoc From) {
+auto To = ToL.castAs();
+IMPORT_TYPE_LOC_OR_RETURN_ERROR(StarLoc);
 return Error::success();
   }
 
   Error VisitBlockPointerTypeLoc(BlockPointerTypeLoc From) {
 auto To = ToL.castAs();
-IMPORT_TYPE_LOC(CaretLoc);
+IMPORT_TYPE_LOC_OR_RETURN_ERROR(CaretLoc);
 return Error::success();
   }
 
   Error VisitMemberPointerTypeLoc(MemberPointerTypeLoc From) {
 auto To = ToL.castAs();
-IMPORT_TYPE_LOC(StarLoc);
+IMPORT_TYPE_LOC_OR_RETURN_ERROR(StarLoc);
+return Error::success();
+  }
+
+  Error VisitObjCObjectPointerTypeLoc(ObjCObjectPointerTypeLoc From) {
+auto To = ToL.castAs();
+IMPORT_TYPE_LOC_OR_RETURN_ERROR(StarLoc);
 return Error::success();
   }
 
   Error VisitLValueReferenceTypeLoc(LValueReferenceTypeLoc From) {
 auto To = ToL.castAs();
-IMPORT_TYPE_LOC(AmpLoc);
+IMPORT_TYPE_LOC_OR_RETURN_ERROR(AmpLoc);
 return Error::success();
   }
 
   Error VisitRValueReferenceTypeLoc(RValueReferenceTypeLoc From) {
 au

[PATCH] D71018: [ASTImporter] Improved import of TypeSourceInfo (TypeLoc)

[Gabor Marton via Phabricator via cfe-commits]

martong updated this revision to Diff 250570.
martong added a comment.

- Fix the base commit of this patch


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71018

Files:
  clang/lib/AST/ASTImporter.cpp
  clang/unittests/AST/ASTImporterTest.cpp

Index: clang/unittests/AST/ASTImporterTest.cpp
===
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -5845,6 +5845,53 @@
   EXPECT_TRUE(isa(To->getReturnType()));
 }
 
+struct ImportTypeLoc : ASTImporterOptionSpecificTestBase {};
+
+TEST_P(ImportTypeLoc, Function) {
+  Decl *FromTU = getTuDecl(
+  R"(
+  int f(int p) {
+return 1;
+  }
+  )",
+  Lang_CXX11, "input0.cc");
+  FunctionDecl *FromF = FirstDeclMatcher().match(
+  FromTU, functionDecl(hasName("f")));
+  FunctionDecl *ToF = Import(FromF, Lang_CXX11);
+  ASSERT_TRUE(ToF);
+  FunctionProtoTypeLoc TL =
+  ToF->getTypeSourceInfo()->getTypeLoc().castAs();
+  EXPECT_EQ(TL.getNumParams(), 1u);
+  EXPECT_TRUE(TL.getParam(0));
+}
+
+TEST_P(ImportTypeLoc, Lambda) {
+  // In this test case, first the CXXRecordDecl of the lambda is imported, then
+  // the operator().
+  Decl *FromTU = getTuDecl(
+  R"(
+  auto l1 = [](unsigned lp) { return 1; };
+  int f(int p) {
+return l1(p);
+  }
+  )",
+  Lang_CXX11, "input0.cc");
+  FunctionDecl *FromF = FirstDeclMatcher().match(
+  FromTU, functionDecl(hasName("f")));
+  FunctionDecl *ToF = Import(FromF, Lang_CXX11);
+  ASSERT_TRUE(ToF);
+
+  TranslationUnitDecl *ToTU = ToAST->getASTContext().getTranslationUnitDecl();
+  CXXMethodDecl *ToLambdaMD = FirstDeclMatcher()
+  .match(ToTU, lambdaExpr())
+  ->getCallOperator();
+  FunctionProtoTypeLoc TL = ToLambdaMD->getTypeSourceInfo()
+->getTypeLoc()
+.castAs();
+  EXPECT_EQ(TL.getNumParams(), 1u);
+  EXPECT_TRUE(TL.getParam(0));
+}
+
 INSTANTIATE_TEST_CASE_P(ParameterizedTests, ASTImporterLookupTableTest,
 DefaultTestValuesForRunOptions, );
 
@@ -5903,5 +5950,8 @@
 INSTANTIATE_TEST_CASE_P(ParameterizedTests, LLDBLookupTest,
 DefaultTestValuesForRunOptions, );
 
+INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportTypeLoc,
+DefaultTestValuesForRunOptions, );
+
 } // end namespace ast_matchers
 } // end namespace clang
Index: clang/lib/AST/ASTImporter.cpp
===
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -42,6 +42,7 @@
 #include "clang/AST/TemplateName.h"
 #include "clang/AST/Type.h"
 #include "clang/AST/TypeLoc.h"
+#include "clang/AST/TypeLocVisitor.h"
 #include "clang/AST/TypeVisitor.h"
 #include "clang/AST/UnresolvedSet.h"
 #include "clang/Basic/Builtins.h"
@@ -695,6 +696,8 @@
 // that type is declared inside the body of the function.
 // E.g. auto f() { struct X{}; return X(); }
 bool hasAutoReturnTypeDeclaredInside(FunctionDecl *D);
+
+friend class TypeLocImporter;
   };
 
 template 
@@ -3275,6 +3278,15 @@
 FromReturnTy, FromFPT->getParamTypes(), FromEPI);
   }
 
+  // Import the function parameters.
+  SmallVector Parameters;
+  for (auto P : D->parameters()) {
+if (Expected ToPOrErr = import(P))
+  Parameters.push_back(*ToPOrErr);
+else
+  return ToPOrErr.takeError();
+  }
+
   QualType T;
   TypeSourceInfo *TInfo;
   SourceLocation ToInnerLocStart, ToEndLoc;
@@ -3288,15 +3300,6 @@
   else
 return Imp.takeError();
 
-  // Import the function parameters.
-  SmallVector Parameters;
-  for (auto P : D->parameters()) {
-if (Expected ToPOrErr = import(P))
-  Parameters.push_back(*ToPOrErr);
-else
-  return ToPOrErr.takeError();
-  }
-
   // Create the imported function.
   FunctionDecl *ToFunction = nullptr;
   if (auto *FromConstructor = dyn_cast(D)) {
@@ -3402,16 +3405,6 @@
   }
   ToFunction->setParams(Parameters);
 
-  // We need to complete creation of FunctionProtoTypeLoc manually with setting
-  // params it refers to.
-  if (TInfo) {
-if (auto ProtoLoc =
-TInfo->getTypeLoc().IgnoreParens().getAs()) {
-  for (unsigned I = 0, N = Parameters.size(); I != N; ++I)
-ProtoLoc.setParam(I, Parameters[I]);
-}
-  }
-
   // Import the describing template function, if any.
   if (FromFT) {
 auto ToFTOrErr = import(FromFT);
@@ -8111,20 +8104,305 @@
   return ToContext.getQualifiedType(*ToTOrErr, FromT.getLocalQualifiers());
 }
 
+namespace clang {
+
+#define IMPORT_TYPE_LOC_OR_RETURN_ERROR(NAME)  \
+  if (auto ImpOrErr = Importer.Import(From.get##NAME()))   \
+To.set##NAME(*ImpOrErr);   \
+  else 

[PATCH] D76196: [ASTMatchers] Extend hasReturnValue to GNU StmtExpr

[Nathan James via Phabricator via cfe-commits]

njames93 updated this revision to Diff 250569.
njames93 added a comment.

- Rename matcher to hasFinalExpr
- Don't review it yet


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76196

Files:
  clang/docs/LibASTMatchersReference.html
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp


Index: clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -3006,6 +3006,13 @@
   EXPECT_FALSE(matches("void F() { return; }", RetVal));
 }
 
+TEST(StatementMatcher, hasFinalExpr) {
+  StatementMatcher RetVal = stmtExpr(hasFinalExpr(binaryOperator()));
+  EXPECT_TRUE(matches("void F() { ({int a, b; a + b;}); }", RetVal));
+  EXPECT_FALSE(matches("void F() { ({int a; a;}); }", RetVal));
+  EXPECT_FALSE(matches("void F() { ({int a;}); }", RetVal));
+}
+
 TEST(StatementMatcher, ForFunction) {
   const auto CppString1 =
 "struct PosVec {"
Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -6729,6 +6729,24 @@
   return false;
 }
 
+/// Matches the result value expression of a GNU statement expression.
+///
+/// Given
+/// \code
+///   ({int *Ptr; *Ptr;});
+/// \endcode
+/// hasFinalExpr(unaryOperator())
+///   matches '({int *Ptr; *Ptr;})'
+/// with unaryOperator()
+///   matching '*Ptr'
+AST_MATCHER_P(StmtExpr, hasFinalExpr, internal::Matcher, InnerMatcher) {
+  if (const CompoundStmt *CS = Node.getSubStmt()) {
+if (const auto *E = dyn_cast_or_null(CS->body_back()))
+  return InnerMatcher.matches(*E, Finder, Builder);
+  }
+  return false;
+}
+
 /// Matches CUDA kernel call expression.
 ///
 /// Example matches,
Index: clang/docs/LibASTMatchersReference.html
===
--- clang/docs/LibASTMatchersReference.html
+++ clang/docs/LibASTMatchersReference.html
@@ -7188,6 +7188,18 @@
 
 
 
+MatcherStmtExpr>hasFinalExprMatcherExpr> 
InnerMatcher
+Matches the result 
value expression of a GNU statement expression.
+
+Given
+  ({int *Ptr; *Ptr;});
+hasFinalExpr(unaryOperator())
+  matches '({int *Ptr; *Ptr;})'
+with unaryOperator()
+  matching '*Ptr'
+
+
+
 MatcherStmt>alignOfExprMatcherUnaryExprOrTypeTraitExpr>
  InnerMatcher
 Same as 
unaryExprOrTypeTraitExpr, but only matching
 alignof.


Index: clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -3006,6 +3006,13 @@
   EXPECT_FALSE(matches("void F() { return; }", RetVal));
 }
 
+TEST(StatementMatcher, hasFinalExpr) {
+  StatementMatcher RetVal = stmtExpr(hasFinalExpr(binaryOperator()));
+  EXPECT_TRUE(matches("void F() { ({int a, b; a + b;}); }", RetVal));
+  EXPECT_FALSE(matches("void F() { ({int a; a;}); }", RetVal));
+  EXPECT_FALSE(matches("void F() { ({int a;}); }", RetVal));
+}
+
 TEST(StatementMatcher, ForFunction) {
   const auto CppString1 =
 "struct PosVec {"
Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -6729,6 +6729,24 @@
   return false;
 }
 
+/// Matches the result value expression of a GNU statement expression.
+///
+/// Given
+/// \code
+///   ({int *Ptr; *Ptr;});
+/// \endcode
+/// hasFinalExpr(unaryOperator())
+///   matches '({int *Ptr; *Ptr;})'
+/// with unaryOperator()
+///   matching '*Ptr'
+AST_MATCHER_P(StmtExpr, hasFinalExpr, internal::Matcher, InnerMatcher) {
+  if (const CompoundStmt *CS = Node.getSubStmt()) {
+if (const auto *E = dyn_cast_or_null(CS->body_back()))
+  return InnerMatcher.matches(*E, Finder, Builder);
+  }
+  return false;
+}
+
 /// Matches CUDA kernel call expression.
 ///
 /// Example matches,
Index: clang/docs/LibASTMatchersReference.html
===
--- clang/docs/LibASTMatchersReference.html
+++ clang/docs/LibASTMatchersReference.html
@@ -7188,6 +7188,18 @@
 
 
 
+MatcherStmtExpr>hasFinalExprMatcherExpr> InnerMatcher
+Matches the result value expression of a GNU st

[PATCH] D76229: [clang-tidy] Added PlacementNewStorageCheck

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

Also fix the test case, premerge found a failure




Comment at: clang-tools-extra/clang-tidy/cert/PlacementNewStorageCheck.cpp:23
+
+  for (const Stmt *Child : TheStmt->children()) {
+if (const auto *TheDeclRefExpr = dyn_cast(Child))

For better or worse children can sometimes contain nullptr, best to check for 
that first



Comment at: clang-tools-extra/clang-tidy/cert/PlacementNewStorageCheck.cpp:52
+
+void PlacementNewStorageCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto *NewExpr = Result.Nodes.getNodeAs("new");

Not gonna say this is blocking anything, but this is quite a large function 
that could be made smaller (more readable) by moving some of those lambdas out 
of the function


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76229



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

[PATCH] D71018: [ASTImporter] Improved import of TypeSourceInfo (TypeLoc)

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

In D71018#1924604 , @martong wrote:

> - Remove redundant VisitQualifiedTypeLoc and VisitAttributedTypeLoc


I realized that the mentioned functions are redundant, because we have a while 
loop which iterates over all related typelocs (`getNextTypeLoc`).
Also, it is better to remove the `llvm_unreachable` because there are many 
`TypeLoc`s that we just don't handle, e.g. `TypeOfExprTypeLoc`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71018



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

[clang] 8a593e2 - [AST] Correct the CXXOperatorCallExpr source range.

[Haojian Wu via cfe-commits]

Author: Haojian Wu
Date: 2020-03-16T16:51:10+01:00
New Revision: 8a593e29ab9b2799ded3d85a8110d51d4283f128

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

LOG: [AST] Correct the CXXOperatorCallExpr source range.

Summary:
Previously, the range for "->" CXXOperatorCallExpr is the range of the
class object (not including the operator!), e.g. "[[vector_ptr]]->size()".

This patch includes the range of the operator, which fixes the issue
where clangd doesn't go to the overloaded operator "->" definition.

Reviewers: sammccall

Reviewed By: sammccall

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

Tags: #clang

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

Added: 


Modified: 
clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
clang-tools-extra/clangd/unittests/SelectionTests.cpp
clang-tools-extra/clangd/unittests/XRefsTests.cpp
clang/lib/AST/ExprCXX.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp 
b/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
index cbec56fbf7a5..fefa56ae5ef5 100644
--- a/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
@@ -649,9 +649,14 @@ StringRef LoopConvertCheck::getContainerString(ASTContext 
*Context,
const ForStmt *Loop,
const Expr *ContainerExpr) {
   StringRef ContainerString;
-  if (isa(ContainerExpr->IgnoreParenImpCasts())) {
+  ContainerExpr = ContainerExpr->IgnoreParenImpCasts();
+  if (isa(ContainerExpr)) {
 ContainerString = "this";
   } else {
+// For CXXOperatorCallExpr (e.g. vector_ptr->size()), its first argument is
+// the class object (vector_ptr) we are targeting.
+if (const auto* E = dyn_cast(ContainerExpr))
+  ContainerExpr = E->getArg(0);
 ContainerString =
 getStringFromRange(Context->getSourceManager(), Context->getLangOpts(),
ContainerExpr->getSourceRange());

diff  --git a/clang-tools-extra/clangd/unittests/SelectionTests.cpp 
b/clang-tools-extra/clangd/unittests/SelectionTests.cpp
index 7316dea42bd2..e1c873a22b13 100644
--- a/clang-tools-extra/clangd/unittests/SelectionTests.cpp
+++ b/clang-tools-extra/clangd/unittests/SelectionTests.cpp
@@ -380,6 +380,14 @@ TEST(SelectionTest, CommonAncestor) {
   {"struct foo { [[^~foo()]]; };", "CXXDestructorDecl"},
   // FIXME: The following to should be class itself instead.
   {"struct foo { [[fo^o(){}]] };", "CXXConstructorDecl"},
+
+  {R"cpp(
+struct S1 { void f(); };
+struct S2 { S1 * operator->(); };
+void test(S2 s2) {
+  s2[[-^>]]f();
+}
+  )cpp", "DeclRefExpr"} // DeclRefExpr to the "operator->" method.
   };
   for (const Case &C : Cases) {
 Annotations Test(C.Code);

diff  --git a/clang-tools-extra/clangd/unittests/XRefsTests.cpp 
b/clang-tools-extra/clangd/unittests/XRefsTests.cpp
index d387fd219a08..1e687fd109e3 100644
--- a/clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ b/clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -452,6 +452,14 @@ TEST(LocateSymbol, All) {
 }
   )cpp",
 
+  R"cpp(
+struct S1 { void f(); };
+struct S2 { S1 * $decl[[operator]]->(); };
+void test(S2 s2) {
+  s2-^>f();
+}
+  )cpp",
+
   R"cpp(// Declaration of explicit template specialization
 template 
 struct $decl[[Foo]] {};

diff  --git a/clang/lib/AST/ExprCXX.cpp b/clang/lib/AST/ExprCXX.cpp
index 69db80f452aa..b507afd6551d 100644
--- a/clang/lib/AST/ExprCXX.cpp
+++ b/clang/lib/AST/ExprCXX.cpp
@@ -637,7 +637,7 @@ SourceRange CXXOperatorCallExpr::getSourceRangeImpl() const 
{
   // Postfix operator
   return SourceRange(getArg(0)->getBeginLoc(), getOperatorLoc());
   } else if (Kind == OO_Arrow) {
-return getArg(0)->getSourceRange();
+return SourceRange(getArg(0)->getBeginLoc(), getOperatorLoc());
   } else if (Kind == OO_Call) {
 return SourceRange(getArg(0)->getBeginLoc(), getRParenLoc());
   } else if (Kind == OO_Subscript) {



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

[PATCH] D76184: [OpenMP][NFC] Remove the need to include `OpenMPClause.h`

[Reid Kleckner via Phabricator via cfe-commits]

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

lgtm, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76184



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

[PATCH] D76220: [Syntax] Build declarator nodes

[Marcel Hlopko via Phabricator via cfe-commits]

hlopko updated this revision to Diff 250574.
hlopko marked 8 inline comments as done.
hlopko added a comment.

Resolving comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76220

Files:
  clang/include/clang/Tooling/Syntax/Nodes.h
  clang/lib/Tooling/Syntax/BuildTree.cpp
  clang/lib/Tooling/Syntax/Nodes.cpp
  clang/unittests/Tooling/Syntax/TreeTest.cpp

Index: clang/unittests/Tooling/Syntax/TreeTest.cpp
===
--- clang/unittests/Tooling/Syntax/TreeTest.cpp
+++ clang/unittests/Tooling/Syntax/TreeTest.cpp
@@ -174,17 +174,21 @@
 *: TranslationUnit
 |-SimpleDeclaration
 | |-int
-| |-main
-| |-(
-| |-)
+| |-SimpleDeclarator
+| | |-main
+| | `-ParametersAndQualifiers
+| |   |-(
+| |   `-)
 | `-CompoundStatement
 |   |-{
 |   `-}
 `-SimpleDeclaration
   |-void
-  |-foo
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-foo
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 `-}
@@ -201,9 +205,11 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-int
-  |-main
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-main
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 |-IfStatement
@@ -246,9 +252,11 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-void
-  |-test
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-test
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 |-ForStatement
@@ -268,18 +276,21 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-void
-  |-test
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-test
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 |-DeclarationStatement
 | |-SimpleDeclaration
 | | |-int
-| | |-a
-| | |-=
-| | `-UnknownExpression
-| |   `-10
+| | `-SimpleDeclarator
+| |   |-a
+| |   |-=
+| |   `-UnknownExpression
+| | `-10
 | `-;
 `-}
 )txt"},
@@ -287,9 +298,11 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-void
-  |-test
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-test
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 |-EmptyStatement
@@ -309,9 +322,11 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-void
-  |-test
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-test
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 |-SwitchStatement
@@ -345,9 +360,11 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-void
-  |-test
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-test
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 |-WhileStatement
@@ -375,9 +392,11 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-int
-  |-test
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-test
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 |-ReturnStatement
@@ -398,26 +417,31 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-void
-  |-test
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-test
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 |-DeclarationStatement
 | |-SimpleDeclaration
 | | |-int
-| | |-a
-| | |-[
-| | |-UnknownExpression
-| | | `-3
-| | `-]
+| | `-SimpleDeclarator
+| |   |-a
+| |   `-ArraySubscript
+| | |-[
+| | |-UnknownExpression
+| | | `-3
+| | `-]
 | `-;
 |-RangeBasedForStatement
 | |-for
 | |-(
 | |-SimpleDeclaration
 | | |-int
-| | |-x
+| | |-SimpleDeclarator
+| | | `-x
 | | `-:
 | |-UnknownExpression
 | | `-a
@@ -433,9 +457,11 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-void
-  |-main
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-main
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 |-UnknownStatement
@@ -460,9 +486,11 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-void
-  |-test
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-test
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 |-ExpressionStatement
@@ -500,10 +528,12 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-int
-  |-*
-  |-a
+  |-SimpleDeclarator
+  | |-*
+  | `-a
   |-,
-  |-b
+  |-SimpleDeclarator
+  | `-b
   `-;
   )txt"},
   {R"cpp(
@@ -514,10 +544,12 @@
 `-SimpleDeclaration
   |-typedef
   |-int
-  |-*
-  |-a
+  |-SimpleDeclarator
+  | |-*
+  | `-a
   |-,
-  |-b
+  |-SimpleDeclarator
+  | `-b
   `-;
   )txt"},
   // Multiple declarators inside a statement.
@@ -531,27 +563,33 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-void
-  |-foo
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-foo
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 |-DeclarationStatement
 | |-SimpleDeclaration
 | | |-int
-| | |-*
-| | |-a
+| | |-SimpleDeclarator
+| | | |-*
+| |

[PATCH] D76220: [Syntax] Build declarator nodes

[Marcel Hlopko via Phabricator via cfe-commits]

hlopko added inline comments.



Comment at: clang/include/clang/Tooling/Syntax/Nodes.h:522
+///  `(volatile int a)` in `int foo(volatile int a);`
+///  `(int&& a)` in `int foo(int&& a);`
+///  `() -> int` in `auto foo() -> int;`

gribozavr2 wrote:
> I meant:
> 
> `int foo() volatile;`
> `int foo() &&;`
> 
> Of course parameters can be cv and ref-qualified, but that's not the 
> interesting part here. What's interesting is function qualifiers.
Rewrote the comment, added tests.



Comment at: clang/include/clang/Tooling/Syntax/Nodes.h:526
+///  `() noexcept` in `int foo() noexcept;`
+///  `() throw()` in `int foo() throw();`
+///

gribozavr2 wrote:
> Would you mind adding these examples to tests?
We already have them (see "Exception specification in parameter lists" and 
"Trailing return type in parameter lists").



Comment at: clang/include/clang/Tooling/Syntax/Nodes.h:544
+/// E.g. `X::*` in `int X::* a = 0;`
+class MemberPointer final : public Tree {
+public:

gribozavr2 wrote:
> Seems a bit weird that we have a separate node for member pointers, array 
> subscripts, but not for regular pointers.
> 
> I think the rationale behind it is that since we are not building a tree 
> structure out of declarators, the only token under a regular pointer 
> declarator would be a star, so creating that node would not be very helpful.
Yeah I agree, and even though not very helpful, it looks more polished to me, 
so I'll add upload a separate patch with that.



Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:61
+///
+///   (!) TypeLocs are stored inside out (in the example above `*volatile` is
+///   the TypeLoc returned by `Decl.getTypeSourceInfo()`, and `*const` is

gribozavr2 wrote:
> I don't think LLVM does `(!)` type of ASCII art.
> 
> """
> It is non-trivial to get the start location because TypeLocs are stored 
> inside out. In the example above, ...
> """
Ack, I saw some usages of (!) so was using it too :) Removed in the whole file.

Updated the comment.



Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:106
+if (T.getTypePtr()->hasTrailingReturn())
+  return SourceLocation(); // avoid recursing into the suffix of 
declarator.
+return VisitTypeLoc(T);

gribozavr2 wrote:
> I suspect there might be an issue here with nested function declarators 
> because we will avoid recursing not only into the suffix (which I think means 
> the trailing return type), but also into parameters.
> 
> If it was not necessary to recurse into parameters, why would the return 
> statement below do that?
Noted, will address in a separate patch.



Comment at: clang/unittests/Tooling/Syntax/TreeTest.cpp:1282
+struct X {};
+int X::* a;
+)cpp",

gribozavr2 wrote:
> Please also add tests with qualifiers.
> 
> `const int X::* b;`
> `const int X::* const c;`
> 
> Also for member functions:
> 
> `int (X::*d)(int param);`
These 2 tests break asserts, will fix in a separate patch.

const int X::* const c;
int (X::*d)(int param);


Added a test for `const int X::* b;` into this one.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76220



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

[PATCH] D75470: [SVE] Auto-generate builtins and header for svld1.

[Nico Weber via Phabricator via cfe-commits]

thakis added inline comments.



Comment at: clang/utils/TableGen/SveEmitter.cpp:32
 #include "llvm/TableGen/Error.h"
+#include "clang/Basic/AArch64SVETypeFlags.h"
 #include 

sdesmalen wrote:
> thakis wrote:
> > Including stuff from `clang/Basic` in clang/utils/TableGen is conceptually 
> > a layering violation: clang-tblgen is used to generate headers included in 
> > clang/Basic. In this case it happens to work, but it's because you're 
> > lucky, and it could break for subtle reasons if the TypeFlags header starts 
> > including some other header in Basic that happens to include something 
> > generated.
> > 
> > Please restructure this so that the TableGen code doesn't need an include 
> > from Basic.
> Thanks for pointing out! The only directory that seems to have common 
> includes between Clang TableGen/CodeGen is the llvm/Support directory, any 
> objection to me moving the header there?
That seems like a strange place for this header.

Maybe you can rework things so that you don't have to share a header between 
clang's tablegen and clang's Basic? No other tablegen output so far has needed 
that. (see e.g. the `  /// These must be kept in sync with the flags in 
utils/TableGen/NeonEmitter.h.` line in TargetBuiltins.h).

If that isn't possible at all, I suppose you could put the .h file in 
clang/utils/TableGen and also make clang-tblgen write the .h file and use the 
written .h file in Basic.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75470



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

[PATCH] D76125: [clangd] Decouple preambleworker from astworker, NFCI

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clang-tools-extra/clangd/TUScheduler.cpp:155
 namespace {
+/// Responsible for building and providing access to the preamble of a TU. This
+/// worker doesn't guarantee that each preamble request will be built, in case

This doc comment doesn't mention "thread" once :-)



Comment at: clang-tools-extra/clangd/TUScheduler.cpp:156
+/// Responsible for building and providing access to the preamble of a TU. This
+/// worker doesn't guarantee that each preamble request will be built, in case
+/// of multiple preamble requests, it will only build the last one. Once stop 
is

move documentation of requestBuild() to that function?



Comment at: clang-tools-extra/clangd/TUScheduler.cpp:157
+/// worker doesn't guarantee that each preamble request will be built, in case
+/// of multiple preamble requests, it will only build the last one. Once stop 
is
+/// called it will build any remaining request and will exit the run loop.

Move documentation of stop() semantics to stop()?



Comment at: clang-tools-extra/clangd/TUScheduler.cpp:159
+/// called it will build any remaining request and will exit the run loop.
+class PreambleWorker {
+public:

we may want to call this PreambleThread? Current name emphasizes peer 
relationship with ASTWorker, but current code has parent/child relationship.



Comment at: clang-tools-extra/clangd/TUScheduler.cpp:171
+
+  void requestBuild(CompilerInvocation *CI, ParseInputs PI) {
+// If compiler invocation was broken, just fail out early.

or just update()?



Comment at: clang-tools-extra/clangd/TUScheduler.cpp:201
+  /// will unblock even after an unsuccessful build.
+  void waitForFirstPreamble() const { BuiltFirstPreamble.wait(); }
+

I think all the members in this class with "preamble" in the name can drop that 
word without any loss, e.g. `PreambleWorker::waitForFirst()`



Comment at: clang-tools-extra/clangd/TUScheduler.cpp:205
+  /// built or latest attempt resulted in a failure.
+  std::shared_ptr getLatestBuiltPreamble() const {
+std::lock_guard Lock(Mutex);

latest()?



Comment at: clang-tools-extra/clangd/TUScheduler.cpp:218
+PreambleRequested.wait(Lock, [this] { return PreambleReq || Done; });
+// No request means we are done.
+if (!PreambleReq)

Why do we rebuild the preamble if stop is requested?



Comment at: clang-tools-extra/clangd/TUScheduler.cpp:265
+  /// Updates the TUStatus and emits it. Only called in the worker thread.
+  void emitTUStatus(TUAction Action,
+const TUStatus::BuildDetails *Details = nullptr) {

as discussed a bit in chat, I think this part needs some more thought. 
Currently the ASTWorker and PreambleWorker emit data for the same file with 
some interleaving that's hard to reason about. The state needs an owner.

(e.g. consider a class that holds a TUStatus and exposes an Event, 
with threadsafe setPreambleAction and setWorkerAction methods, or so)



Comment at: clang-tools-extra/clangd/TUScheduler.cpp:291
+  /// valid for Req.
+  std::shared_ptr buildPreamble(Request Req) {
+assert(Req.CI && "Got preamble request with null compiler invocation");

just build(), to avoid confusion clangd::buildPreamble?

Also consider merging with updateLatestBuiltPreamble, both are small and 
private and they always go together.



Comment at: clang-tools-extra/clangd/TUScheduler.cpp:295
+std::shared_ptr OldPreamble =
+Inputs.ForceRebuild ? std::shared_ptr()
+: getLatestBuiltPreamble();

(nullptr should work here?)



Comment at: clang-tools-extra/clangd/TUScheduler.cpp:312
+
+  mutable std::mutex Mutex;
+  bool Done = false;

Yikes, lots of state :-)
I'd consider merging the two CVs - I find keeping the events separate and 
reasoning when you need notify_one vs _all doesn't seem to pay off, may just be 
the way I think about queues. Up to you.



Comment at: clang-tools-extra/clangd/TUScheduler.cpp:322
+  mutable std::condition_variable PreambleBuilt;
+  std::shared_ptr LastBuiltPreamble;
+

nit: last vs latest, be consistent



Comment at: clang-tools-extra/clangd/TUScheduler.cpp:325
+  Notification BuiltFirstPreamble;
+  const PathRef FileName;
+  ParsingCallbacks &Callbacks;

please make this owning for simplicity, this isn't a lightweight object anyway



Comment at: clang-tools-extra/clangd/TUScheduler.cpp:600
 Inputs, CompilerInvocationDiagConsumer, &CC1Args);
+auto OldPreamble = PW.getLatestBuiltPreamble();
+PW.requestBuild(Invocation.get(

[PATCH] D76128: [AST] Correct the CXXOperatorCallExpr source range.

[Haojian Wu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG8a593e29ab9b: [AST] Correct the CXXOperatorCallExpr source 
range. (authored by hokein).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76128

Files:
  clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
  clang-tools-extra/clangd/unittests/SelectionTests.cpp
  clang-tools-extra/clangd/unittests/XRefsTests.cpp
  clang/lib/AST/ExprCXX.cpp


Index: clang/lib/AST/ExprCXX.cpp
===
--- clang/lib/AST/ExprCXX.cpp
+++ clang/lib/AST/ExprCXX.cpp
@@ -637,7 +637,7 @@
   // Postfix operator
   return SourceRange(getArg(0)->getBeginLoc(), getOperatorLoc());
   } else if (Kind == OO_Arrow) {
-return getArg(0)->getSourceRange();
+return SourceRange(getArg(0)->getBeginLoc(), getOperatorLoc());
   } else if (Kind == OO_Call) {
 return SourceRange(getArg(0)->getBeginLoc(), getRParenLoc());
   } else if (Kind == OO_Subscript) {
Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -452,6 +452,14 @@
 }
   )cpp",
 
+  R"cpp(
+struct S1 { void f(); };
+struct S2 { S1 * $decl[[operator]]->(); };
+void test(S2 s2) {
+  s2-^>f();
+}
+  )cpp",
+
   R"cpp(// Declaration of explicit template specialization
 template 
 struct $decl[[Foo]] {};
Index: clang-tools-extra/clangd/unittests/SelectionTests.cpp
===
--- clang-tools-extra/clangd/unittests/SelectionTests.cpp
+++ clang-tools-extra/clangd/unittests/SelectionTests.cpp
@@ -380,6 +380,14 @@
   {"struct foo { [[^~foo()]]; };", "CXXDestructorDecl"},
   // FIXME: The following to should be class itself instead.
   {"struct foo { [[fo^o(){}]] };", "CXXConstructorDecl"},
+
+  {R"cpp(
+struct S1 { void f(); };
+struct S2 { S1 * operator->(); };
+void test(S2 s2) {
+  s2[[-^>]]f();
+}
+  )cpp", "DeclRefExpr"} // DeclRefExpr to the "operator->" method.
   };
   for (const Case &C : Cases) {
 Annotations Test(C.Code);
Index: clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
===
--- clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
@@ -649,9 +649,14 @@
const ForStmt *Loop,
const Expr *ContainerExpr) {
   StringRef ContainerString;
-  if (isa(ContainerExpr->IgnoreParenImpCasts())) {
+  ContainerExpr = ContainerExpr->IgnoreParenImpCasts();
+  if (isa(ContainerExpr)) {
 ContainerString = "this";
   } else {
+// For CXXOperatorCallExpr (e.g. vector_ptr->size()), its first argument is
+// the class object (vector_ptr) we are targeting.
+if (const auto* E = dyn_cast(ContainerExpr))
+  ContainerExpr = E->getArg(0);
 ContainerString =
 getStringFromRange(Context->getSourceManager(), Context->getLangOpts(),
ContainerExpr->getSourceRange());


Index: clang/lib/AST/ExprCXX.cpp
===
--- clang/lib/AST/ExprCXX.cpp
+++ clang/lib/AST/ExprCXX.cpp
@@ -637,7 +637,7 @@
   // Postfix operator
   return SourceRange(getArg(0)->getBeginLoc(), getOperatorLoc());
   } else if (Kind == OO_Arrow) {
-return getArg(0)->getSourceRange();
+return SourceRange(getArg(0)->getBeginLoc(), getOperatorLoc());
   } else if (Kind == OO_Call) {
 return SourceRange(getArg(0)->getBeginLoc(), getRParenLoc());
   } else if (Kind == OO_Subscript) {
Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -452,6 +452,14 @@
 }
   )cpp",
 
+  R"cpp(
+struct S1 { void f(); };
+struct S2 { S1 * $decl[[operator]]->(); };
+void test(S2 s2) {
+  s2-^>f();
+}
+  )cpp",
+
   R"cpp(// Declaration of explicit template specialization
 template 
 struct $decl[[Foo]] {};
Index: clang-tools-extra/clangd/unittests/SelectionTests.cpp
===
--- clang-tools-extra/clangd/unittests/SelectionTests.cpp
+++ clang-tools-extra/clangd/unittests/SelectionTests.cpp
@@ -380,6 +380,14 @@
   {"struct foo { [[^~foo()]]; };", "CXXDestructorDecl"},
   // FIXME: The following to should be class itself instead.
   {"struct foo

[PATCH] D73638: [AST] Move dependence computations into a separate file

[Sam McCall via Phabricator via cfe-commits]

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

(I do think there are ways we could write the dependency computations more 
clearly, but don't want to diverge further from the existing code)




Comment at: clang/include/clang/AST/TemplateBase.h:672
   TemplateArgumentLoc *OutArgArray);
+  // FIXME: cleanup the unsued Deps.
   void initializeFrom(SourceLocation TemplateKWLoc,

hokein wrote:
> sammccall wrote:
> > I don't understand this comment, can you expand it?
> The parameter `Deps` is the result populated by this method, the caller 
> doesn't need it since we have the `computeDependencies`.
Thanks, but I meant can you expand the comment :-)
(The explanation you just gave is fine)



Comment at: clang/lib/AST/ComputeDependence.cpp:171
+  E->getSubExpr()->getDependence();
+  if (E->getType()->isDependentType())
+D |= ExprDependence::Type;

the old code for type-dependence is `E->getType()->isDependentType()`.
The new one is the typedependence of `E->getType() | E->getSubExpr() | 
E->getWrittenTypeInfo()->getType()`.

E->getType() is always the getNonLValueExprType of the 
E->getWrittenTypeInfo()->getType(), and that function doesn't change the 
dependence of a type, so I think you can drop the E->getType() here.

You still have E->getSubExpr() as a driver for type-dependence, whith I think 
is incorrect. va_arg(list, t) returns type t regardless of the type of list.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73638



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

[PATCH] D76234: clang: Simplify implementation of Type::isXXXType()

[Yannic Bonenberger via Phabricator via cfe-commits]

yannic created this revision.
yannic added reviewers: klimek, rsmith.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D76234

Files:
  clang/include/clang/AST/Type.h


Index: clang/include/clang/AST/Type.h
===
--- clang/include/clang/AST/Type.h
+++ clang/include/clang/AST/Type.h
@@ -6777,9 +6777,9 @@
 }
 
 inline bool Type::isSpecificBuiltinType(unsigned K) const {
-  if (const BuiltinType *BT = getAs())
-if (BT->getKind() == (BuiltinType::Kind) K)
-  return true;
+  if (const BuiltinType *BT = getAs()) {
+return BT->getKind() == static_cast(K);
+  }
   return false;
 }
 
@@ -6798,9 +6798,7 @@
 
 inline bool Type::isSpecificPlaceholderType(unsigned K) const {
   assert(BuiltinType::isPlaceholderTypeKind((BuiltinType::Kind) K));
-  if (const auto *BT = dyn_cast(this))
-return (BT->getKind() == (BuiltinType::Kind) K);
-  return false;
+  return isSpecificBuiltinType(K);
 }
 
 inline bool Type::isNonOverloadPlaceholderType() const {
@@ -6810,34 +6808,24 @@
 }
 
 inline bool Type::isVoidType() const {
-  if (const auto *BT = dyn_cast(CanonicalType))
-return BT->getKind() == BuiltinType::Void;
-  return false;
+  return isSpecificBuiltinType(BuiltinType::Void);
 }
 
 inline bool Type::isHalfType() const {
-  if (const auto *BT = dyn_cast(CanonicalType))
-return BT->getKind() == BuiltinType::Half;
   // FIXME: Should we allow complex __fp16? Probably not.
-  return false;
+  return isSpecificBuiltinType(BuiltinType::Half);
 }
 
 inline bool Type::isFloat16Type() const {
-  if (const auto *BT = dyn_cast(CanonicalType))
-return BT->getKind() == BuiltinType::Float16;
-  return false;
+  return isSpecificBuiltinType(BuiltinType::Float16);
 }
 
 inline bool Type::isFloat128Type() const {
-  if (const auto *BT = dyn_cast(CanonicalType))
-return BT->getKind() == BuiltinType::Float128;
-  return false;
+  return isSpecificBuiltinType(BuiltinType::Float128);
 }
 
 inline bool Type::isNullPtrType() const {
-  if (const auto *BT = getAs())
-return BT->getKind() == BuiltinType::NullPtr;
-  return false;
+  return isSpecificBuiltinType(BuiltinType::NullPtr);
 }
 
 bool IsEnumDeclComplete(EnumDecl *);


Index: clang/include/clang/AST/Type.h
===
--- clang/include/clang/AST/Type.h
+++ clang/include/clang/AST/Type.h
@@ -6777,9 +6777,9 @@
 }
 
 inline bool Type::isSpecificBuiltinType(unsigned K) const {
-  if (const BuiltinType *BT = getAs())
-if (BT->getKind() == (BuiltinType::Kind) K)
-  return true;
+  if (const BuiltinType *BT = getAs()) {
+return BT->getKind() == static_cast(K);
+  }
   return false;
 }
 
@@ -6798,9 +6798,7 @@
 
 inline bool Type::isSpecificPlaceholderType(unsigned K) const {
   assert(BuiltinType::isPlaceholderTypeKind((BuiltinType::Kind) K));
-  if (const auto *BT = dyn_cast(this))
-return (BT->getKind() == (BuiltinType::Kind) K);
-  return false;
+  return isSpecificBuiltinType(K);
 }
 
 inline bool Type::isNonOverloadPlaceholderType() const {
@@ -6810,34 +6808,24 @@
 }
 
 inline bool Type::isVoidType() const {
-  if (const auto *BT = dyn_cast(CanonicalType))
-return BT->getKind() == BuiltinType::Void;
-  return false;
+  return isSpecificBuiltinType(BuiltinType::Void);
 }
 
 inline bool Type::isHalfType() const {
-  if (const auto *BT = dyn_cast(CanonicalType))
-return BT->getKind() == BuiltinType::Half;
   // FIXME: Should we allow complex __fp16? Probably not.
-  return false;
+  return isSpecificBuiltinType(BuiltinType::Half);
 }
 
 inline bool Type::isFloat16Type() const {
-  if (const auto *BT = dyn_cast(CanonicalType))
-return BT->getKind() == BuiltinType::Float16;
-  return false;
+  return isSpecificBuiltinType(BuiltinType::Float16);
 }
 
 inline bool Type::isFloat128Type() const {
-  if (const auto *BT = dyn_cast(CanonicalType))
-return BT->getKind() == BuiltinType::Float128;
-  return false;
+  return isSpecificBuiltinType(BuiltinType::Float128);
 }
 
 inline bool Type::isNullPtrType() const {
-  if (const auto *BT = getAs())
-return BT->getKind() == BuiltinType::NullPtr;
-  return false;
+  return isSpecificBuiltinType(BuiltinType::NullPtr);
 }
 
 bool IsEnumDeclComplete(EnumDecl *);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D60748: Fix i386 struct and union parameter alignment

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Oh, I see you just updated your patch months ago without ever mentioning that 
it was ready for review.

It sounds to me like GCC retroactively added a switch specifying which version 
of the ABI to follow on this point, somewhat confusingly called `-malign-data`. 
 That's probably the right move here for us, too, especially since FreeBSD says 
they'd like to use it.  That also means the condition of when to use your new 
logic will have to change; basically, we need a CodeGenOption for this that 
will default to the old ABI, and the driver will pass down a different default 
on Linux.


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

https://reviews.llvm.org/D60748



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

[PATCH] D75063: [analyzer] StdLibraryFunctionsChecker: Add NotNull Arg Constraint

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

balazske added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:189
+ValueConstraintPtr negate() const override {
+  NotNullConstraint tmp(*this);
+  tmp.CannotBeNull = !this->CannotBeNull;

Is `Tmp` better name?



Comment at: clang/test/Analysis/std-c-library-functions-arg-constraints.c:59
+fread(buf, sizeof(int), 10, fp); // expected-warning{{Function argument 
constraint is not satisfied}}
+}

One difficulty is that `fread` can work if the `buf` is 0 and `count` is 0 too. 
There is for sure code that uses this feature.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75063



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

[PATCH] D75682: [Analyzer][StreamChecker] Introduction of stream error handling.

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

balazske updated this revision to Diff 250588.
balazske marked 2 inline comments as done.
balazske added a comment.

Addressed some comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75682

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

Index: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -19,17 +19,21 @@
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h"
-#include 
 
 using namespace clang;
 using namespace ento;
 using namespace std::placeholders;
 
+#define ASSERT_STREAMSTATE_OPENED  \
+  assert(SS->isOpened() && \
+ "Create of error node failed (only way to get this state)?");
+
 namespace {
 
 /// Full state information about a stream pointer.
 struct StreamState {
   /// State of a stream symbol.
+  /// FIXME: We need maybe an "escaped" state later.
   enum KindTy {
 Opened, /// Stream is opened.
 Closed, /// Closed stream (an invalid stream pointer after it was closed).
@@ -37,37 +41,44 @@
   } State;
 
   /// The error state of a stream.
+  /// Valid only if the stream is opened.
   enum ErrorKindTy {
-NoError,/// No error flag is set or stream is not open.
-EofError,   /// EOF condition (`feof` is true).
-OtherError, /// Other (non-EOF) error (`ferror` is true).
-AnyError/// EofError or OtherError, used if exact error is unknown.
+/// No error flag is set (or stream is not open).
+NoError,
+/// EOF condition (`feof` is true).
+FEof,
+/// Other generic (non-EOF) error (`ferror` is true).
+FError,
   } ErrorState = NoError;
 
   bool isOpened() const { return State == Opened; }
   bool isClosed() const { return State == Closed; }
   bool isOpenFailed() const { return State == OpenFailed; }
 
-  bool isNoError() const { return ErrorState == NoError; }
-  bool isEofError() const { return ErrorState == EofError; }
-  bool isOtherError() const { return ErrorState == OtherError; }
-  bool isAnyError() const { return ErrorState == AnyError; }
+  bool isNoError() const {
+assert(State == Opened && "Error undefined for closed stream.");
+return ErrorState == NoError;
+  }
+  bool isFEof() const {
+assert(State == Opened && "Error undefined for closed stream.");
+return ErrorState == FEof;
+  }
+  bool isFError() const {
+assert(State == Opened && "Error undefined for closed stream.");
+return ErrorState == FError;
+  }
 
   bool operator==(const StreamState &X) const {
+// In not opened state error should always NoError.
 return State == X.State && ErrorState == X.ErrorState;
   }
 
   static StreamState getOpened() { return StreamState{Opened}; }
   static StreamState getClosed() { return StreamState{Closed}; }
   static StreamState getOpenFailed() { return StreamState{OpenFailed}; }
-  static StreamState getOpenedWithAnyError() {
-return StreamState{Opened, AnyError};
-  }
-  static StreamState getOpenedWithEofError() {
-return StreamState{Opened, EofError};
-  }
-  static StreamState getOpenedWithOtherError() {
-return StreamState{Opened, OtherError};
+  static StreamState getOpenedWithFEof() { return StreamState{Opened, FEof}; }
+  static StreamState getOpenedWithFError() {
+return StreamState{Opened, FError};
   }
 
   void Profile(llvm::FoldingSetNodeID &ID) const {
@@ -102,7 +113,7 @@
 DefinedSVal makeRetVal(CheckerContext &C, const CallExpr *CE) {
   assert(CE && "Expecting a call expression.");
 
-  const LocationContext *LCtx = C.getPredecessor()->getLocationContext();
+  const LocationContext *LCtx = C.getLocationContext();
   return C.getSValBuilder()
   .conjureSymbolVal(nullptr, CE, LCtx, C.blockCount())
   .castAs();
@@ -135,9 +146,12 @@
   {{"fsetpos", 2}, {&StreamChecker::preDefault, nullptr, 0}},
   {{"clearerr", 1},
{&StreamChecker::preDefault, &StreamChecker::evalClearerr, 0}},
-  {{"feof", 1}, {&StreamChecker::preDefault, &StreamChecker::evalFeof, 0}},
+  {{"feof", 1},
+   {&StreamChecker::preDefault,
+&StreamChecker::evalFeofFerror<&StreamState::isFEof>, 0}},
   {{"ferror", 1},
-   {&StreamChecker::preDefault, &StreamChecker::evalFerror, 0}},
+   {&StreamChecker::preDefault,
+&StreamChecker::evalFeofFerror<&StreamState::isFError>, 0}},
   {{"fileno", 1}, {&StreamChecker::preDefault, nullptr, 0}},
   };
 
@@ -161,11 +175,9 @@
   void evalClearerr(const FnDescription *Desc, const CallEvent &Call,
 CheckerContext &C) const;
 
-  void evalFeof(const FnDesc

[PATCH] D75682: [Analyzer][StreamChecker] Introduction of stream error handling.

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

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



Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:88
 struct FnDescription;
 using FnCheck = std::function;

NoQ wrote:
> balazske wrote:
> > NoQ wrote:
> > > `llvm::function_ref`?
> > `function_ref`'s documentation says:
> > > This class does not own the callable, so it is not in general safe to 
> > > store a function_ref.
> > The `FnDescription` stores these functions so it is not safe to use 
> > `llvm::function_ref`?
> > 
> > 
> > 
> I think you're using it only for global function pointers, no?
Probably can work but I tried it and got compile errors.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75682



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

[PATCH] D76122: [ARM,MVE] Add intrinsics and isel for MVE integer VMLA.

[Simon Tatham via Phabricator via cfe-commits]

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



Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:14289-14292
   case Intrinsic::arm_neon_vqrshifts:
   case Intrinsic::arm_neon_vqrshiftu:
 // No immediate versions of these to check for.
 break;

dmgreen wrote:
> I know it's not part of this patch, but what is this code for?
I have no idea! They date from rG2e076c4e02fb99 in 2009 which first introduced 
NEON support, and even then, the switch they appear in had a `default: break` 
clause.

My best guess is that they're intended as a kind of documentation. They signal 
to the reader: "I didn't //forget// about these intrinsics, I carefully 
considered them, decided they didn't need handling here, and included a comment 
saying why not."


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76122



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

[PATCH] D76220: [Syntax] Build declarator nodes

[Marcel Hlopko via Phabricator via cfe-commits]

hlopko updated this revision to Diff 250591.
hlopko added a comment.

Fix clang tidy warning.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76220

Files:
  clang/include/clang/Tooling/Syntax/Nodes.h
  clang/lib/Tooling/Syntax/BuildTree.cpp
  clang/lib/Tooling/Syntax/Nodes.cpp
  clang/unittests/Tooling/Syntax/TreeTest.cpp

Index: clang/unittests/Tooling/Syntax/TreeTest.cpp
===
--- clang/unittests/Tooling/Syntax/TreeTest.cpp
+++ clang/unittests/Tooling/Syntax/TreeTest.cpp
@@ -174,17 +174,21 @@
 *: TranslationUnit
 |-SimpleDeclaration
 | |-int
-| |-main
-| |-(
-| |-)
+| |-SimpleDeclarator
+| | |-main
+| | `-ParametersAndQualifiers
+| |   |-(
+| |   `-)
 | `-CompoundStatement
 |   |-{
 |   `-}
 `-SimpleDeclaration
   |-void
-  |-foo
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-foo
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 `-}
@@ -201,9 +205,11 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-int
-  |-main
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-main
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 |-IfStatement
@@ -246,9 +252,11 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-void
-  |-test
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-test
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 |-ForStatement
@@ -268,18 +276,21 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-void
-  |-test
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-test
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 |-DeclarationStatement
 | |-SimpleDeclaration
 | | |-int
-| | |-a
-| | |-=
-| | `-UnknownExpression
-| |   `-10
+| | `-SimpleDeclarator
+| |   |-a
+| |   |-=
+| |   `-UnknownExpression
+| | `-10
 | `-;
 `-}
 )txt"},
@@ -287,9 +298,11 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-void
-  |-test
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-test
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 |-EmptyStatement
@@ -309,9 +322,11 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-void
-  |-test
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-test
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 |-SwitchStatement
@@ -345,9 +360,11 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-void
-  |-test
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-test
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 |-WhileStatement
@@ -375,9 +392,11 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-int
-  |-test
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-test
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 |-ReturnStatement
@@ -398,26 +417,31 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-void
-  |-test
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-test
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 |-DeclarationStatement
 | |-SimpleDeclaration
 | | |-int
-| | |-a
-| | |-[
-| | |-UnknownExpression
-| | | `-3
-| | `-]
+| | `-SimpleDeclarator
+| |   |-a
+| |   `-ArraySubscript
+| | |-[
+| | |-UnknownExpression
+| | | `-3
+| | `-]
 | `-;
 |-RangeBasedForStatement
 | |-for
 | |-(
 | |-SimpleDeclaration
 | | |-int
-| | |-x
+| | |-SimpleDeclarator
+| | | `-x
 | | `-:
 | |-UnknownExpression
 | | `-a
@@ -433,9 +457,11 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-void
-  |-main
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-main
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 |-UnknownStatement
@@ -460,9 +486,11 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-void
-  |-test
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-test
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 |-ExpressionStatement
@@ -500,10 +528,12 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-int
-  |-*
-  |-a
+  |-SimpleDeclarator
+  | |-*
+  | `-a
   |-,
-  |-b
+  |-SimpleDeclarator
+  | `-b
   `-;
   )txt"},
   {R"cpp(
@@ -514,10 +544,12 @@
 `-SimpleDeclaration
   |-typedef
   |-int
-  |-*
-  |-a
+  |-SimpleDeclarator
+  | |-*
+  | `-a
   |-,
-  |-b
+  |-SimpleDeclarator
+  | `-b
   `-;
   )txt"},
   // Multiple declarators inside a statement.
@@ -531,27 +563,33 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-void
-  |-foo
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-foo
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 |-DeclarationStatement
 | |-SimpleDeclaration
 | | |-int
-| | |-*
-| | |-a
+| | |-SimpleDeclarator
+| | | |-*
+| | | `-a
 | | |-,
-| | `-b
+   

[PATCH] D76062: [PATCH] [ARM] ARMv8.6-a command-line + BFloat16 Asm Support

[Ties Stuij via Phabricator via cfe-commits]

stuij commandeered this revision.
stuij added a reviewer: LukeGeeson.
stuij added a comment.

Commandeered because Luke is on vacation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76062



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

[PATCH] D76130: [PPC][AIX] Implement variadic function handling in LowerFormalArguments_AIX.

[Sean Fertile via Phabricator via cfe-commits]

sfertile added a comment.

As a first step, I suggest we break the clang changes and the LLVM changes into 
2 separate patches.


Repository:
  rZORG LLVM Github Zorg

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

https://reviews.llvm.org/D76130



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

[PATCH] D76218: [Sema][SVE] Reject "new" with sizeless types

[Eli Friedman via Phabricator via cfe-commits]

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76218



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

[PATCH] D75010: [OpenMP] Adding InaccessibleMemOnly and InaccessibleMemOrArgMemOnly for runtime calls.

[Stefan Stipanovic via Phabricator via cfe-commits]

sstefan1 updated this revision to Diff 250599.
sstefan1 added a comment.

more tests

couldn't get `update_test_checks.py` to update function signatures, even with 
the flag.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75010

Files:
  clang/test/OpenMP/barrier_codegen.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
  llvm/test/Transforms/OpenMP/add_attributes.ll
  llvm/test/Transforms/OpenMP/parallel_deletion.ll

Index: llvm/test/Transforms/OpenMP/parallel_deletion.ll
===
--- llvm/test/Transforms/OpenMP/parallel_deletion.ll
+++ llvm/test/Transforms/OpenMP/parallel_deletion.ll
@@ -1,4 +1,4 @@
-; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --function-signature
 ; RUN: opt -S -attributor -openmpopt -attributor-disable=false < %s | FileCheck %s
 ; RUN: opt -S -passes='attributor,cgscc(openmpopt)' -attributor-disable=false < %s | FileCheck %s
 ;
@@ -142,7 +142,7 @@
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:[[A:%.*]] = alloca i32, align 4
 ; CHECK-NEXT:[[TMP:%.*]] = bitcast i32* [[A]] to i8*
-; CHECK-NEXT:call void @llvm.lifetime.start.p0i8(i64 4, i8* nonnull align 4 dereferenceable(4) [[TMP]])
+; CHECK-NEXT:call void @llvm.lifetime.start.p0i8(i64 4, i8* nonnull align 4 dereferenceable(4) [[TMP]]) #0
 ; CHECK-NEXT:store i32 0, i32* [[A]], align 4
 ; CHECK-NEXT:call void (%struct.ident_t*, i32, void (i32*, i32*, ...)*, ...) @__kmpc_fork_call(%struct.ident_t* nonnull align 8 dereferenceable(24) @0, i32 1, void (i32*, i32*, ...)* nonnull bitcast (void (i32*, i32*, i32*)* @.omp_outlined..3 to void (i32*, i32*, ...)*), i32* nocapture nofree nonnull align 4 dereferenceable(4) [[A]])
 ; CHECK-NEXT:call void (%struct.ident_t*, i32, void (i32*, i32*, ...)*, ...) @__kmpc_fork_call(%struct.ident_t* nonnull align 8 dereferenceable(24) @0, i32 1, void (i32*, i32*, ...)* nonnull bitcast (void (i32*, i32*, i32*)* @.omp_outlined..4 to void (i32*, i32*, ...)*), i32* nocapture nonnull align 4 dereferenceable(4) [[A]])
Index: llvm/test/Transforms/OpenMP/add_attributes.ll
===
--- llvm/test/Transforms/OpenMP/add_attributes.ll
+++ llvm/test/Transforms/OpenMP/add_attributes.ll
@@ -9,6 +9,7 @@
 
 %struct.omp_lock_t = type { i8* }
 %struct.omp_nest_lock_t = type { i8* }
+%struct.ident_t = type { i32, i32, i32, i32, i8* }
 
 define void @call_all(i32 %schedule, %struct.omp_lock_t* %lock, i32 %lock_hint, %struct.omp_nest_lock_t* %nest_lock, i32 %i, i8* %s, i64 %st, i8* %vp, double %d, i32 %proc_bind, i64 %allocator_handle, i8* %cp, i64 %event_handle, i32 %pause_resource) {
 entry:
@@ -460,6 +461,40 @@
 
 declare dso_local i32 @omp_get_supported_active_levels()
 
+declare void @__kmpc_barrier(%struct.ident_t*, i32)
+
+declare i32 @__kmpc_cancel(%struct.ident_t*, i32, i32)
+
+declare i32 @__kmpc_cancel_barrier(%struct.ident_t*, i32)
+
+declare void @__kmpc_flush(%struct.ident_t*)
+
+declare i32 @__kmpc_global_thread_num(%struct.ident_t*)
+
+declare void @__kmpc_fork_call(%struct.ident_t*, i32, void (i32*, i32*, ...)*, ...)
+
+declare i32 @__kmpc_omp_taskwait(%struct.ident_t*, i32)
+
+declare i32 @__kmpc_omp_taskyield(%struct.ident_t*, i32, i32)
+
+declare void @__kmpc_push_num_threads(%struct.ident_t*, i32, i32)
+
+declare void @__kmpc_push_proc_bind(%struct.ident_t*, i32, i32)
+
+declare void @__kmpc_serialized_parallel(%struct.ident_t*, i32)
+
+declare void @__kmpc_end_serialized_parallel(%struct.ident_t*, i32)
+
+declare i32 @__kmpc_master(%struct.ident_t*, i32)
+
+declare i32 @__kmpc_end_master(%struct.ident_t*, i32)
+
+declare void @__kmpc_critical(%struct.ident_t*, i32, [8 x i32])
+
+declare void @__kmpc_critical_with_hint(%struct.ident_t*, i32, [8 x i32], i32)
+
+declare void @__kmpc_end_critical(%struct.ident_t*, i32, [8 x i32])
+
 ; CHECK: ; Function Attrs: nounwind
 ; CHECK-NEXT: declare dso_local void @omp_set_num_threads(i32)
 
@@ -685,67 +720,118 @@
 ; CHECK: ; Function Attrs: nounwind
 ; CHECK-NEXT: declare dso_local i32 @omp_get_supported_active_levels() #0
 
-; OPTIMISTIC: ; Function Attrs: nofree nosync nounwind writeonly
+; CHECK: Function Attrs: inaccessiblemem_or_argmemonly
+; CHECK-NEXT: declare void @__kmpc_barrier(%struct.ident_t*, i32)
+
+; CHECK: Function Attrs: inaccessiblemem_or_argmemonly
+; CHECK-NEXT: declare i32 @__kmpc_cancel(%struct.ident_t*, i32, i32)
+
+; CHECK: Function Attrs: inaccessiblemem_or_argmemonly
+; CHECK-NEXT: declare i32 @__kmpc_cancel_barrier(%struct.ident_t*, i32)
+
+; CHECK: Function Attrs: inaccessiblemem_or_argmemonly
+; CHECK-NEXT: declare void @__kmpc_flush(%struct.ident_t*)
+
+; CHECK: Function Attrs: nounwind
+; CHECK-NEXT: declare i32 @__kmpc_global_thread_num(%struct.ident_t*)
+
+; CHECK: Function Attrs: nounwind
+; CHECK-NEXT: dec

[clang] 7d382dc - [Syntax] Build declarator nodes

[Dmitri Gribenko via cfe-commits]

Author: Marcel Hlopko
Date: 2020-03-16T19:13:59+01:00
New Revision: 7d382dcd46a18c23a01e3754807f577598a9bc84

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

LOG: [Syntax] Build declarator nodes

Summary:
Copy of https://reviews.llvm.org/D72089 with Ilya's permission. See
https://reviews.llvm.org/D72089 for the first batch of comments.

Reviewers: gribozavr2

Reviewed By: gribozavr2

Subscribers: cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang/include/clang/Tooling/Syntax/Nodes.h
clang/lib/Tooling/Syntax/BuildTree.cpp
clang/lib/Tooling/Syntax/Nodes.cpp
clang/unittests/Tooling/Syntax/TreeTest.cpp

Removed: 




diff  --git a/clang/include/clang/Tooling/Syntax/Nodes.h 
b/clang/include/clang/Tooling/Syntax/Nodes.h
index 25acc1757428..82fcac33f99b 100644
--- a/clang/include/clang/Tooling/Syntax/Nodes.h
+++ b/clang/include/clang/Tooling/Syntax/Nodes.h
@@ -38,10 +38,10 @@ enum class NodeKind : uint16_t {
   Leaf,
   TranslationUnit,
 
-  // Expressions
+  // Expressions.
   UnknownExpression,
 
-  // Statements
+  // Statements.
   UnknownStatement,
   DeclarationStatement,
   EmptyStatement,
@@ -58,7 +58,7 @@ enum class NodeKind : uint16_t {
   ExpressionStatement,
   CompoundStatement,
 
-  // Declarations
+  // Declarations.
   UnknownDeclaration,
   EmptyDeclaration,
   StaticAssertDeclaration,
@@ -68,7 +68,16 @@ enum class NodeKind : uint16_t {
   NamespaceAliasDefinition,
   UsingNamespaceDirective,
   UsingDeclaration,
-  TypeAliasDeclaration
+  TypeAliasDeclaration,
+
+  // Declarators.
+  SimpleDeclarator,
+  ParenDeclarator,
+
+  ArraySubscript,
+  TrailingReturnType,
+  ParametersAndQualifiers,
+  MemberPointer
 };
 /// For debugging purposes.
 llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, NodeKind K);
@@ -101,11 +110,19 @@ enum class NodeRole : uint8_t {
   ExpressionStatement_expression,
   CompoundStatement_statement,
   StaticAssertDeclaration_condition,
-  StaticAssertDeclaration_message
+  StaticAssertDeclaration_message,
+  SimpleDeclaration_declarator,
+  ArraySubscript_sizeExpression,
+  TrailingReturnType_arrow,
+  TrailingReturnType_declarator,
+  ParametersAndQualifiers_parameter,
+  ParametersAndQualifiers_trailingReturn
 };
 /// For debugging purposes.
 llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, NodeRole R);
 
+class SimpleDeclarator;
+
 /// A root node for a translation unit. Parent is always null.
 class TranslationUnit final : public Tree {
 public:
@@ -375,6 +392,8 @@ class SimpleDeclaration final : public Declaration {
   static bool classof(const Node *N) {
 return N->kind() == NodeKind::SimpleDeclaration;
   }
+  /// FIXME: use custom iterator instead of 'vector'.
+  std::vector declarators();
 };
 
 /// namespace  {  }
@@ -424,6 +443,113 @@ class TypeAliasDeclaration final : public Declaration {
   }
 };
 
+/// Covers a name, an initializer and a part of the type outside declaration
+/// specifiers. Examples are:
+/// `*a` in `int *a`
+/// `a[10]` in `int a[10]`
+/// `*a = nullptr` in `int *a = nullptr`
+/// Declarators can be unnamed too:
+/// `**` in `new int**`
+/// `* = nullptr` in `void foo(int* = nullptr)`
+/// Most declarators you encounter are instances of SimpleDeclarator. They may
+/// contain an inner declarator inside parentheses, we represent it as
+/// ParenDeclarator. E.g.
+/// `(*a)` in `int (*a) = 10`
+class Declarator : public Tree {
+public:
+  Declarator(NodeKind K) : Tree(K) {}
+  static bool classof(const Node *N) {
+return NodeKind::SimpleDeclarator <= N->kind() &&
+   N->kind() <= NodeKind::ParenDeclarator;
+  }
+};
+
+/// A top-level declarator without parentheses. See comment of Declarator for
+/// more details.
+class SimpleDeclarator final : public Declarator {
+public:
+  SimpleDeclarator() : Declarator(NodeKind::SimpleDeclarator) {}
+  static bool classof(const Node *N) {
+return N->kind() == NodeKind::SimpleDeclarator;
+  }
+};
+
+/// Declarator inside parentheses.
+/// E.g. `(***a)` from `int (***a) = nullptr;`
+/// See comment of Declarator for more details.
+class ParenDeclarator final : public Declarator {
+public:
+  ParenDeclarator() : Declarator(NodeKind::ParenDeclarator) {}
+  static bool classof(const Node *N) {
+return N->kind() == NodeKind::ParenDeclarator;
+  }
+  syntax::Leaf *lparen();
+  syntax::Leaf *rparen();
+};
+
+/// Array size specified inside a declarator.
+/// E.g:
+///   `[10]` in `int a[10];`
+///   `[static 10]` in `void f(int xs[static 10]);`
+class ArraySubscript final : public Tree {
+public:
+  ArraySubscript() : Tree(NodeKind::ArraySubscript) {}
+  static bool classof(const Node *N) {
+return N->kind() == NodeKind::ArraySubscript;
+  }
+  // TODO: a

[PATCH] D76220: [Syntax] Build declarator nodes

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr2 accepted this revision.
gribozavr2 added a comment.

BTW, would be also nice to have tests for trailing return types not at the top 
level -- in a follow-up:

`auto x(char a, auto (*b)(int) -> short) -> void;`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76220



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

[PATCH] D72089: [Syntax] Build declarator nodes

[Dmitri Gribenko via Phabricator via cfe-commits]

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

Superseded by https://reviews.llvm.org/D76220.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72089



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

[PATCH] D76086: [Sema][SVE] Reject arithmetic on pointers to sizeless types

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

I don't think there's really any reason to force the user into this sort of 
thing... but again, we can relax this later, so I'm not that concerned about it 
at the moment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76086



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

[PATCH] D76220: [Syntax] Build declarator nodes

[Dmitri Gribenko via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG7d382dcd46a1: [Syntax] Build declarator nodes (authored by 
hlopko, committed by gribozavr).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76220

Files:
  clang/include/clang/Tooling/Syntax/Nodes.h
  clang/lib/Tooling/Syntax/BuildTree.cpp
  clang/lib/Tooling/Syntax/Nodes.cpp
  clang/unittests/Tooling/Syntax/TreeTest.cpp

Index: clang/unittests/Tooling/Syntax/TreeTest.cpp
===
--- clang/unittests/Tooling/Syntax/TreeTest.cpp
+++ clang/unittests/Tooling/Syntax/TreeTest.cpp
@@ -174,17 +174,21 @@
 *: TranslationUnit
 |-SimpleDeclaration
 | |-int
-| |-main
-| |-(
-| |-)
+| |-SimpleDeclarator
+| | |-main
+| | `-ParametersAndQualifiers
+| |   |-(
+| |   `-)
 | `-CompoundStatement
 |   |-{
 |   `-}
 `-SimpleDeclaration
   |-void
-  |-foo
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-foo
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 `-}
@@ -201,9 +205,11 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-int
-  |-main
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-main
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 |-IfStatement
@@ -246,9 +252,11 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-void
-  |-test
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-test
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 |-ForStatement
@@ -268,18 +276,21 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-void
-  |-test
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-test
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 |-DeclarationStatement
 | |-SimpleDeclaration
 | | |-int
-| | |-a
-| | |-=
-| | `-UnknownExpression
-| |   `-10
+| | `-SimpleDeclarator
+| |   |-a
+| |   |-=
+| |   `-UnknownExpression
+| | `-10
 | `-;
 `-}
 )txt"},
@@ -287,9 +298,11 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-void
-  |-test
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-test
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 |-EmptyStatement
@@ -309,9 +322,11 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-void
-  |-test
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-test
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 |-SwitchStatement
@@ -345,9 +360,11 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-void
-  |-test
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-test
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 |-WhileStatement
@@ -375,9 +392,11 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-int
-  |-test
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-test
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 |-ReturnStatement
@@ -398,26 +417,31 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-void
-  |-test
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-test
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 |-DeclarationStatement
 | |-SimpleDeclaration
 | | |-int
-| | |-a
-| | |-[
-| | |-UnknownExpression
-| | | `-3
-| | `-]
+| | `-SimpleDeclarator
+| |   |-a
+| |   `-ArraySubscript
+| | |-[
+| | |-UnknownExpression
+| | | `-3
+| | `-]
 | `-;
 |-RangeBasedForStatement
 | |-for
 | |-(
 | |-SimpleDeclaration
 | | |-int
-| | |-x
+| | |-SimpleDeclarator
+| | | `-x
 | | `-:
 | |-UnknownExpression
 | | `-a
@@ -433,9 +457,11 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-void
-  |-main
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-main
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 |-UnknownStatement
@@ -460,9 +486,11 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-void
-  |-test
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-test
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 |-ExpressionStatement
@@ -500,10 +528,12 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-int
-  |-*
-  |-a
+  |-SimpleDeclarator
+  | |-*
+  | `-a
   |-,
-  |-b
+  |-SimpleDeclarator
+  | `-b
   `-;
   )txt"},
   {R"cpp(
@@ -514,10 +544,12 @@
 `-SimpleDeclaration
   |-typedef
   |-int
-  |-*
-  |-a
+  |-SimpleDeclarator
+  | |-*
+  | `-a
   |-,
-  |-b
+  |-SimpleDeclarator
+  | `-b
   `-;
   )txt"},
   // Multiple declarators inside a statement.
@@ -531,27 +563,33 @@
 *: TranslationUnit
 `-SimpleDeclaration
   |-void
-  |-foo
-  |-(
-  |-)
+  |-SimpleDeclarator
+  | |-foo
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   `-)
   `-CompoundStatement
 |-{
 |-DeclarationStatement
 | |-SimpleDeclaration
 | | |-int
-| | |-*
-| | |

[PATCH] D76084: [Sema][SVE] Reject subscripts on pointers to sizeless types

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

We already support sizeof expressions where the result can't be computed at 
compile-time.  For example: `int f(int n, int (*p)[n]) { return sizeof(*p); }`. 
 (For constant contexts specifically, see HandleSizeof() in ExprConstant.cpp.)  
I think any argument that it would be hard to extend that to cover sizeless 
types is pretty weak.

Of course, just because we can support it doesn't mean we should.  As a matter 
of language design, we might want to discourage users from writing that sort of 
thing.  It could be argued either way.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76084



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

[PATCH] D76084: [Sema][SVE] Reject subscripts on pointers to sizeless types

[Eli Friedman via Phabricator via cfe-commits]

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

This patch LGTM, at least for now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76084



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

[PATCH] D75579: [WIP] Replace MCTargetOptionsCommandFlags.inc and CommandFlags.inc by libraries

[serge via Phabricator via cfe-commits]

serge-sans-paille added a comment.

@MaskRay gentle up :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75579



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

[PATCH] D75811: [CUDA] Choose default architecture based on CUDA installation

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

In D75811#1923281 , @tambre wrote:

> Your help here and over on CMake's side has been very helpful. Thank you!
>  I'll @ you on CMake's side if I need any help while working on CUDA support. 
> Hopefully you won't mind. :)


No problem. I'll be happy to help.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75811



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

[PATCH] D76219: [Sema][SVE] Reject "delete" with sizeless types

[Eli Friedman via Phabricator via cfe-commits]

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76219



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

[PATCH] D73967: Implement _ExtInt as an extended int type specifier.

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

Ping!


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

https://reviews.llvm.org/D73967



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

[PATCH] D76078: [AArch64][SVE] Add a pass for SVE intrinsic optimisations

[Andrzej Warzynski via Phabricator via cfe-commits]

andwar added a comment.

Cheers for working on this @kmclaughlin ! Have you considered adding an 
interface for the new PM? You could check this for reference: 
https://reviews.llvm.org/rGd6de5f12d485a85504bc99d384a85634574a27e2 (also 
implements a FunctionPass).




Comment at: llvm/lib/Target/AArch64/SVEIntrinsicOpts.cpp:30
+
+#define DEBUG_TYPE "sve-intrinsicopts"
+

In AArch64TargetMachine.cpp it's:
```
static cl::opt EnableSVEIntrinsicOpts(
"aarch64-sve-intrinsic-opts", cl::Hidden,
cl::desc("Enable SVE intrinsic opts"),
cl::init(true));
```
so it probably make sense to use `see-intrinsic-opts` here as well?



Comment at: llvm/lib/Target/AArch64/SVEIntrinsicOpts.cpp:110
+  auto *PN = dyn_cast(X->getOperand(0));
+  if (!PN)
+return false;

Given where this method is called, this is guaranteed to be always non-null. 
Perhaps `assert` instead?



Comment at: llvm/lib/Target/AArch64/SVEIntrinsicOpts.cpp:120
+if (!Reinterpret ||
+RequiredType != Reinterpret->getArgOperand(0)->getType())
+  return false;

Isn't it guaranteed that `RequiredType == 
Reinterpret->getArgOperand(0)->getType()` is always true? I.e., `PN` and the 
incoming values have identical type.



Comment at: llvm/lib/Target/AArch64/SVEIntrinsicOpts.cpp:125
+  // Create the new Phi
+  LLVMContext &C1 = PN->getContext();
+  IRBuilder<> Builder(C1);

[nit] Perhaps `Ctx` instead of `C1`?



Comment at: llvm/lib/Target/AArch64/SVEIntrinsicOpts.cpp:131
+
+  for (unsigned i = 0; i < PN->getNumIncomingValues(); i++) {
+auto *Reinterpret = cast(PN->getIncomingValue(i));

`i` -> `I`



Comment at: llvm/lib/Target/AArch64/SVEIntrinsicOpts.cpp:187
+  // elide away both reinterprets if there are no other users of Y.
+  if (auto *Y = isReinterpretToSVBool(I->getArgOperand(0))) {
+Value *SourceVal = Y->getArgOperand(0);

I'd be tempted to rewrite this to use early exit 
(https://llvm.org/docs/CodingStandards.html#use-early-exits-and-continue-to-simplify-code):

```
auto *Y = isReinterpretToSVBool(I->getArgOperand(0));
if (nullptr == Y)
  return false;

// The rest of the function
```



Comment at: llvm/lib/Target/AArch64/SVEIntrinsicOpts.cpp:224
+  bool Changed = false;
+  for (auto II = BB->begin(), IE = BB->end(); II != IE;) {
+Instruction *I = &(*II);

1. Could this be a for-range loop instead?

2. This loop seems to be a perfect candidate for `make_early_inc_range` 
(https://github.com/llvm/llvm-project/blob/172f1460ae05ab5c33c757142c8bdb10acfbdbe1/llvm/include/llvm/ADT/STLExtras.h#L499),
 e.g.
```
 for (Instruction &I : make_early_inc_range(BB))
  Changed |= optimizeIntrinsic(&I);
```



Comment at: llvm/lib/Target/AArch64/SVEIntrinsicOpts.cpp:240
+  ReversePostOrderTraversal RPOT(Root);
+  for (auto I = RPOT.begin(), E = RPOT.end(); I != E; ++I) {
+Changed |= optimizeBlock(*I);

AFAIK, this iterates over BBs so `I` should be replaced with `BB`. 

Also, perhaps `for (auto *BB : RPOT) {` instead? 



Comment at: llvm/test/CodeGen/AArch64/sve-intrinsic-opts-ptest.ll:18
+; OPT-LABEL: ptest_any2
+; OPT: %out = call i1 @llvm.aarch64.sve.ptest.any.nxv16i1( 
%1,  %2)
+  %mask = tail call  @llvm.aarch64.sve.ptrue.nxv2i1(i32 31)

What's `%1` and `%2`? Is it worth adding the calls that generated them in the 
expected output?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76078



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

[PATCH] D73720: [Analyzer] Use note tags to track container begin and and changes

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

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



Comment at: clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp:713
+  StringRef Name;
+  if (const auto *DRE = dyn_cast(ContE->IgnoreParenCasts())) {
+Name = DRE->getDecl()->getName();

NoQ wrote:
> Hmm, i think you should instead look at `ContReg`, i.e. whether it's a 
> non-anonymous `VarRegion` or a `FieldRegion` or something like that (in other 
> patches as well). It would work more often and it'll transparently handle 
> references.
Unfortunately it is a `SymRegion` so it does not work :-( (Even using 
`getMostDerivedRegion()` does not help.)


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

https://reviews.llvm.org/D73720



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

[PATCH] D73720: [Analyzer] Use note tags to track container begin and and changes

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

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



Comment at: clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp:713
+  StringRef Name;
+  if (const auto *DRE = dyn_cast(ContE->IgnoreParenCasts())) {
+Name = DRE->getDecl()->getName();

baloghadamsoftware wrote:
> NoQ wrote:
> > Hmm, i think you should instead look at `ContReg`, i.e. whether it's a 
> > non-anonymous `VarRegion` or a `FieldRegion` or something like that (in 
> > other patches as well). It would work more often and it'll transparently 
> > handle references.
> Unfortunately it is a `SymRegion` so it does not work :-( (Even using 
> `getMostDerivedRegion()` does not help.)
You mean the first checking form `SymbolicRegion`, then get its symbol, check 
for `SymbolRegionValue`, then get its `TypedValueRegion`, check for 
`DeclRegion` and use its `Decl`? This sound waaay more complicated and less 
readable. I am not sure which are the side cases: is it always 
`SymbolicRegion`? Is the `Symbol` of `SymbolicRegion` always a 
`SymbolRegionValue`? Is ithe `TypedValueRegion` (the return value of its 
`getRegion()`) always a `DeclRegion`?


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

https://reviews.llvm.org/D73720



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

[PATCH] D75514: [Analyzer] Only add container note tags to the operations of the affected container

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

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



Comment at: clang/lib/StaticAnalyzer/Checkers/DebugContainerModeling.cpp:32
 
   template 
   void analyzerContainerDataField(const CallExpr *CE, CheckerContext &C,

Szelethus wrote:
> NoQ wrote:
> > `llvm::function_ref`?
> Is that a thing? Nice.
You mean a callback instead of a template?


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

https://reviews.llvm.org/D75514



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

[PATCH] D76229: [clang-tidy] Added PlacementNewStorageCheck

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Have you considered making this a static analyzer check as opposed to a 
clang-tidy check? I think using dataflow analysis will really reduce the false 
positives because it will be able to track the allocation and alignment data 
across control flow.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76229



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

[PATCH] D75687: [clangd] Only minimally escape text when rendering to markdown.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Sorry for the slow turnaround, bit off more than I could chew on a few fronts 
:-(




Comment at: clang-tools-extra/clangd/FormattedString.cpp:69
+return false;
+  if (Contents.front() == '!' || Contents.front() == '?' ||
+  Contents.front() == '/')

kadircet wrote:
> what is the `?` for ?
` nit: After.ltrim('#')
I'm trying to avoid repeating the known values of C as there's various 
fallthrough/copied cases and it seems more fragile.



Comment at: clang-tools-extra/clangd/FormattedString.cpp:164
+  case '<': // HTML tag (or autolink, which we choose not to escape)
+return looksLikeTag(After);
+  case '>': // Quote marker. Needs escaping at start of line.

kadircet wrote:
> is this really worth all the trouble ?
Simplified it a bit. I think it's worth not escaping `<` when not matched with 
`>`. Sadly the rule can't be quite that simple because multiple chunks can form 
a tag.



Comment at: clang-tools-extra/clangd/FormattedString.cpp:166
+  case '>': // Quote marker. Needs escaping at start of line.
+return StartsLine && Before.empty();
+  case '&': { // HTML entity reference

kadircet wrote:
> I believe it is also allowed to have whitespaces(less than 4?) before the `>` 
> to be interpreted as a quote marker.
Yes, but a chunk never begins with whitespace (see assert at top of function)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75687



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

[PATCH] D75687: [clangd] Only minimally escape text when rendering to markdown.

[Sam McCall via Phabricator via cfe-commits]

sammccall updated this revision to Diff 250624.
sammccall marked 10 inline comments as done.
sammccall added a comment.

Address review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75687

Files:
  clang-tools-extra/clangd/FormattedString.cpp
  clang-tools-extra/clangd/FormattedString.h
  clang-tools-extra/clangd/unittests/FormattedStringTests.cpp
  clang-tools-extra/clangd/unittests/HoverTests.cpp

Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -1905,7 +1905,7 @@
   llvm::StringRef ExpectedMarkdown = R"md(### variable `foo`  
 
 ---
-Value \= `val`  
+Value = `val`  
 
 ---
 ```cpp
Index: clang-tools-extra/clangd/unittests/FormattedStringTests.cpp
===
--- clang-tools-extra/clangd/unittests/FormattedStringTests.cpp
+++ clang-tools-extra/clangd/unittests/FormattedStringTests.cpp
@@ -17,25 +17,94 @@
 namespace markup {
 namespace {
 
-TEST(Render, Escaping) {
-  // Check some ASCII punctuation
-  Paragraph P;
-  P.appendText("*!`");
-  EXPECT_EQ(P.asMarkdown(), "\\*\\!\\`");
+std::string escape(llvm::StringRef Text) {
+  return Paragraph().appendText(Text.str()).asMarkdown();
+}
+
+MATCHER_P(escaped, C, "") {
+  return testing::ExplainMatchResult(::testing::HasSubstr(std::string{'\\', C}),
+ arg, result_listener);
+}
 
+MATCHER(escapedNone, "") {
+  return testing::ExplainMatchResult(::testing::Not(::testing::HasSubstr("\\")),
+ arg, result_listener);
+}
+
+TEST(Render, Escaping) {
   // Check all ASCII punctuation.
-  P = Paragraph();
   std::string Punctuation = R"txt(!"#$%&'()*+,-./:;<=>?@[\]^_`{|}~)txt";
-  // Same text, with each character escaped.
-  std::string EscapedPunctuation;
-  EscapedPunctuation.reserve(2 * Punctuation.size());
-  for (char C : Punctuation)
-EscapedPunctuation += std::string("\\") + C;
-  P.appendText(Punctuation);
-  EXPECT_EQ(P.asMarkdown(), EscapedPunctuation);
+  std::string EscapedPunc = R"txt(!"#$%&'()\*+,-./:;<=>?@[\\]^\_\`{|}~)txt";
+  EXPECT_EQ(escape(Punctuation), EscapedPunc);
+
+  // Inline code
+  EXPECT_EQ(escape("`foo`"), R"(\`foo\`)");
+  EXPECT_EQ(escape("`foo"), R"(\`foo)");
+  EXPECT_EQ(escape("foo`"), R"(foo\`)");
+  EXPECT_EQ(escape("``foo``"), R"(\`\`foo\`\`)");
+  // Code blocks
+  EXPECT_EQ(escape("```"), R"(\`\`\`)"); // This could also be inline code!
+  EXPECT_EQ(escape("~~~"), R"(\~~~)");
+
+  // Rulers and headings
+  EXPECT_THAT(escape("## Heading"), escaped('#'));
+  EXPECT_THAT(escape("Foo # bar"), escapedNone());
+  EXPECT_EQ(escape("---"), R"(\---)");
+  EXPECT_EQ(escape("-"), R"(\-)");
+  EXPECT_EQ(escape("==="), R"(\===)");
+  EXPECT_EQ(escape("="), R"(\=)");
+  EXPECT_EQ(escape("***"), R"(\*\*\*)"); // \** could start emphasis!
+
+  // HTML tags.
+  EXPECT_THAT(escape(""), escaped('<'));
+  EXPECT_THAT(escape("std::vector"), escaped('<'));
+  EXPECT_THAT(escape("std::map"), escapedNone());
+  // Autolinks
+  EXPECT_THAT(escape("Email "), escapedNone());
+  EXPECT_THAT(escape("Website "), escapedNone());
+
+  // Bullet lists.
+  EXPECT_THAT(escape("- foo"), escaped('-'));
+  EXPECT_THAT(escape("* foo"), escaped('*'));
+  EXPECT_THAT(escape("+ foo"), escaped('+'));
+  EXPECT_THAT(escape("+"), escaped('+'));
+  EXPECT_THAT(escape("a + foo"), escapedNone());
+  EXPECT_THAT(escape("a+ foo"), escapedNone());
+  EXPECT_THAT(escape("1. foo"), escaped('.'));
+  EXPECT_THAT(escape("a. foo"), escapedNone());
+
+  // Emphasis.
+  EXPECT_EQ(escape("*foo*"), R"(\*foo\*)");
+  EXPECT_EQ(escape("**foo**"), R"(\*\*foo\*\*)");
+  EXPECT_THAT(escape("*foo"), escaped('*'));
+  EXPECT_THAT(escape("foo *"), escapedNone());
+  EXPECT_THAT(escape("foo * bar"), escapedNone());
+  EXPECT_THAT(escape("foo_bar"), escaped('_'));
+  EXPECT_THAT(escape("foo _ bar"), escapedNone());
+
+  // HTML entities.
+  EXPECT_THAT(escape("fish &chips;"), escaped('&'));
+  EXPECT_THAT(escape("fish & chips;"), escapedNone());
+  EXPECT_THAT(escape("fish &chips"), escapedNone());
+  EXPECT_THAT(escape("foo * bar"), escaped('&'));
+  EXPECT_THAT(escape("foo ¯ bar"), escaped('&'));
+  EXPECT_THAT(escape("foo &?; bar"), escapedNone());
+
+  // Links.
+  EXPECT_THAT(escape("[foo](bar)"), escaped(']'));
+  EXPECT_THAT(escape("[foo]: bar"), escaped(']'));
+  // No need to escape these, as the target never exists.
+  EXPECT_THAT(escape("[foo][]"), escapedNone());
+  EXPECT_THAT(escape("[foo][bar]"), escapedNone());
+  EXPECT_THAT(escape("[foo]"), escapedNone());
 
   // In code blocks we don't need to escape ASCII punctuation.
-  P = Paragraph();
+  Paragraph P = Paragraph();
   P.appendCode("* foo !+ bar * baz");
   EXPECT_EQ(P.asMarkdown(), "`* foo !+ bar * b

[PATCH] D75579: [WIP] Replace MCTargetOptionsCommandFlags.inc and CommandFlags.inc by libraries

[Fangrui Song via Phabricator via cfe-commits]

MaskRay accepted this revision.
MaskRay added a comment.

The title still says this is a WIP..


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75579



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

[PATCH] D75951: Keep a list of already-included pragma-once files for mods.

[Vy Nguyen via Phabricator via cfe-commits]

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



Comment at: clang/lib/Lex/HeaderSearch.cpp:1266
+  if (PP.isIncludeVisibleInLocalModule(File, M)) return false;
+  else  PP.setIncludeVisibleForHeader(File, M);
+} else {

jyknight wrote:
> I wonder if this should be just using the CurSubmoduleState. Actually, is "M" 
> even needed in this function at all -- why isn't everything just using 
> CurSubmoduleState? (It's very likely I'm just confused about what the 
> semantics of this are...).
> "Is M needed ...  ?"

Yes - I think so because if we're looking at Module header, then I *think* the 
header's content will/should be visible to the submods in it.

On the other hand (ie., the else branch), if we're looking a "regular" header, 
then it should not be visible to the submods  


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75951



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

[PATCH] D76229: [clang-tidy] Added PlacementNewStorageCheck

[Roman Lebedev via Phabricator via cfe-commits]

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

This seems to be already handled by clang static analyzer? 
(`clang-analyzer-cplusplus.PlacementNew`)

  :19:5: warning: Storage provided to placement new is only 2 bytes, 
whereas the allocated type requires 8 bytes 
[clang-analyzer-cplusplus.PlacementNew]
  ::new (&s1) long;
  ^
  :19:5: note: Storage provided to placement new is only 2 bytes, 
whereas the allocated type requires 8 bytes
  :64:3: warning: Storage provided to placement new is only 2 bytes, 
whereas the allocated type requires 8 bytes 
[clang-analyzer-cplusplus.PlacementNew]
::new (buffer3) long;
^

https://godbolt.org/z/9VX5WW


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76229



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

[PATCH] D76229: [clang-tidy] Added PlacementNewStorageCheck

[Karasev Nikita via Phabricator via cfe-commits]

f00kat added a comment.

In D76229#1925268 , @aaron.ballman 
wrote:

> Have you considered making this a static analyzer check as opposed to a 
> clang-tidy check? I think using dataflow analysis will really reduce the 
> false positives because it will be able to track the allocation and alignment 
> data across control flow.


I have never try to write static analyzer before. Okay, I will look at examples 
of how to work with dataflow.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76229



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

[PATCH] D76062: [PATCH] [ARM] ARMv8.6-a command-line + BFloat16 Asm Support

[Ties Stuij via Phabricator via cfe-commits]

stuij updated this revision to Diff 250630.
stuij edited the summary of this revision.
stuij added a comment.

Updating D76062 : [PATCH] [ARM] ARMv8.6-a 
command-line + BFloat16 Asm Support

follow changes in patch: [TableGen] Support combining AssemblerPredicates with 
ORs


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76062

Files:
  clang/lib/Basic/Targets/AArch64.cpp
  clang/lib/Basic/Targets/AArch64.h
  clang/lib/Basic/Targets/ARM.cpp
  clang/test/Driver/aarch64-cpus.c
  clang/test/Driver/arm-cortex-cpus.c
  clang/test/Preprocessor/arm-target-features.c
  llvm/include/llvm/ADT/Triple.h
  llvm/include/llvm/Support/AArch64TargetParser.def
  llvm/include/llvm/Support/AArch64TargetParser.h
  llvm/include/llvm/Support/ARMTargetParser.def
  llvm/include/llvm/Support/ARMTargetParser.h
  llvm/lib/Support/AArch64TargetParser.cpp
  llvm/lib/Support/ARMTargetParser.cpp
  llvm/lib/Support/Triple.cpp
  llvm/lib/Target/AArch64/AArch64.td
  llvm/lib/Target/AArch64/AArch64InstrFormats.td
  llvm/lib/Target/AArch64/AArch64InstrInfo.td
  llvm/lib/Target/AArch64/AArch64SVEInstrInfo.td
  llvm/lib/Target/AArch64/AArch64Subtarget.h
  llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp
  llvm/lib/Target/AArch64/SVEInstrFormats.td
  llvm/lib/Target/ARM/ARM.td
  llvm/lib/Target/ARM/ARMInstrNEON.td
  llvm/lib/Target/ARM/ARMInstrVFP.td
  llvm/lib/Target/ARM/ARMPredicates.td
  llvm/lib/Target/ARM/ARMSubtarget.h
  llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
  llvm/lib/Target/ARM/MCTargetDesc/ARMELFStreamer.cpp
  llvm/test/MC/AArch64/SVE/bfcvt-diagnostics.s
  llvm/test/MC/AArch64/SVE/bfcvt.s
  llvm/test/MC/AArch64/SVE/bfcvtnt-diagnostics.s
  llvm/test/MC/AArch64/SVE/bfcvtnt.s
  llvm/test/MC/AArch64/SVE/bfdot-diagnostics.s
  llvm/test/MC/AArch64/SVE/bfdot.s
  llvm/test/MC/AArch64/SVE/bfmlal-diagnostics.s
  llvm/test/MC/AArch64/SVE/bfmlal.s
  llvm/test/MC/AArch64/SVE/bfmmla-diagnostics.s
  llvm/test/MC/AArch64/SVE/bfmmla.s
  llvm/test/MC/AArch64/armv8.6a-bf16.s
  llvm/test/MC/ARM/bfloat16-a32-errors.s
  llvm/test/MC/ARM/bfloat16-a32-errors2.s
  llvm/test/MC/ARM/bfloat16-a32.s
  llvm/test/MC/ARM/bfloat16-t32-errors.s
  llvm/test/MC/ARM/bfloat16-t32.s
  llvm/test/MC/Disassembler/AArch64/armv8.6a-bf16.txt
  llvm/test/MC/Disassembler/ARM/bfloat16-a32_1.txt
  llvm/test/MC/Disassembler/ARM/bfloat16-a32_2.txt
  llvm/test/MC/Disassembler/ARM/bfloat16-t32.txt
  llvm/test/MC/Disassembler/ARM/bfloat16-t32_errors.txt
  llvm/unittests/Support/TargetParserTest.cpp

Index: llvm/unittests/Support/TargetParserTest.cpp
===
--- llvm/unittests/Support/TargetParserTest.cpp
+++ llvm/unittests/Support/TargetParserTest.cpp
@@ -26,9 +26,9 @@
 "armv7e-m","armv7em",  "armv8-a", "armv8","armv8a",
 "armv8l",  "armv8.1-a","armv8.1a","armv8.2-a","armv8.2a",
 "armv8.3-a",   "armv8.3a", "armv8.4-a",   "armv8.4a", "armv8.5-a",
-"armv8.5a", "armv8-r", "armv8r",  "armv8-m.base", "armv8m.base",
-"armv8-m.main", "armv8m.main", "iwmmxt",  "iwmmxt2",  "xscale",
-"armv8.1-m.main",
+"armv8.5a", "armv8.6-a",   "armv8.6a", "armv8-r", "armv8r",
+"armv8-m.base", "armv8m.base", "armv8-m.main", "armv8m.main", "iwmmxt",
+"iwmmxt2",  "xscale",  "armv8.1-m.main",
 };
 
 bool testARMCPU(StringRef CPUName, StringRef ExpectedArch,
@@ -411,6 +411,9 @@
   testARMArch("armv8.5-a", "generic", "v8.5a",
   ARMBuildAttrs::CPUArch::v8_A));
   EXPECT_TRUE(
+  testARMArch("armv8.6-a", "generic", "v8.6a",
+  ARMBuildAttrs::CPUArch::v8_A));
+  EXPECT_TRUE(
   testARMArch("armv8-r", "cortex-r52", "v8r",
   ARMBuildAttrs::CPUArch::v8_R));
   EXPECT_TRUE(
@@ -678,7 +681,7 @@
   "v7",   "v7a","v7ve",  "v7hl",   "v7l",   "v7-r",   "v7r",   "v7-m",
   "v7m",  "v7k","v7s",   "v7e-m",  "v7em",  "v8-a",   "v8","v8a",
   "v8l",  "v8.1-a", "v8.1a", "v8.2-a", "v8.2a", "v8.3-a", "v8.3a", "v8.4-a",
-  "v8.4a", "v8.5-a","v8.5a", "v8-r",   "v8m.base", "v8m.main", "v8.1m.main"
+  "v8.4a", "v8.5-a","v8.5a", "v8.6-a", "v8.6a", "v8-r",   "v8m.base", "v8m.main", "v8.1m.main"
   };
 
   for (unsigned i = 0; i < array_lengthof(Arch); i++) {
@@ -743,6 +746,7 @@
 case ARM::ArchKind::ARMV8_3A:
 case ARM::ArchKind::ARMV8_4A:
 case ARM::ArchKind::ARMV8_5A:
+case ARM::ArchKind::ARMV8_6A:
   EXPECT_EQ(ARM::ProfileKind::A, ARM::parseArchProfile(ARMArch[i]));
   break;
 default:
@@ -1008,6 +1012,8 @@
   ARMBuildAttrs::CPUArch::v8_A));
   EXPECT_TRUE(testAArch64Arch("armv8.5-a", "generic", "v8.5a",
   ARMBuildAttrs::CPUArch::v8_A));
+  EXPECT_TRUE(testAArch64Arch("armv8.6-a", "generic", "v8.6a",
+  ARMBu

[PATCH] D76229: [clang-tidy] Added PlacementNewStorageCheck

[Karasev Nikita via Phabricator via cfe-commits]

f00kat added a comment.

In D76229#1925360 , @lebedev.ri wrote:

> This seems to be already handled by clang static analyzer? 
> (`clang-analyzer-cplusplus.PlacementNew`)
>
>   :19:5: warning: Storage provided to placement new is only 2 bytes, 
> whereas the allocated type requires 8 bytes 
> [clang-analyzer-cplusplus.PlacementNew]
>   ::new (&s1) long;
>   ^
>   :19:5: note: Storage provided to placement new is only 2 bytes, 
> whereas the allocated type requires 8 bytes
>   :64:3: warning: Storage provided to placement new is only 2 bytes, 
> whereas the allocated type requires 8 bytes 
> [clang-analyzer-cplusplus.PlacementNew]
> ::new (buffer3) long;
> ^
>
>
> https://godbolt.org/z/9VX5WW


But it seems like not all of my tests pass on static analyzer?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76229



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

[PATCH] D75579: Replace MCTargetOptionsCommandFlags.inc and CommandFlags.inc by runtime-registration

[serge via Phabricator via cfe-commits]

serge-sans-paille added a comment.

@MaskRay no longer a WIP ;-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75579



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

[PATCH] D75579: Replace MCTargetOptionsCommandFlags.inc and CommandFlags.inc by runtime-registration

[serge via Phabricator via cfe-commits]

serge-sans-paille updated this revision to Diff 250631.
serge-sans-paille added a comment.

Renamed registration constructor to use a more llvm-ish name.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75579

Files:
  clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp
  lld/Common/TargetOptionsCommandFlags.cpp
  llvm/include/llvm/CodeGen/CommandFlags.h
  llvm/include/llvm/CodeGen/CommandFlags.inc
  llvm/include/llvm/MC/MCTargetOptionsCommandFlags.h
  llvm/include/llvm/MC/MCTargetOptionsCommandFlags.inc
  llvm/include/llvm/module.modulemap
  llvm/lib/CodeGen/CMakeLists.txt
  llvm/lib/CodeGen/CommandFlags.cpp
  llvm/lib/MC/CMakeLists.txt
  llvm/lib/MC/MCTargetOptionsCommandFlags.cpp
  llvm/tools/dsymutil/DwarfStreamer.cpp
  llvm/tools/gold/gold-plugin.cpp
  llvm/tools/llc/CMakeLists.txt
  llvm/tools/llc/llc.cpp
  llvm/tools/lli/CMakeLists.txt
  llvm/tools/lli/lli.cpp
  llvm/tools/llvm-dwp/llvm-dwp.cpp
  llvm/tools/llvm-isel-fuzzer/llvm-isel-fuzzer.cpp
  llvm/tools/llvm-lto/CMakeLists.txt
  llvm/tools/llvm-lto/llvm-lto.cpp
  llvm/tools/llvm-lto2/CMakeLists.txt
  llvm/tools/llvm-lto2/llvm-lto2.cpp
  llvm/tools/llvm-mc-assemble-fuzzer/CMakeLists.txt
  llvm/tools/llvm-mc-assemble-fuzzer/llvm-mc-assemble-fuzzer.cpp
  llvm/tools/llvm-mc/CMakeLists.txt
  llvm/tools/llvm-mc/llvm-mc.cpp
  llvm/tools/llvm-mca/llvm-mca.cpp
  llvm/tools/llvm-ml/llvm-ml.cpp
  llvm/tools/llvm-opt-fuzzer/llvm-opt-fuzzer.cpp
  llvm/tools/lto/CMakeLists.txt
  llvm/tools/lto/lto.cpp
  llvm/tools/opt/opt.cpp
  llvm/unittests/DebugInfo/DWARF/DwarfGenerator.cpp

Index: llvm/unittests/DebugInfo/DWARF/DwarfGenerator.cpp
===
--- llvm/unittests/DebugInfo/DWARF/DwarfGenerator.cpp
+++ llvm/unittests/DebugInfo/DWARF/DwarfGenerator.cpp
@@ -25,7 +25,7 @@
 #include "llvm/MC/MCRegisterInfo.h"
 #include "llvm/MC/MCStreamer.h"
 #include "llvm/MC/MCSubtargetInfo.h"
-#include "llvm/MC/MCTargetOptionsCommandFlags.inc"
+#include "llvm/MC/MCTargetOptionsCommandFlags.h"
 #include "llvm/PassAnalysisSupport.h"
 #include "llvm/Support/LEB128.h"
 #include "llvm/Support/TargetRegistry.h"
@@ -37,6 +37,8 @@
 using namespace llvm;
 using namespace dwarf;
 
+mc::RegisterMCTargetOptionsFlags MOF;
+
 namespace {} // end anonymous namespace
 
 //===--===//
@@ -433,7 +435,7 @@
TripleName,
inconvertibleErrorCode());
 
-  MCTargetOptions MCOptions = InitMCTargetOptionsFromFlags();
+  MCTargetOptions MCOptions = mc::InitMCTargetOptionsFromFlags();
   MAI.reset(TheTarget->createMCAsmInfo(*MRI, TripleName, MCOptions));
   if (!MAI)
 return make_error("no asm info for target " + TripleName,
Index: llvm/tools/opt/opt.cpp
===
--- llvm/tools/opt/opt.cpp
+++ llvm/tools/opt/opt.cpp
@@ -22,7 +22,7 @@
 #include "llvm/Analysis/TargetLibraryInfo.h"
 #include "llvm/Analysis/TargetTransformInfo.h"
 #include "llvm/Bitcode/BitcodeWriterPass.h"
-#include "llvm/CodeGen/CommandFlags.inc"
+#include "llvm/CodeGen/CommandFlags.h"
 #include "llvm/CodeGen/TargetPassConfig.h"
 #include "llvm/Config/llvm-config.h"
 #include "llvm/IR/DataLayout.h"
@@ -62,6 +62,8 @@
 using namespace llvm;
 using namespace opt_tool;
 
+static codegen::RegisterCodeGenFlags CFG;
+
 // The OptimizationList is automatically populated with registered Passes by the
 // PassNameParser.
 //
@@ -471,16 +473,17 @@
StringRef FeaturesStr,
const TargetOptions &Options) {
   std::string Error;
-  const Target *TheTarget = TargetRegistry::lookupTarget(MArch, TheTriple,
- Error);
+  const Target *TheTarget =
+  TargetRegistry::lookupTarget(codegen::getMArch(), TheTriple, Error);
   // Some modules don't specify a triple, and this is okay.
   if (!TheTarget) {
 return nullptr;
   }
 
-  return TheTarget->createTargetMachine(TheTriple.getTriple(), CPUStr,
-FeaturesStr, Options, getRelocModel(),
-getCodeModel(), GetCodeGenOptLevel());
+  return TheTarget->createTargetMachine(
+  TheTriple.getTriple(), codegen::getCPUStr(), codegen::getFeaturesStr(),
+  Options, codegen::getExplicitRelocModel(),
+  codegen::getExplicitCodeModel(), GetCodeGenOptLevel());
 }
 
 #ifdef BUILD_EXAMPLES
@@ -667,11 +670,11 @@
   Triple ModuleTriple(M->getTargetTriple());
   std::string CPUStr, FeaturesStr;
   TargetMachine *Machine = nullptr;
-  const TargetOptions Options = InitTargetOptionsFromCodeGenFlags();
+  const TargetOptions Options = codegen::InitTargetOptionsFromCodeGenFlags();
 
   if (ModuleTriple.getArch()) {
-CPUStr = getCPUStr();
-FeaturesStr = getFeaturesS

[PATCH] D76229: [clang-tidy] Added PlacementNewStorageCheck

[Karasev Nikita via Phabricator via cfe-commits]

f00kat added a comment.

In D76229#1925360 , @lebedev.ri wrote:

> This seems to be already handled by clang static analyzer? 
> (`clang-analyzer-cplusplus.PlacementNew`)
>
>   :19:5: warning: Storage provided to placement new is only 2 bytes, 
> whereas the allocated type requires 8 bytes 
> [clang-analyzer-cplusplus.PlacementNew]
>   ::new (&s1) long;
>   ^
>   :19:5: note: Storage provided to placement new is only 2 bytes, 
> whereas the allocated type requires 8 bytes
>   :64:3: warning: Storage provided to placement new is only 2 bytes, 
> whereas the allocated type requires 8 bytes 
> [clang-analyzer-cplusplus.PlacementNew]
> ::new (buffer3) long;
> ^
>
>
> https://godbolt.org/z/9VX5WW


Oh no..It is sad :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76229



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

[PATCH] D75951: Keep a list of already-included pragma-once files for mods.

[Vy Nguyen via Phabricator via cfe-commits]

oontvoo marked an inline comment as done and an inline comment as not done.
oontvoo added inline comments.



Comment at: clang/lib/Lex/HeaderSearch.cpp:1266
+  if (PP.isIncludeVisibleInLocalModule(File, M)) return false;
+  else  PP.setIncludeVisibleForHeader(File, M);
+} else {

oontvoo wrote:
> jyknight wrote:
> > I wonder if this should be just using the CurSubmoduleState. Actually, is 
> > "M" even needed in this function at all -- why isn't everything just using 
> > CurSubmoduleState? (It's very likely I'm just confused about what the 
> > semantics of this are...).
> > "Is M needed ...  ?"
> 
> Yes - I think so because if we're looking at Module header, then I *think* 
> the header's content will/should be visible to the submods in it.
> 
> On the other hand (ie., the else branch), if we're looking a "regular" 
> header, then it should not be visible to the submods  
[not done - re-opening for question]


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75951



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

[PATCH] D76030: [CUDA] Warn about unsupported CUDA SDK version only if it's used.

[Artem Belevich via Phabricator via cfe-commits]

tra added a subscriber: hans.
tra added a comment.

@hans  -- this should be cherry-picked into 10 if it's not too late yet.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76030



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

[PATCH] D76096: [clang] allow const structs to be constant expressions in initializer lists

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

rsmith added inline comments.



Comment at: clang/lib/CodeGen/CGExprConstant.cpp:1013
+  if (V->hasInit())
+return Visit(V->getInit(), V->getType());
+return nullptr;

nickdesaulniers wrote:
> efriedma wrote:
> > You need to be more careful here; we can call ConstExprEmitter for 
> > arbitrary expressions.
> "Be more careful" how?
Here are some specific cases in which you need to be more careful because the 
code as-is would do the wrong thing:

 * when emitting a global's initializer in C++, where the value of the object 
denoted by the DeclRefExpr could have changed between its initialization and 
the expression we're currently emitting 
 * when emitting anything other than a global initializer in C, where the value 
of a global could have changed after its emission
 * when emitting a reference to a non-global declaration in C (local variables 
might change between initialization and use)

We would need to restrict this to the cases where the variable's value cannot 
possibly have changed between initialization and use.

In C, that's (mostly) the case for a static storage variable referenced from 
the initializer of a static storage variable, for a thread storage variable 
referenced from the initializer of a static storage variable, or for a thread 
storage variable referenced from the initializer of a thread storage variable. 
Even then, this isn't strictly correct in the presence of DSOs, but I think it 
should be correct if the two variables are defined in the same translation unit.

In C++, that's (mostly) the case when the variable is `const` or `constexpr` 
and has no mutable subobjects. (There's still the case where the reference 
happens outside the lifetime of the object -- for the most part we can handwave 
that away by saying it must be UB, but that's not true in general in the period 
of construction and period of destruction.)

In both cases, the optimization is (arguably) still wrong if there are any 
volatile subobjects.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76096



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

[clang] 2a3723e - [memtag] Plug in stack safety analysis.

[Evgenii Stepanov via cfe-commits]

Author: Evgenii Stepanov
Date: 2020-03-16T16:35:25-07:00
New Revision: 2a3723ef114d467179d463539dd73974b87ccf85

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

LOG: [memtag] Plug in stack safety analysis.

Summary:
Run StackSafetyAnalysis at the end of the IR pipeline and annotate
proven safe allocas with !stack-safe metadata. Do not instrument such
allocas in the AArch64StackTagging pass.

Reviewers: pcc, vitalybuka, ostannard

Reviewed By: vitalybuka

Subscribers: merge_guards_bot, kristof.beyls, hiraditya, cfe-commits, gilang, 
llvm-commits

Tags: #clang, #llvm

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

Added: 
clang/test/Driver/memtag.c
llvm/test/Analysis/StackSafetyAnalysis/ipa-attr.ll

Modified: 
clang/lib/CodeGen/BackendUtil.cpp
llvm/include/llvm/Analysis/StackSafetyAnalysis.h
llvm/lib/Analysis/StackSafetyAnalysis.cpp
llvm/lib/Passes/PassRegistry.def
llvm/lib/Target/AArch64/AArch64StackTagging.cpp
llvm/test/CodeGen/AArch64/stack-tagging.ll

Removed: 




diff  --git a/clang/lib/CodeGen/BackendUtil.cpp 
b/clang/lib/CodeGen/BackendUtil.cpp
index b6ca46e7e835..28e4ecc7b4bf 100644
--- a/clang/lib/CodeGen/BackendUtil.cpp
+++ b/clang/lib/CodeGen/BackendUtil.cpp
@@ -18,6 +18,7 @@
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/ADT/Triple.h"
+#include "llvm/Analysis/StackSafetyAnalysis.h"
 #include "llvm/Analysis/TargetLibraryInfo.h"
 #include "llvm/Analysis/TargetTransformInfo.h"
 #include "llvm/Bitcode/BitcodeReader.h"
@@ -345,6 +346,11 @@ static void addDataFlowSanitizerPass(const 
PassManagerBuilder &Builder,
   PM.add(createDataFlowSanitizerPass(LangOpts.SanitizerBlacklistFiles));
 }
 
+static void addMemTagOptimizationPasses(const PassManagerBuilder &Builder,
+legacy::PassManagerBase &PM) {
+  PM.add(createStackSafetyGlobalInfoWrapperPass(/*SetMetadata=*/true));
+}
+
 static TargetLibraryInfoImpl *createTLII(llvm::Triple &TargetTriple,
  const CodeGenOptions &CodeGenOpts) {
   TargetLibraryInfoImpl *TLII = new TargetLibraryInfoImpl(TargetTriple);
@@ -696,6 +702,11 @@ void EmitAssemblyHelper::CreatePasses(legacy::PassManager 
&MPM,
addDataFlowSanitizerPass);
   }
 
+  if (LangOpts.Sanitize.has(SanitizerKind::MemTag)) {
+PMBuilder.addExtension(PassManagerBuilder::EP_OptimizerLast,
+   addMemTagOptimizationPasses);
+  }
+
   // Set up the per-function pass manager.
   FPM.add(new TargetLibraryInfoWrapperPass(*TLII));
   if (CodeGenOpts.VerifyModule)
@@ -1300,6 +1311,11 @@ void EmitAssemblyHelper::EmitAssemblyWithNewPassManager(
   /*CompileKernel=*/true, /*Recover=*/true));
 }
 
+if (CodeGenOpts.OptimizationLevel > 0 &&
+LangOpts.Sanitize.has(SanitizerKind::MemTag)) {
+  MPM.addPass(StackSafetyGlobalAnnotatorPass());
+}
+
 if (CodeGenOpts.OptimizationLevel == 0) {
   addCoroutinePassesAtO0(MPM, LangOpts, CodeGenOpts);
   addSanitizersAtO0(MPM, TargetTriple, LangOpts, CodeGenOpts);

diff  --git a/clang/test/Driver/memtag.c b/clang/test/Driver/memtag.c
new file mode 100644
index ..9c548910048e
--- /dev/null
+++ b/clang/test/Driver/memtag.c
@@ -0,0 +1,23 @@
+// REQUIRES: aarch64-registered-target
+
+// Old pass manager.
+// RUN: %clang -fno-experimental-new-pass-manager -target 
aarch64-unknown-linux -march=armv8+memtag -fsanitize=memtag %s -S -emit-llvm -o 
- | FileCheck %s --check-prefix=CHECK-NO-SAFETY
+// RUN: %clang -O1 -fno-experimental-new-pass-manager -target 
aarch64-unknown-linux -march=armv8+memtag -fsanitize=memtag %s -S -emit-llvm -o 
- | FileCheck %s --check-prefix=CHECK-SAFETY
+// RUN: %clang -O2 -fno-experimental-new-pass-manager -target 
aarch64-unknown-linux -march=armv8+memtag -fsanitize=memtag %s -S -emit-llvm -o 
- | FileCheck %s --check-prefix=CHECK-SAFETY
+// RUN: %clang -O3 -fno-experimental-new-pass-manager -target 
aarch64-unknown-linux -march=armv8+memtag -fsanitize=memtag %s -S -emit-llvm -o 
- | FileCheck %s --check-prefix=CHECK-SAFETY
+
+// New pass manager.
+// RUN: %clang -fexperimental-new-pass-manager -target 
aarch64-unknown-linux -march=armv8+memtag -fsanitize=memtag %s -S -emit-llvm -o 
- | FileCheck %s --check-prefix=CHECK-NO-SAFETY
+// RUN: %clang -O1 -fexperimental-new-pass-manager -target 
aarch64-unknown-linux -march=armv8+memtag -fsanitize=memtag %s -S -emit-llvm -o 
- | FileCheck %s --check-prefix=CHECK-SAFETY
+// RUN: %clang -O2 -fexperimental-new-pass-manager -target 
aarch64-unknown-linux -march=armv8+memtag -fsanitize=memtag %s -S -emit-llvm -o 
- | FileCheck %s --check-prefix=CHECK-SAFETY
+// RUN: %clang -O3 -fexperimental-new-pass-manager -target 
aarch64