[PATCH] D75665: [analyzer] On-demand parsing capability for CTU

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

balazske added inline comments.



Comment at: clang/include/clang/CrossTU/CrossTranslationUnit.h:222
+
+  struct ASTLoader {
+/// Load the ASTUnit by an identifier. Subclasses should determine what 
this

`class` would look better here? (The descendants are `class` too.)



Comment at: clang/include/clang/CrossTU/CrossTranslationUnit.h:225
+/// would be.
+virtual LoadResultTy load(StringRef Identifier) = 0;
+virtual ~ASTLoader() = default;

Probably it is good to mention in the documentation that the function is used 
with a string read from a file and the type of the file determines the meaning 
of the `Identifier` here (the calling code does have no direct knowledge about 
it).



Comment at: clang/lib/CrossTU/CrossTranslationUnit.cpp:460
+  /// Ideally there is exactly one entry in the compilation database that
+  /// matchse the source file.
+  if (ASTs.size() != 1)

matches



Comment at: clang/lib/CrossTU/CrossTranslationUnit.cpp:604
+   index_error_code::failed_to_get_external_ast);
+}
+

Members of `ASTOnDemandLoader` would be better in a single group in the source 
file.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75665



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

[PATCH] D75723: [X86] Make intrinsics _BitScan* not limited to Windows

[Kan Shengchen via Phabricator via cfe-commits]

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



Comment at: clang/include/clang/Basic/BuiltinsX86.def:1904
+// BITSCAN
+TARGET_BUILTIN(_BitScanForward, "UcUNi*UNi", "n", "")
+TARGET_BUILTIN(_BitScanReverse, "UcUNi*UNi", "n", "")

craig.topper wrote:
> The N specifier here is sort of MSVC mode specific. I need to think about 
> this.
> 
> This also makes this available without including a header file which isn't 
> good if it doesn't start with __builtin.
I think we can define a new builtin `__builtin_ia32_BitScanForward` in 
BuiltinsX86.def. And when macro `_MSC_VER` is not defined, define 
`_BitScanForward` in ia32intrin.h by calling the 
`__builtin_ia32_BitScanForward`. Then we can avoid the N specifier and the name 
issue. Do you think it's appropriate?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75723



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

[PATCH] D74935: [LangRef][AliasAnalysis] Clarify `noalias` affects only modified objects

[Jeroen Dobbelaere via Phabricator via cfe-commits]

jeroen.dobbelaere added a comment.

In D74935#1908665 , @jdoerfert wrote:

> I think we conflate two things here:
>
> 1. The modifications to the LangRef which should be in accordance with the C 
> standard (at least I haven't seen you contradict the new wording directly).


imho, the proposed wording is still confusing, and does not handle the case 
with the extra indirections.
Unless the 'by any means' was meant to also include the  '.. Every  access  
that modifies X shall be considered also to modify P,for the purposes of this 
subclause. .. ' from the restrict specification.
If that is the idea, we should mention it explicitly.

> 2. The extended `noalias` deduction D73428 .
> 
>   If you look at the commit message in D73428 
> , it says that `noalias` is not just derived 
> for all `readonly` arguments. In your example we access `unknown` memory, 
> e.g., `**pA=42`, so case (1) cannot be applied. For case (2) we need to show 
> that the loads of `pA` and `pB` do not alias, which we cannot.

That sounds good. Is there also a testcase (similar to D74935#1907100 
 or D74935#1907939 
 ) that explicitly checks that 
'noalias' is not deduced ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74935



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

[PATCH] D73846: make sure to not warn about unused macros from -D

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

Ping


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

https://reviews.llvm.org/D73846



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

[PATCH] D69585: PerformPendingInstatiations() already in the PCH

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

Ping...


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

https://reviews.llvm.org/D69585



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

[PATCH] D74846: fix -fcodegen-modules code when used with PCH (PR44953)

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

Ping. I don't particularly care about the declspec(selectany) corner case, but 
at least the mistake from  D69778  should be 
fixed (and it's a simple fix), so that it can be committed again.


Repository:
  rC Clang

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

https://reviews.llvm.org/D74846



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

[PATCH] D75726: [ConstExprPreter] Updated constant interpreter documentation

[Nandor Licker via Phabricator via cfe-commits]

nand created this revision.
nand added reviewers: rsmith, Bigcheese, dexonsmith, jfb.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Updated the documentation to better reflect features implemented on the
constexpr branch at https://github.com/nandor/llvm-project and extended
the TODO list with known missing features


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D75726

Files:
  clang/docs/ConstantInterpreter.rst

Index: clang/docs/ConstantInterpreter.rst
===
--- clang/docs/ConstantInterpreter.rst
+++ clang/docs/ConstantInterpreter.rst
@@ -44,21 +44,28 @@
 
 * ``PT_Ptr``
 
-  Pointer type, defined in ``"Pointer.h"``.
+  Pointer type, defined in ``"Pointer.h"``. A pointer can be either null, reference interpreter-allocated memory (``BlockPointer``) or point to an address which can be derived, but not accessed (``ExternPointer``).
 
 * ``PT_FnPtr``
 
-  Function pointer type, can also be a null function pointer. Defined in ``"Pointer.h"``.
+  Function pointer type, can also be a null function pointer. Defined in ``"FnPointer.h"``.
 
 * ``PT_MemPtr``
 
-  Member pointer type, can also be a null member pointer. Defined in ``"Pointer.h"``
+  Member pointer type, can also be a null member pointer. Defined in ``"MemberPointer.h"``
+
+* ``PT_VoidPtr``
+
+  Void pointer type, can be used for rount-trip casts. Represented as the union of all pointers which can be cast to void. Defined in ``"VoidPointer.h"``.
+
+* ``PT_ObjCBlockPtr``
+
+  Pointer type for ObjC blocks. Defined in ``"ObjCBlockPointer.h"``.
 
 Composite types
 ---
 
-The interpreter distinguishes two kinds of composite types: arrays and records. Unions are represented as records, except a single field can be marked as active. The contents of inactive fields are kept until they
-are reactivated and overwritten.
+The interpreter distinguishes two kinds of composite types: arrays and records (structs and classes). Unions are represented as records, except at most a single field can be marked as active. The contents of inactive fields are kept until they are reactivated and overwritten. Complex numbers (``_Complex``) and vectors (``__attribute((vector_size(16)))``) are treated as arrays.
 
 
 Bytecode Execution
@@ -85,8 +92,6 @@
 
 * ``IsStatic`` indicates whether the block has static duration in the interpreter, i.e. it is not a local in a frame.
 
-* ``IsExtern`` indicates that the block was created for an extern and the storage cannot be read or written.
-
 * ``DeclID`` identifies each global declaration (it is set to an invalid and irrelevant value for locals) in order to prevent illegal writes and reads involving globals and temporaries with static storage duration.
 
 Static blocks are never deallocated, but local ones might be deallocated even when there are live pointers to them. Pointers are only valid as long as the blocks they point to are valid, so a block with pointers to it whose lifetime ends is kept alive until all pointers to it go out of scope. Since the frame is destroyed on function exit, such blocks are turned into a ``DeadBlock`` and copied to storage managed by the interpreter itself, not the frame. Reads and writes to these blocks are illegal and cause an appropriate diagnostic to be emitted. When the last pointer goes out of scope, dead blocks are also deallocated.
@@ -97,7 +102,7 @@
 * **DtorFn**: invokes the destructors of non-trivial objects.
 * **MoveFn**: moves a block to dead storage.
 
-Non-static blocks track all the pointers into them through an intrusive doubly-linked list, this is required in order to adjust all pointers when transforming a block into a dead block.
+Non-static blocks track all the pointers into them through an intrusive doubly-linked list, required to adjust all pointers when transforming a block into a dead block.
 
 Descriptors
 ---
@@ -110,13 +115,13 @@
 
 * **Arrays of primitives**
 
-  An array of primitives contains a pointer to an ``InitMap`` storage as its first field: the initialisation map is a bit map indicating all elements of the array which were initialised. If the pointer is null, no elements were initialised, while a value of ``(InitMap)-1`` indicates that the object was fully initialised. when all fields are initialised, the map is deallocated and replaced with that token.
+  An array of primitives contains a pointer to an ``InitMap`` storage as its first field: the initialisation map is a bit map indicating all elements of the array which were initialised. If the pointer is null, no elements were initialised, while a value of ``(InitMap*)-1`` indicates that the object was fully initialised. when all fields are initialised, the map is deallocated and replaced with that token.
 
   Array elements are stored sequentially, without padding, after the pointer to the map.
 
 * **Arrays of composites and records**
 
-  Each element in an array of composites is preceded

[PATCH] D74735: [analyzer] Add support for CXXInheritedCtorInitExpr.

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

steakhal added a comment.
Herald added a subscriber: danielkiss.

This patch introduced a crash while I was analyzing the libpressio 
.
I was using the `CodeChecker` to drive the analysis with the `--enable-all` 
flag.

The exact command was the following:

  /home/username/git/llvm-project/build/debug/bin/clang-11 --analyze 
-Qunused-arguments -Xclang -analyzer-opt-analyze-headers -Xclang 
-analyzer-output=plist-multi-file -o 
/home/username/git/libpressio/build/results/pressio_options.cc_clangsa_0316a939d2e5f7ba700a67a7cc467d92.plist
 -Xclang -analyzer-config -Xclang expand-macros=true -Xclang 
-analyzer-checker=apiModeling.StdCLibraryFunctions -Xclang 
-analyzer-checker=apiModeling.TrustNonnull -Xclang 
-analyzer-checker=apiModeling.google.GTest -Xclang 
-analyzer-checker=apiModeling.llvm.CastValue -Xclang 
-analyzer-checker=apiModeling.llvm.ReturnValue -Xclang 
-analyzer-checker=core.CallAndMessage -Xclang -analyzer-checker=core.DivideZero 
-Xclang -analyzer-checker=core.DynamicTypePropagation -Xclang 
-analyzer-checker=core.NonNullParamChecker -Xclang 
-analyzer-checker=core.NonnilStringConstants -Xclang 
-analyzer-checker=core.NullDereference -Xclang 
-analyzer-checker=core.StackAddrEscapeBase -Xclang 
-analyzer-checker=core.StackAddressEscape -Xclang 
-analyzer-checker=core.UndefinedBinaryOperatorResult -Xclang 
-analyzer-checker=core.VLASize -Xclang 
-analyzer-checker=core.builtin.BuiltinFunctions -Xclang 
-analyzer-checker=core.builtin.NoReturnFunctions -Xclang 
-analyzer-checker=core.uninitialized.ArraySubscript -Xclang 
-analyzer-checker=core.uninitialized.Assign -Xclang 
-analyzer-checker=core.uninitialized.Branch -Xclang 
-analyzer-checker=core.uninitialized.CapturedBlockVariable -Xclang 
-analyzer-checker=core.uninitialized.UndefReturn -Xclang 
-analyzer-checker=cplusplus.InnerPointer -Xclang 
-analyzer-checker=cplusplus.Move -Xclang -analyzer-checker=cplusplus.NewDelete 
-Xclang -analyzer-checker=cplusplus.NewDeleteLeaks -Xclang 
-analyzer-checker=cplusplus.PlacementNew -Xclang 
-analyzer-checker=cplusplus.PureVirtualCall -Xclang 
-analyzer-checker=cplusplus.SelfAssignment -Xclang 
-analyzer-checker=cplusplus.SmartPtr -Xclang 
-analyzer-checker=cplusplus.VirtualCallModeling -Xclang 
-analyzer-checker=deadcode.DeadStores -Xclang 
-analyzer-checker=fuchsia.HandleChecker -Xclang 
-analyzer-checker=nullability.NullPassedToNonnull -Xclang 
-analyzer-checker=nullability.NullReturnedFromNonnull -Xclang 
-analyzer-checker=nullability.NullabilityBase -Xclang 
-analyzer-checker=nullability.NullableDereferenced -Xclang 
-analyzer-checker=nullability.NullablePassedToNonnull -Xclang 
-analyzer-checker=nullability.NullableReturnedFromNonnull -Xclang 
-analyzer-checker=optin.cplusplus.UninitializedObject -Xclang 
-analyzer-checker=optin.cplusplus.VirtualCall -Xclang 
-analyzer-checker=optin.mpi.MPI-Checker -Xclang 
-analyzer-checker=optin.osx.OSObjectCStyleCast -Xclang 
-analyzer-checker=optin.osx.cocoa.localizability.EmptyLocalizationContextChecker
 -Xclang 
-analyzer-checker=optin.osx.cocoa.localizability.NonLocalizedStringChecker 
-Xclang -analyzer-checker=optin.performance.GCDAntipattern -Xclang 
-analyzer-checker=optin.performance.Padding -Xclang 
-analyzer-checker=optin.portability.UnixAPI -Xclang 
-analyzer-checker=security.FloatLoopCounter -Xclang 
-analyzer-checker=security.insecureAPI.DeprecatedOrUnsafeBufferHandling -Xclang 
-analyzer-checker=security.insecureAPI.SecuritySyntaxChecker -Xclang 
-analyzer-checker=security.insecureAPI.UncheckedReturn -Xclang 
-analyzer-checker=security.insecureAPI.bcmp -Xclang 
-analyzer-checker=security.insecureAPI.bcopy -Xclang 
-analyzer-checker=security.insecureAPI.bzero -Xclang 
-analyzer-checker=security.insecureAPI.decodeValueOfObjCType -Xclang 
-analyzer-checker=security.insecureAPI.getpw -Xclang 
-analyzer-checker=security.insecureAPI.gets -Xclang 
-analyzer-checker=security.insecureAPI.mkstemp -Xclang 
-analyzer-checker=security.insecureAPI.mktemp -Xclang 
-analyzer-checker=security.insecureAPI.rand -Xclang 
-analyzer-checker=security.insecureAPI.strcpy -Xclang 
-analyzer-checker=security.insecureAPI.vfork -Xclang -analyzer-checker=unix.API 
-Xclang -analyzer-checker=unix.DynamicMemoryModeling -Xclang 
-analyzer-checker=unix.Malloc -Xclang -analyzer-checker=unix.MallocSizeof 
-Xclang -analyzer-checker=unix.MismatchedDeallocator -Xclang 
-analyzer-checker=unix.Vfork -Xclang -analyzer-checker=unix.cstring.BadSizeArg 
-Xclang -analyzer-checker=unix.cstring.CStringModeling -Xclang 
-analyzer-checker=unix.cstring.NullArg -Xclang 
-analyzer-checker=valist.CopyToSelf -Xclang 
-analyzer-checker=valist.Uninitialized -Xclang 
-analyzer-checker=valist.Unterminated -Xclang 
-analyzer-checker=valist.ValistBase -Xclang -analyzer-config -Xclang 
aggressive-binary-operation-simplification=true -Xclang -analyzer-config 
-Xclang crosscheck-with-z3=true -x c++ --target=x86_64-linux-gnu -std=gnu++14 
-Dlibpressio_EXPORTS -I/home/usern

[PATCH] D75323: Support relative dest paths in headermap files

[Takuto Ikuta via Phabricator via cfe-commits]

takuto.ikuta added a comment.

Nico, can we merge this?


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

https://reviews.llvm.org/D75323



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

[clang] af473d0 - [Analyzer][StreamChecker] Adding PreCall and refactoring (NFC).

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

Author: Balázs Kéri
Date: 2020-03-06T10:17:58+01:00
New Revision: af473d0e84f1f7bcaca6017012e22beddd260b67

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

LOG: [Analyzer][StreamChecker] Adding PreCall and refactoring (NFC).

Summary:
Adding PreCall callback.
Argument validity checks are moved into the PreCall callback.
Code is restructured, functions renamed.
There are "pre" and "eval" functions for the file operations.
And additional state check (validate) functions.

Reviewers: Szelethus

Reviewed By: Szelethus

Subscribers: xazax.hun, baloghadamsoftware, szepet, a.sidorin, mikhail.ramalho, 
Szelethus, donat.nagy, dkrupp, gamesh411, Charusso, martong, cfe-commits

Tags: #clang

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

Added: 


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

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index 64412442a528..9d8d276bd701 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -50,98 +50,140 @@ struct StreamState {
 };
 
 class StreamChecker;
+struct FnDescription;
+using FnCheck = std::function;
 
-using FnCheck = std::function;
+using ArgNoTy = unsigned int;
+static const ArgNoTy ArgNone = std::numeric_limits::max();
 
 struct FnDescription {
+  FnCheck PreFn;
   FnCheck EvalFn;
+  ArgNoTy StreamArgNo;
 };
 
-class StreamChecker : public Checker {
+/// Get the value of the stream argument out of the passed call event.
+/// The call should contain a function that is described by Desc.
+SVal getStreamArg(const FnDescription *Desc, const CallEvent &Call) {
+  assert(Desc && Desc->StreamArgNo != ArgNone &&
+ "Try to get a non-existing stream argument.");
+  return Call.getArgSVal(Desc->StreamArgNo);
+}
+
+class StreamChecker
+: public Checker {
   mutable std::unique_ptr BT_nullfp, BT_illegalwhence,
   BT_doubleclose, BT_ResourceLeak;
 
 public:
+  void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
   bool evalCall(const CallEvent &Call, CheckerContext &C) const;
   void checkDeadSymbols(SymbolReaper &SymReaper, CheckerContext &C) const;
 
 private:
-
   CallDescriptionMap FnDescriptions = {
-  {{"fopen"}, {&StreamChecker::evalFopen}},
-  {{"freopen", 3}, {&StreamChecker::evalFreopen}},
-  {{"tmpfile"}, {&StreamChecker::evalFopen}},
-  {{"fclose", 1}, {&StreamChecker::evalFclose}},
-  {{"fread", 4},
-   {std::bind(&StreamChecker::checkArgNullStream, _1, _2, _3, 3)}},
-  {{"fwrite", 4},
-   {std::bind(&StreamChecker::checkArgNullStream, _1, _2, _3, 3)}},
-  {{"fseek", 3}, {&StreamChecker::evalFseek}},
-  {{"ftell", 1},
-   {std::bind(&StreamChecker::checkArgNullStream, _1, _2, _3, 0)}},
-  {{"rewind", 1},
-   {std::bind(&StreamChecker::checkArgNullStream, _1, _2, _3, 0)}},
-  {{"fgetpos", 2},
-   {std::bind(&StreamChecker::checkArgNullStream, _1, _2, _3, 0)}},
-  {{"fsetpos", 2},
-   {std::bind(&StreamChecker::checkArgNullStream, _1, _2, _3, 0)}},
-  {{"clearerr", 1},
-   {std::bind(&StreamChecker::checkArgNullStream, _1, _2, _3, 0)}},
-  {{"feof", 1},
-   {std::bind(&StreamChecker::checkArgNullStream, _1, _2, _3, 0)}},
-  {{"ferror", 1},
-   {std::bind(&StreamChecker::checkArgNullStream, _1, _2, _3, 0)}},
-  {{"fileno", 1},
-   {std::bind(&StreamChecker::checkArgNullStream, _1, _2, _3, 0)}},
+  {{"fopen"}, {nullptr, &StreamChecker::evalFopen, ArgNone}},
+  {{"freopen", 3},
+   {&StreamChecker::preFreopen, &StreamChecker::evalFreopen, 2}},
+  {{"tmpfile"}, {nullptr, &StreamChecker::evalFopen, ArgNone}},
+  {{"fclose", 1},
+   {&StreamChecker::preFclose, &StreamChecker::evalFclose, 0}},
+  {{"fread", 4}, {&StreamChecker::preDefault, nullptr, 3}},
+  {{"fwrite", 4}, {&StreamChecker::preDefault, nullptr, 3}},
+  {{"fseek", 3}, {&StreamChecker::preFseek, nullptr, 0}},
+  {{"ftell", 1}, {&StreamChecker::preDefault, nullptr, 0}},
+  {{"rewind", 1}, {&StreamChecker::preDefault, nullptr, 0}},
+  {{"fgetpos", 2}, {&StreamChecker::preDefault, nullptr, 0}},
+  {{"fsetpos", 2}, {&StreamChecker::preDefault, nullptr, 0}},
+  {{"clearerr", 1}, {&StreamChecker::preDefault, nullptr, 0}},
+  {{"feof", 1}, {&StreamChecker::preDefault, nullptr, 0}},
+  {{"ferror", 1}, {&StreamChecker::preDefault, nullptr, 0}},
+  {{"fileno", 1}, {&StreamChecker::preDefault, nullptr, 0}},
   };
 
-  void evalFopen(const CallEvent &Call, CheckerContext &C) const;
-  void evalFreopen(const CallEvent &Call, CheckerContext &C) const;
-  void evalFclose(const CallEvent &Call, CheckerContext &C) const;
-  v

[PATCH] D75726: [ConstExprPreter] Updated constant interpreter documentation

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

This file needs linewrapping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75726



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

[PATCH] D75479: [clangd] go-to-def on names in comments etc that are used nearby.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In D75479#1908774 , @nridge wrote:

> This method only returns `Bar::uniqueMethodName()` because it's closer in 
> terms of distance


Yeah, missing data is definitely bad:

- not finding results makes the feature feel flaky or unreliable
- finding *partial* results can be misleading and makes the feature 
untrustworthy

> Is that perhaps a reason to adjust the order in which we try the approaches 
> (i.e. use this one as a fallback if index lookup fails)?

Well, it's even easier to construct examples where the index approach is 
missing some data (most symbols are not indexable) or gives wrong results (name 
collisions are common) which is probably worse.
So I'm not convinced by this that the index is more accurate.

> Or should we try to allow multiple results with this approach as well?

It's an interesting idea, but two immediate risks:

- it's clearly worse for things like comment navigation local variables
- it helps your example only quite narrowly: all the targets need to be in the 
current file

We have several building blocks and various ways for how to put them together.
I think we should go back to the use cases (e.g. dependent code) and work out 
how each is best served, my intuition is that pretty different. Discussing how 
the building blocks should relate to each other in the general case may run in 
circles a bit :-) I'll make a proposal on 
https://github.com/clangd/clangd/issues/241 for a bit more visibility.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75479



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

[PATCH] D74618: [ARM] Creating 'call_mangled' for Neon intrinsics definitions

[Lucas Prates via Phabricator via cfe-commits]

pratlucas updated this revision to Diff 248659.
pratlucas added a comment.

Applying clang-format.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74618

Files:
  clang/include/clang/Basic/arm_neon_incl.td
  clang/utils/TableGen/NeonEmitter.cpp

Index: clang/utils/TableGen/NeonEmitter.cpp
===
--- clang/utils/TableGen/NeonEmitter.cpp
+++ clang/utils/TableGen/NeonEmitter.cpp
@@ -27,8 +27,9 @@
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/None.h"
-#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Casting.h"
@@ -518,7 +519,8 @@
 std::pair emitDagDupTyped(DagInit *DI);
 std::pair emitDagShuffle(DagInit *DI);
 std::pair emitDagCast(DagInit *DI, bool IsBitCast);
-std::pair emitDagCall(DagInit *DI);
+std::pair emitDagCall(DagInit *DI,
+ bool MatchMangledName);
 std::pair emitDagNameReplace(DagInit *DI);
 std::pair emitDagLiteral(DagInit *DI);
 std::pair emitDagOp(DagInit *DI);
@@ -546,7 +548,8 @@
 public:
   /// Called by Intrinsic - this attempts to get an intrinsic that takes
   /// the given types as arguments.
-  Intrinsic &getIntrinsic(StringRef Name, ArrayRef Types);
+  Intrinsic &getIntrinsic(StringRef Name, ArrayRef Types,
+  Optional MangledName);
 
   /// Called by Intrinsic - returns a globally-unique number.
   unsigned getUniqueNumber() { return UniqueNumber++; }
@@ -1383,8 +1386,8 @@
 return emitDagSaveTemp(DI);
   if (Op == "op")
 return emitDagOp(DI);
-  if (Op == "call")
-return emitDagCall(DI);
+  if (Op == "call" || Op == "call_mangled")
+return emitDagCall(DI, Op == "call_mangled");
   if (Op == "name_replace")
 return emitDagNameReplace(DI);
   if (Op == "literal")
@@ -1411,7 +1414,8 @@
   }
 }
 
-std::pair Intrinsic::DagEmitter::emitDagCall(DagInit *DI) {
+std::pair
+Intrinsic::DagEmitter::emitDagCall(DagInit *DI, bool MatchMangledName) {
   std::vector Types;
   std::vector Values;
   for (unsigned I = 0; I < DI->getNumArgs() - 1; ++I) {
@@ -1427,7 +1431,13 @@
 N = SI->getAsUnquotedString();
   else
 N = emitDagArg(DI->getArg(0), "").second;
-  Intrinsic &Callee = Intr.Emitter.getIntrinsic(N, Types);
+  Optional MangledName;
+  if (MatchMangledName) {
+if (Intr.getRecord()->getValueAsBit("isLaneQ"))
+  N += "q";
+MangledName = Intr.mangleName(N, ClassS);
+  }
+  Intrinsic &Callee = Intr.Emitter.getIntrinsic(N, Types, MangledName);
 
   // Make sure the callee is known as an early def.
   Callee.setNeededEarly();
@@ -1832,7 +1842,8 @@
 // NeonEmitter implementation
 //===--===//
 
-Intrinsic &NeonEmitter::getIntrinsic(StringRef Name, ArrayRef Types) {
+Intrinsic &NeonEmitter::getIntrinsic(StringRef Name, ArrayRef Types,
+ Optional MangledName) {
   // First, look up the name in the intrinsic map.
   assert_with_loc(IntrinsicMap.find(Name.str()) != IntrinsicMap.end(),
   ("Intrinsic '" + Name + "' not found!").str());
@@ -1861,17 +1872,19 @@
 }
 ErrMsg += ")\n";
 
+if (MangledName && MangledName != I.getMangledName(true))
+  continue;
+
 if (I.getNumParams() != Types.size())
   continue;
 
-bool Good = true;
-for (unsigned Arg = 0; Arg < Types.size(); ++Arg) {
-  if (I.getParamType(Arg) != Types[Arg]) {
-Good = false;
-break;
-  }
-}
-if (Good)
+unsigned ArgNum = 0;
+bool MatchingArgumentTypes =
+std::all_of(Types.begin(), Types.end(), [&](const auto &Type) {
+  return Type == I.getParamType(ArgNum++);
+});
+
+if (MatchingArgumentTypes)
   GoodVec.push_back(&I);
   }
 
Index: clang/include/clang/Basic/arm_neon_incl.td
===
--- clang/include/clang/Basic/arm_neon_incl.td
+++ clang/include/clang/Basic/arm_neon_incl.td
@@ -60,6 +60,11 @@
 // example: (call "vget_high", $p0) -> "vgetq_high_s16(__p0)"
 //(assuming $p0 has type int16x8_t).
 def call;
+// call_mangled - Invoke another intrinsic matching the mangled name variation
+//of the caller's base type. If there is no intrinsic defined
+//that has the variation and takes the given types, an error
+//is generated at tblgen time.
+def call_mangled;
 // cast - Perform a cast to a different type. This gets emitted as a static
 //C-style cast. For a pure reinterpret cast (T x = *(T*)&y), use
 //"bitcast".
___
cfe-commits mailing list
cfe-commits@lists.llvm.

[PATCH] D75612: [Analyzer][StreamChecker] Adding PreCall and refactoring (NFC).

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rGaf473d0e84f1: [Analyzer][StreamChecker] Adding PreCall and 
refactoring (NFC). (authored by balazske).

Changed prior to commit:
  https://reviews.llvm.org/D75612?vs=248176&id=248665#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75612

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
@@ -50,98 +50,140 @@
 };
 
 class StreamChecker;
+struct FnDescription;
+using FnCheck = std::function;
 
-using FnCheck = std::function;
+using ArgNoTy = unsigned int;
+static const ArgNoTy ArgNone = std::numeric_limits::max();
 
 struct FnDescription {
+  FnCheck PreFn;
   FnCheck EvalFn;
+  ArgNoTy StreamArgNo;
 };
 
-class StreamChecker : public Checker {
+/// Get the value of the stream argument out of the passed call event.
+/// The call should contain a function that is described by Desc.
+SVal getStreamArg(const FnDescription *Desc, const CallEvent &Call) {
+  assert(Desc && Desc->StreamArgNo != ArgNone &&
+ "Try to get a non-existing stream argument.");
+  return Call.getArgSVal(Desc->StreamArgNo);
+}
+
+class StreamChecker
+: public Checker {
   mutable std::unique_ptr BT_nullfp, BT_illegalwhence,
   BT_doubleclose, BT_ResourceLeak;
 
 public:
+  void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
   bool evalCall(const CallEvent &Call, CheckerContext &C) const;
   void checkDeadSymbols(SymbolReaper &SymReaper, CheckerContext &C) const;
 
 private:
-
   CallDescriptionMap FnDescriptions = {
-  {{"fopen"}, {&StreamChecker::evalFopen}},
-  {{"freopen", 3}, {&StreamChecker::evalFreopen}},
-  {{"tmpfile"}, {&StreamChecker::evalFopen}},
-  {{"fclose", 1}, {&StreamChecker::evalFclose}},
-  {{"fread", 4},
-   {std::bind(&StreamChecker::checkArgNullStream, _1, _2, _3, 3)}},
-  {{"fwrite", 4},
-   {std::bind(&StreamChecker::checkArgNullStream, _1, _2, _3, 3)}},
-  {{"fseek", 3}, {&StreamChecker::evalFseek}},
-  {{"ftell", 1},
-   {std::bind(&StreamChecker::checkArgNullStream, _1, _2, _3, 0)}},
-  {{"rewind", 1},
-   {std::bind(&StreamChecker::checkArgNullStream, _1, _2, _3, 0)}},
-  {{"fgetpos", 2},
-   {std::bind(&StreamChecker::checkArgNullStream, _1, _2, _3, 0)}},
-  {{"fsetpos", 2},
-   {std::bind(&StreamChecker::checkArgNullStream, _1, _2, _3, 0)}},
-  {{"clearerr", 1},
-   {std::bind(&StreamChecker::checkArgNullStream, _1, _2, _3, 0)}},
-  {{"feof", 1},
-   {std::bind(&StreamChecker::checkArgNullStream, _1, _2, _3, 0)}},
-  {{"ferror", 1},
-   {std::bind(&StreamChecker::checkArgNullStream, _1, _2, _3, 0)}},
-  {{"fileno", 1},
-   {std::bind(&StreamChecker::checkArgNullStream, _1, _2, _3, 0)}},
+  {{"fopen"}, {nullptr, &StreamChecker::evalFopen, ArgNone}},
+  {{"freopen", 3},
+   {&StreamChecker::preFreopen, &StreamChecker::evalFreopen, 2}},
+  {{"tmpfile"}, {nullptr, &StreamChecker::evalFopen, ArgNone}},
+  {{"fclose", 1},
+   {&StreamChecker::preFclose, &StreamChecker::evalFclose, 0}},
+  {{"fread", 4}, {&StreamChecker::preDefault, nullptr, 3}},
+  {{"fwrite", 4}, {&StreamChecker::preDefault, nullptr, 3}},
+  {{"fseek", 3}, {&StreamChecker::preFseek, nullptr, 0}},
+  {{"ftell", 1}, {&StreamChecker::preDefault, nullptr, 0}},
+  {{"rewind", 1}, {&StreamChecker::preDefault, nullptr, 0}},
+  {{"fgetpos", 2}, {&StreamChecker::preDefault, nullptr, 0}},
+  {{"fsetpos", 2}, {&StreamChecker::preDefault, nullptr, 0}},
+  {{"clearerr", 1}, {&StreamChecker::preDefault, nullptr, 0}},
+  {{"feof", 1}, {&StreamChecker::preDefault, nullptr, 0}},
+  {{"ferror", 1}, {&StreamChecker::preDefault, nullptr, 0}},
+  {{"fileno", 1}, {&StreamChecker::preDefault, nullptr, 0}},
   };
 
-  void evalFopen(const CallEvent &Call, CheckerContext &C) const;
-  void evalFreopen(const CallEvent &Call, CheckerContext &C) const;
-  void evalFclose(const CallEvent &Call, CheckerContext &C) const;
-  void evalFseek(const CallEvent &Call, CheckerContext &C) const;
-  void checkArgNullStream(const CallEvent &Call, CheckerContext &C,
-  unsigned ArgI) const;
-
-  ProgramStateRef checkNullStream(SVal SV, CheckerContext &C,
-  ProgramStateRef State) const;
-  ProgramStateRef checkFseekWhence(SVal SV, CheckerContext &C,
-   ProgramStateRef State) const;
-  ProgramStateRef checkDoubleClose(const CallEvent &Call, CheckerContext &C,
-   ProgramStateRef State) const;
+  void evalFopen(const FnDescription *Desc, const CallEvent 

Re: [clang] bdad0a1 - PR45083: Mark statement expressions as being dependent if they appear in

[Hans Wennborg via cfe-commits]

On Fri, Mar 6, 2020 at 4:12 AM Richard Smith  wrote:
>
> I implemented a completely different fix in 
> a95cc77be154433c37a3110ac9af394b7447fcba. Please can you let me know if it 
> works out?

Stephan reports that it's still breaking compilations (now with an
error diagnostic rather than a crash.)

Stephan, can you share the link to the error (off-list)?

>
> The old approach was to treat statement expressions as being value- and 
> instantiation-dependent if they appear in a dependent context (matching GCC's 
> apparent behavior). But it turns out that Clang doesn't really know whether 
> it's in a dependent context! (In particular, Sema::CurContext might be a 
> template when substituting into that template, might be the enclosing 
> context, and might be the resulting declaration if it already exists. This 
> has other implications -- we at least get access checks wrong in some cases 
> -- but fixing it would be far to invasive to do for Clang 10.)
> The new approach is to walk the body of the statement expression and see if 
> it contains any value- or instantiation-dependent subexpressions, and treat 
> the statement expression as being correspondingly dependent if so.
>
> Hans, the new fix will need some work to backport due to ec3060c (it should 
> be largely mechanical, but perhaps not completely obvious what the mechanical 
> steps are). Let me know if you'd like me to do the port.
>
> On Thu, 5 Mar 2020 at 12:20, Richard Smith  wrote:
>>
>> On Thu, 5 Mar 2020 at 06:13, Benjamin Kramer via cfe-commits 
>>  wrote:
>>>
>>> creduce produced this. It's a crash on invalid, but was created from a
>>> valid input.
>>
>>
>> Well, "valid" is unclear when using language extensions, but OK.
>>
>>>
>>> $ cat r.ii
>>> template  auto b(a) {
>>>   auto c = [](auto, int) -> decltype(({})) {};
>>>   return c(0, 0);
>>> }
>>> using d = decltype(b(0));
>>> bool f = d ::e;
>>
>>
>> How important is this testcase? The fix you reverted was fixing a 
>> release-blocker; is this also a release-blocker in your view?
>>
>>>
>>> $ clang r.ii -std=c++17 -w
>>> clang-11: clang/lib/AST/Decl.cpp:2343: clang::APValue
>>> *clang::VarDecl::evaluateValue(SmallVectorImpl
>>> &) const: Assertion `!Init->isValueDependent()' failed.
>>>
>>> On Thu, Mar 5, 2020 at 2:18 PM Benjamin Kramer  wrote:
>>> >
>>> > It's still crashing clang, reverted this and
>>> > f545ede91c9d9f271e7504282cab7bf509607ead in 66addf8e8036. c-reduce is
>>> > still chewing on the reproducer.
>>> >
>>> > On Wed, Mar 4, 2020 at 10:20 PM Richard Smith via cfe-commits
>>> >  wrote:
>>> > >
>>> > > We found a regression introduced by this patch; fixed in 
>>> > > f545ede91c9d9f271e7504282cab7bf509607ead.
>>> > >
>>> > > On Wed, 4 Mar 2020 at 00:30, Hans Wennborg via cfe-commits 
>>> > >  wrote:
>>> > >>
>>> > >> Pushed to 10.x as 3a843031a5ad83a00d2603f623881cb2b2bf719d. Please let
>>> > >> me know if you hear about any follow-up issues.
>>> > >>
>>> > >> Thanks!
>>> > >>
>>> > >> On Wed, Mar 4, 2020 at 12:28 AM Richard Smith via cfe-commits
>>> > >>  wrote:
>>> > >> >
>>> > >> >
>>> > >> > Author: Richard Smith
>>> > >> > Date: 2020-03-03T15:20:40-08:00
>>> > >> > New Revision: bdad0a1b79273733df9acc1be4e992fa5d70ec56
>>> > >> >
>>> > >> > URL: 
>>> > >> > https://github.com/llvm/llvm-project/commit/bdad0a1b79273733df9acc1be4e992fa5d70ec56
>>> > >> > DIFF: 
>>> > >> > https://github.com/llvm/llvm-project/commit/bdad0a1b79273733df9acc1be4e992fa5d70ec56.diff
>>> > >> >
>>> > >> > LOG: PR45083: Mark statement expressions as being dependent if they 
>>> > >> > appear in
>>> > >> > dependent contexts.
>>> > >> >
>>> > >> > We previously assumed they were neither value- nor
>>> > >> > instantiation-dependent under any circumstances, which would lead to
>>> > >> > crashes and other misbehavior.
>>> > >> >
>>> > >> > Added:
>>> > >> >
>>> > >> >
>>> > >> > Modified:
>>> > >> > clang/include/clang/AST/Expr.h
>>> > >> > clang/include/clang/Sema/Sema.h
>>> > >> > clang/lib/AST/ASTImporter.cpp
>>> > >> > clang/lib/Parse/ParseExpr.cpp
>>> > >> > clang/lib/Sema/SemaExpr.cpp
>>> > >> > clang/lib/Sema/SemaExprCXX.cpp
>>> > >> > clang/lib/Sema/TreeTransform.h
>>> > >> > clang/test/SemaTemplate/dependent-expr.cpp
>>> > >> >
>>> > >> > Removed:
>>> > >> >
>>> > >> >
>>> > >> >
>>> > >> > 
>>> > >> > diff  --git a/clang/include/clang/AST/Expr.h 
>>> > >> > b/clang/include/clang/AST/Expr.h
>>> > >> > index fcdb0b992134..87f9b883486a 100644
>>> > >> > --- a/clang/include/clang/AST/Expr.h
>>> > >> > +++ b/clang/include/clang/AST/Expr.h
>>> > >> > @@ -3960,14 +3960,14 @@ class StmtExpr : public Expr {
>>> > >> >Stmt *SubStmt;
>>> > >> >SourceLocation LParenLoc, RParenLoc;
>>> > >> >  public:
>>> > >> > -  // FIXME: Does type-dependence need to be computed
>>> > >> > diff erently?
>>> > >> > -  // FIXME: Do we need to compute instantiation 
>>> > >> > instantiation-dependen

[clang] 8e4a867 - Revert "PR45083: Mark statement expressions as being dependent if they contain"

[Stephan Herhut via cfe-commits]

Author: Stephan Herhut
Date: 2020-03-06T11:09:45+01:00
New Revision: 8e4a8677be3061317056335d298d85ce60c23dff

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

LOG: Revert "PR45083: Mark statement expressions as being dependent if they 
contain"

This reverts commit a95cc77be154433c37a3110ac9af394b7447fcba.

Causes an internal build failure. I followed up with the author by mail.

Added: 


Modified: 
clang/include/clang/AST/Expr.h
clang/lib/AST/Expr.cpp
clang/test/SemaTemplate/dependent-expr.cpp

Removed: 




diff  --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h
index 75b7a5f6ecd3..7271dbb830a2 100644
--- a/clang/include/clang/AST/Expr.h
+++ b/clang/include/clang/AST/Expr.h
@@ -3959,8 +3959,14 @@ class StmtExpr : public Expr {
   Stmt *SubStmt;
   SourceLocation LParenLoc, RParenLoc;
 public:
-  StmtExpr(CompoundStmt *SubStmt, QualType T,
-   SourceLocation LParen, SourceLocation RParen);
+  // FIXME: Does type-dependence need to be computed 
diff erently?
+  // FIXME: Do we need to compute instantiation instantiation-dependence for
+  // statements? (ugh!)
+  StmtExpr(CompoundStmt *substmt, QualType T,
+   SourceLocation lp, SourceLocation rp) :
+Expr(StmtExprClass, T, VK_RValue, OK_Ordinary,
+ T->isDependentType(), false, false, false),
+SubStmt(substmt), LParenLoc(lp), RParenLoc(rp) { }
 
   /// Build an empty statement expression.
   explicit StmtExpr(EmptyShell Empty) : Expr(StmtExprClass, Empty) { }

diff  --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index 78fd96fd76e6..79f9f42224d0 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -4097,53 +4097,6 @@ void ExtVectorElementExpr::getEncodedElementAccess(
   }
 }
 
-StmtExpr::StmtExpr(CompoundStmt *SubStmt, QualType T, SourceLocation LParen,
-   SourceLocation RParen)
-: Expr(StmtExprClass, T, VK_RValue, OK_Ordinary, T->isDependentType(),
-   false, false, false),
-  SubStmt(SubStmt), LParenLoc(LParen), RParenLoc(RParen) {
-  llvm::SmallVector Queue(1, SubStmt);
-  while (!Queue.empty()) {
-Stmt *S = Queue.pop_back_val();
-if (!S)
-  continue;
-
-// If any subexpression is dependent, the statement expression is dependent
-// in the same way.
-if (Expr *E = dyn_cast(S)) {
-  addDependence(E->getDependence());
-  continue;
-}
-
-// FIXME: Need to properly compute whether DeclStmts contain unexpanded
-// parameter packs.
-if (DeclStmt *DS = dyn_cast(S)) {
-  for (Decl *D : DS->decls()) {
-// If any contained declaration is in a dependent context, then it
-// needs to be instantiated, so the statement expression itself is
-// instantiation-dependent.
-//
-// Note that we don't need to worry about the case where the context is
-// non-dependent but contains dependent entities here (eg, inside a
-// variable template or alias template): that can only happen at file
-// scope, where statement expressions are prohibited.
-if (D->getLexicalDeclContext()->isDependentContext())
-  addDependence(ExprDependence::Instantiation);
-
-// If any contained variable declaration has a dependent type, we can't
-// evaluate that declaration.
-if (auto *VD = dyn_cast(D))
-  if (VD->getType()->isDependentType())
-addDependence(ExprDependence::Value);
-  }
-}
-
-// Recurse to substatements.
-// FIXME: Should we skip the unchosen side of 'if constexpr' if known?
-Queue.insert(Queue.end(), S->child_begin(), S->child_end());
-  }
-}
-
 ShuffleVectorExpr::ShuffleVectorExpr(const ASTContext &C, ArrayRef args,
  QualType Type, SourceLocation BLoc,
  SourceLocation RP)

diff  --git a/clang/test/SemaTemplate/dependent-expr.cpp 
b/clang/test/SemaTemplate/dependent-expr.cpp
index 12a99acc21cd..bb1e239c3490 100644
--- a/clang/test/SemaTemplate/dependent-expr.cpp
+++ b/clang/test/SemaTemplate/dependent-expr.cpp
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -fsyntax-only -verify -std=c++2a %s
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+// expected-no-diagnostics
 
 // PR5908
 template 
@@ -107,42 +108,3 @@ namespace PR18152 {
   };
   template struct A<0>;
 }
-
-template void stmt_expr_1() {
-  // GCC doesn't check this: it appears to treat statement-expressions as being
-  // value-dependent if they appear in a dependent context, regardless of their
-  // contents.
-  static_assert( ({ false; }), "" ); // expected-error {{failed}}
-}
-void stmt_expr_2() {
-  static_assert( ({ false; }), "" ); // expected-error {{failed}}
-}
-
-namespace PR45083 {
-  struct A { bool x; };
-
-  templa

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

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

balazske added a comment.

To avoid problems I created a new version of this diff too that follows after 
the other new ones:
https://reviews.llvm.org/D75682
(Adding a new diff can make inline comment positions even more inexact 
specially if the diff has many differences from an older one?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75356



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

[PATCH] D74692: [clang-tidy][bugprone-use-after-move] Warn on std::move for consts

[Zinovy Nis via Phabricator via cfe-commits]

zinovy.nis added a comment.

Any further comments?


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

https://reviews.llvm.org/D74692



___
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.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

Just writing down my investigation results for context:
Looks like we'll never escape `$%'(,/:;?@[^{|}` anymore.

Markdown already doesn't provide backslash escaping for `$%',/:;?@^|` which 
leaves us with parentheses `([{}`:

- `[` looks fine as it is only used for hyperlinks and we escape the `]` if 
need be
- '(' again seems to be only special for providing a text for hyperlinks, so 
that should also be OK.
- `{}` apparently doesn't do anything in normal markdown but some extensions 
make use of it for adding `id`s to headers, i.e `# header1 {#mycrazyheader}`, 
so again should be OK.




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

what is the `?` for ?



Comment at: clang-tools-extra/clangd/FormattedString.cpp:73
+  // Check the start of the tag name.
+  if (Contents.empty() || !llvm::isAlpha(Contents.front()))
+return false;

we early exited already in case of empty `contents`



Comment at: clang-tools-extra/clangd/FormattedString.cpp:130
+  case '#': { // ATX heading.
+if (!(StartsLine && Before.empty()))
+  return false;

nit: if(!StartsLine || !Before.empty()))



Comment at: clang-tools-extra/clangd/FormattedString.cpp:132
+  return false;
+llvm::StringRef Rest = After.ltrim(C);
+return Rest.empty() || Rest.startswith(" ");

nit: After.ltrim('#')



Comment at: clang-tools-extra/clangd/FormattedString.cpp:133
+llvm::StringRef Rest = After.ltrim(C);
+return Rest.empty() || Rest.startswith(" ");
+  }

markdown is weird :(



Comment at: clang-tools-extra/clangd/FormattedString.cpp:139
+//   ]: is a link reference
+// The following are only links if the a link reference exists:
+//   ] by itself is a shortcut link

s/the a/the/



Comment at: clang-tools-extra/clangd/FormattedString.cpp:150
+// Not a delimiter if surrounded by space.
+return !SpaceSurrounds();
+  case '-': // Setex heading, horizontal ruler, or bullet.

`_` seems to behave different than `*` :(

it seems to rather depend on the spaces around the text being emphasized, i.e

```
foo _ bar _ foo -> no emphasis
foo _ bar_ foo -> no emphasis
foo _bar_ foo -> emphasis on bar
foo_bar_ foo -> no emphasis
```

so this should rather be `Before.endswith(" ") && isAlpha(After)` for the 
beginning of emphasis and the opposite for the ending.
Not sure if there's an easy way to decide on it in isolation.



Comment at: clang-tools-extra/clangd/FormattedString.cpp:162
+  return true;
+return !SpaceSurrounds();
+  case '<': // HTML tag (or autolink, which we choose not to escape)

nit: `return IsBullet() || RulerLength() || !SpaceSurrounds()`;



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.

is this really worth all the trouble ?



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

I believe it is also allowed to have whitespaces(less than 4?) before the `>` 
to be interpreted as a quote marker.


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] D75678: [analyzer] Skip analysis of inherited ctor as top-level function

[Gabor Marton via Phabricator via cfe-commits]

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

- Remove superfluous param x from test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75678

Files:
  clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
  clang/test/Analysis/cxx-inherited-ctor-init-expr.cpp
  clang/test/Analysis/cxx-inherited-ctor-is-skipped-as-top-level.cpp


Index: clang/test/Analysis/cxx-inherited-ctor-is-skipped-as-top-level.cpp
===
--- /dev/null
+++ clang/test/Analysis/cxx-inherited-ctor-is-skipped-as-top-level.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-display-progress 
%s 2>&1 | FileCheck %s
+
+// Test that inheriting constructors are not analyzed as top-level functions.
+
+// CHECK: ANALYZE (Path,  Inline_Regular): {{.*}} c()
+// CHECK: ANALYZE (Path,  Inline_Regular): {{.*}} a::a(int)
+// CHECK-NOT: ANALYZE (Path,  Inline_Regular): {{.*}} b::a(int)
+
+class a {
+public:
+  a(int) {}
+};
+struct b : a {
+  using a::a; // Ihnerited ctor.
+};
+void c() {
+  int d;
+  (b(d));
+  (a(d));
+}
Index: clang/test/Analysis/cxx-inherited-ctor-init-expr.cpp
===
--- clang/test/Analysis/cxx-inherited-ctor-init-expr.cpp
+++ clang/test/Analysis/cxx-inherited-ctor-init-expr.cpp
@@ -57,3 +57,19 @@
   clang_analyzer_eval(b.z == 3); // expected-warning{{TRUE}}
 }
 } // namespace arguments_with_constructors
+
+namespace inherited_constructor_crash {
+class a {
+public:
+  a(int);
+};
+struct b : a {
+  using a::a; // Ihnerited ctor.
+};
+void c() {
+  int d;
+  // This construct expr utilizes the inherited ctor.
+  // Note that d must be uninitialized to cause the crash.
+  (b(d)); // expected-warning{{1st function call argument is an uninitialized 
value}}
+}
+} // namespace inherited_constructor_crash
Index: clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
===
--- clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
+++ clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
@@ -504,6 +504,29 @@
   if (VisitedAsTopLevel.count(D))
 return true;
 
+  // Skip analysis of inheriting constructors as top-level functions. These
+  // constructors don't even have a body written down in the code, so even if
+  // we find a bug, we won't be able to display it.
+  //
+  // Also, we cannot model the parameters. CXXInheritedCtorInitExpr doesn't
+  // take arguments and doesn't model parameter initialization because there is
+  // none: the arguments in the outer CXXConstructExpr directly initialize the
+  // parameters of the base class constructor, and no copies are made. (Making
+  // a copy of the parameter is incorrect, at least if it's done in an
+  // observable way.) The derived class constructor doesn't even exist in the
+  // formal model.
+  // E.g., in:
+  //
+  // struct X { X *p = this; ~X() {} };
+  // struct A { A(X x) : b(x.p == &x) {} bool b; };
+  // struct B : A { using A::A; };
+  // B b = X{};
+  //
+  // ... b.b is initialized to true.
+  if (const auto *CD = dyn_cast(D))
+if (CD->isInheritingConstructor())
+  return true;
+
   // We want to re-analyse the functions as top level in the following cases:
   // - The 'init' methods should be reanalyzed because
   //   ObjCNonNilReturnValueChecker assumes that '[super init]' never returns


Index: clang/test/Analysis/cxx-inherited-ctor-is-skipped-as-top-level.cpp
===
--- /dev/null
+++ clang/test/Analysis/cxx-inherited-ctor-is-skipped-as-top-level.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-display-progress %s 2>&1 | FileCheck %s
+
+// Test that inheriting constructors are not analyzed as top-level functions.
+
+// CHECK: ANALYZE (Path,  Inline_Regular): {{.*}} c()
+// CHECK: ANALYZE (Path,  Inline_Regular): {{.*}} a::a(int)
+// CHECK-NOT: ANALYZE (Path,  Inline_Regular): {{.*}} b::a(int)
+
+class a {
+public:
+  a(int) {}
+};
+struct b : a {
+  using a::a; // Ihnerited ctor.
+};
+void c() {
+  int d;
+  (b(d));
+  (a(d));
+}
Index: clang/test/Analysis/cxx-inherited-ctor-init-expr.cpp
===
--- clang/test/Analysis/cxx-inherited-ctor-init-expr.cpp
+++ clang/test/Analysis/cxx-inherited-ctor-init-expr.cpp
@@ -57,3 +57,19 @@
   clang_analyzer_eval(b.z == 3); // expected-warning{{TRUE}}
 }
 } // namespace arguments_with_constructors
+
+namespace inherited_constructor_crash {
+class a {
+public:
+  a(int);
+};
+struct b : a {
+  using a::a; // Ihnerited ctor.
+};
+void c() {
+  int d;
+  // This construct expr utilizes the inherited ctor.
+  // Note that d must be uninitialized to cause the crash.
+  (b(d)); // expected-warning{{1st function call argument is an uninitialized value}}
+}

[PATCH] D75621: [clang-tidy] Use ; as separator for HeaderFileExtensions

[Nathan James via Phabricator via cfe-commits]

njames93 added inline comments.



Comment at: clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.cpp:43-45
+llvm::errs()
+<< "Using ',' as a file extension delimiter is deprecated. Please "
+   "switch your configuration to use ';'.\n";

lebedev.ri wrote:
> Can this spam be avoided?
> It's going to be really intrusive, and it is not always possible to
> just perform the migration (think: using more than one clang-tidy version.)
> The deprecation message should be in the docs/releasenotes.
That seems like a good idea. Would take a good many versions before the comma 
could safely be removed without fallout so people would get the warning too 
often and not be able to change it


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75621



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

[PATCH] D75678: [analyzer] Skip analysis of inherited ctor as top-level function

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

Ping.

Please prioritize this patch, since it is fixing a regression caused by 
https://reviews.llvm.org/D74735.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75678



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

[PATCH] D75573: [Sema][SVE] Reject aligned/_Alignas for sizeless types

[Richard Sandiford via Phabricator via cfe-commits]

rsandifo-arm updated this revision to Diff 248672.
rsandifo-arm added a comment.

Apply changes from git-clang-format.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75573

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/Sema/sizeless-1.c
  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
@@ -61,6 +61,10 @@
   svint8_t local_int8;
   svint16_t local_int16;
 
+  svint8_t __attribute__((aligned)) aligned_int8_1;// expected-error 
{{'aligned' attribute cannot be applied to sizeless type 'svint8_t'}}
+  svint8_t __attribute__((aligned(4))) aligned_int8_2; // expected-error 
{{'aligned' attribute cannot be applied to sizeless type 'svint8_t'}}
+  svint8_t _Alignas(int) aligned_int8_3;   // expected-error 
{{'_Alignas' attribute cannot be applied to sizeless type 'svint8_t'}}
+
   int _Alignas(svint8_t) aligned_int; // expected-error {{invalid application 
of 'alignof' to sizeless type 'svint8_t'}}
 
   // Using pointers to sizeless data isn't wrong here, but because the
Index: clang/test/Sema/sizeless-1.c
===
--- clang/test/Sema/sizeless-1.c
+++ clang/test/Sema/sizeless-1.c
@@ -51,6 +51,10 @@
   svint8_t local_int8;
   svint16_t local_int16;
 
+  svint8_t __attribute__((aligned)) aligned_int8_1;// expected-error 
{{'aligned' attribute cannot be applied to sizeless type 'svint8_t'}}
+  svint8_t __attribute__((aligned(4))) aligned_int8_2; // expected-error 
{{'aligned' attribute cannot be applied to sizeless type 'svint8_t'}}
+  svint8_t _Alignas(int) aligned_int8_3;   // expected-error 
{{'_Alignas' attribute cannot be applied to sizeless type 'svint8_t'}}
+
   int _Alignas(svint8_t) aligned_int; // expected-error {{invalid application 
of 'alignof' to sizeless type 'svint8_t'}}
 
   // Using pointers to sizeless data isn't wrong here, but because the
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -3868,6 +3868,7 @@
   //   not specify an alignment that is less strict than the alignment that
   //   would otherwise be required for the entity being declared.
   AlignedAttr *AlignasAttr = nullptr;
+  AlignedAttr *LastAlignedAttr = nullptr;
   unsigned Align = 0;
   for (auto *I : D->specific_attrs()) {
 if (I->isAlignmentDependent())
@@ -3875,9 +3876,13 @@
 if (I->isAlignas())
   AlignasAttr = I;
 Align = std::max(Align, I->getAlignment(Context));
+LastAlignedAttr = I;
   }
 
-  if (AlignasAttr && Align) {
+  if (Align && DiagTy->isSizelessType()) {
+Diag(LastAlignedAttr->getLocation(), diag::err_attribute_sizeless_type)
+<< LastAlignedAttr << DiagTy;
+  } else if (AlignasAttr && Align) {
 CharUnits RequestedAlign = Context.toCharUnitsFromBits(Align);
 CharUnits NaturalAlign = Context.getTypeAlignInChars(UnderlyingTy);
 if (NaturalAlign > RequestedAlign)
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -2801,6 +2801,8 @@
   "redeclaration has different alignment requirement (%1 vs %0)">;
 def err_alignas_underaligned : Error<
   "requested alignment is less than minimum alignment of %1 for type %0">;
+def err_attribute_sizeless_type : Error<
+  "%0 attribute cannot be applied to sizeless type %1">;
 def err_attribute_argument_n_type : Error<
   "%0 attribute requires parameter %1 to be %select{int or bool|an integer "
   "constant|a string|an identifier}2">;


Index: clang/test/SemaCXX/sizeless-1.cpp
===
--- clang/test/SemaCXX/sizeless-1.cpp
+++ clang/test/SemaCXX/sizeless-1.cpp
@@ -61,6 +61,10 @@
   svint8_t local_int8;
   svint16_t local_int16;
 
+  svint8_t __attribute__((aligned)) aligned_int8_1;// expected-error {{'aligned' attribute cannot be applied to sizeless type 'svint8_t'}}
+  svint8_t __attribute__((aligned(4))) aligned_int8_2; // expected-error {{'aligned' attribute cannot be applied to sizeless type 'svint8_t'}}
+  svint8_t _Alignas(int) aligned_int8_3;   // expected-error {{'_Alignas' attribute cannot be applied to sizeless type 'svint8_t'}}
+
   int _Alignas(svint8_t) aligned_int; // expected-error {{invalid application of 'alignof' to sizeless type 'svint8_t'}}
 
   // Using pointers to sizeless data isn't wrong here, but because the
Index: clang/test/Sema/sizeless-1.c
===
--- clang/test/Sema/siz

[PATCH] D72874: [clangd] Add a textual fallback for go-to-definition

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

I'd like to sync up briefly on https://github.com/clangd/clangd/issues/241 so 
we know where we want to end up.

I think this is in good shape and certainly doesn't need a bigger scope, just 
want to be able to reason about how things will fit together.




Comment at: clang-tools-extra/clangd/SourceCode.h:93
+/// the entire comment or string token.
+SourceRange getWordAtPosition(const Position &Pos, const SourceManager &SM,
+  const LangOptions &LangOpts);

nridge wrote:
> sammccall wrote:
> > sammccall wrote:
> > > consider moving the isLikelyToBeIdentifier check inside. The current API 
> > > is pretty general and it's not clear yet what (else) it's good for so 
> > > it's nice to direct towards intended usage.
> > > 
> > > Also doing the identifier check inside this function is more convenient 
> > > when it relies on markers outside the identifier range (like doxygen `\p` 
> > > or backtick-quoted identifiers)
> > > 
> > > That said, you may still want to return the range when it's not a likely 
> > > identifier, with a signature like `StringRef getWordAtPosition(bool 
> > > *LikelyIdentifier = nullptr)`. I'm thinking of the future case where the 
> > > caller wants to find a nearby matching token and resolve it - resolving 
> > > belongs in the caller so there's not much point having this function 
> > > duplicate the check.
> > This doesn't use the SourceManager-structure of the file, so the natural 
> > signature would be `StringRef getWordAtPosition(StringRef Code, unsigned 
> > Offset)`.
> > 
> > (what are the practical cases where langopts is relevant?)
> Now that I'm using `wordTouching()` from D75479, I think this comment no 
> longer applies?
I think the reasons still apply - D75479 doesn't need to check likelihood (it 
considers actual use as identifier evidence enough) so I didn't include it 
there, but we should eventually merge these more thoroughly I think. No need to 
do that until we actually want to implement different heuristics though.



Comment at: clang-tools-extra/clangd/XRefs.cpp:240
+  Req.ProximityPaths = {MainFilePath};
+  // FIXME: Set Req.Scopes to the lexically enclosing scopes.
+  // For extra strictness, consider AnyScope=false.

nridge wrote:
> sammccall wrote:
> > FWIW the API for this is visibleNamespaces() from SourceCode.cpp.
> > (No enclosing classes, but I suspect we can live without them once we have 
> > a nearby-tokens solution too)
> Thanks, that's convenient!
> 
> Out of curiosity, though: is the reason to prefer this lexer-based approach 
> over hit-testing the query location against `NamespaceDecl`s in the AST, 
> mainly for performance?
Well, it was written for fallback code completion when we have no AST at all :-)

Gathering from the AST should be better, though it's not quite as simple as 
hit-testing (you also have to find `using namespace`). But this exists today, 
which is a feature!



Comment at: clang-tools-extra/clangd/XRefs.cpp:258
+
+// For now, only consider exact name matches, including case.
+// This is to avoid too many false positives.

nridge wrote:
> sammccall wrote:
> > I wouldn't bother qualifying this as "for now". Any code is subject to 
> > change in the future, but requiring an exact name match for index-based 
> > results seems more like a design decision than a fixme.
> Do we want to rule out the possibility of handling typos in an identifier 
> name in a comment (in cases where we have high confidence in the match, e.g. 
> a long / unique name, small edit distance, only one potential match) in the 
> future?
> 
> This is also relevant to whether we want to keep the `FuzzyMatcher` or not.
No idea whether typo-correction is a good idea in principle - tradeoff between 
current false negatives and false positives+compute.

However neither FuzzyMatcher nor the existing index implementations support/can 
easily support real typo correction, and it seems implausible to me we'd add it 
for this feature.

Compare to e.g:
 - allowing case-insensitive match in some cases: `fooBar` vs `FooBar` is a 
plausible "typo". This is easy to implement.
 - correct the typo where we see the fixed version used as an identifier in 
this file (and not the original). Excludes some cases, but drives 
false-positives way down, and easy to implement.

I don't think we need to rule things out, but I'm uncertain enough about the 
approach to think that putting comments, fuzzymatcher etc here speculatively 
isn't worth it.



Comment at: clang-tools-extra/clangd/XRefs.cpp:489
+  Index->fuzzyFind(Req, [&](const Symbol &Sym) {
+auto MaybeDeclLoc =
+symbolLocationToLocation(Sym.CanonicalDeclaration, MainFilePath);

nridge wrote:
> Sorry this location-setting code is so messy. All my attempts to make it more 
> concise have been thwarted by `llvm::Expected`'s very

[PATCH] D75731: [clang-format] C# does not indent braced initializers as continuations

[Jonathan B Coe via Phabricator via cfe-commits]

jbcoe created this revision.
jbcoe added a reviewer: krasimir.
jbcoe added a project: clang-format.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

C# treats object initializers as braced init blocks. Braced init blocks are no 
longer indented as continuations.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D75731

Files:
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/FormatToken.h
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/FormatTestCSharp.cpp


Index: clang/unittests/Format/FormatTestCSharp.cpp
===
--- clang/unittests/Format/FormatTestCSharp.cpp
+++ clang/unittests/Format/FormatTestCSharp.cpp
@@ -527,14 +527,14 @@
 
   verifyFormat(R"(//
 Shape[] shapes = new[] {
-new Circle {
-Radius = 2.7281,
-Colour = Colours.Red,
-},
-new Square {
-Side = 101.1,
-Colour = Colours.Yellow,
-},
+  new Circle {
+Radius = 2.7281,
+Colour = Colours.Red,
+  },
+  new Square {
+Side = 101.1,
+Colour = Colours.Yellow,
+  },
 };)",
Style);
 
@@ -542,8 +542,8 @@
   verifyFormat(R"(//
 Shape[] shapes = new[] { new Circle { Radius = 2.7281, Colour = Colours.Red },
  new Square {
- Side = 101.1,
- Colour = Colours.Yellow,
+   Side = 101.1,
+   Colour = Colours.Yellow,
  } };)",
Style);
 
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1677,7 +1677,10 @@
   }
   break;
 case tok::l_square:
-  tryToParseLambda();
+  if (Style.isCSharp())
+parseSquare();
+  else
+tryToParseLambda();
   break;
 case tok::l_paren:
   parseParens();
Index: clang/lib/Format/FormatToken.h
===
--- clang/lib/Format/FormatToken.h
+++ clang/lib/Format/FormatToken.h
@@ -509,6 +509,9 @@
   /// Returns \c true if this tokens starts a block-type list, i.e. a
   /// list that should be indented with a block indent.
   bool opensBlockOrBlockTypeList(const FormatStyle &Style) const {
+// C# Does not indent object initialisers as continuations.
+if (is(tok::l_brace) && BlockKind == BK_BracedInit && Style.isCSharp())
+  return true;
 if (is(TT_TemplateString) && opensScope())
   return true;
 return is(TT_ArrayInitializerLSquare) || is(TT_ProtoExtensionLSquare) ||
Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -1047,6 +1047,9 @@
   if (NextNonComment->is(TT_ArraySubscriptLSquare)) {
 if (State.Stack.back().StartOfArraySubscripts != 0)
   return State.Stack.back().StartOfArraySubscripts;
+// C# allows `["Key"] = value,` in braced init lists (Object initializers).
+if (State.Stack.back().Tok->BlockKind == BK_BracedInit && Style.isCSharp())
+  return State.Stack.back().Indent;
 return ContinuationIndent;
   }
 


Index: clang/unittests/Format/FormatTestCSharp.cpp
===
--- clang/unittests/Format/FormatTestCSharp.cpp
+++ clang/unittests/Format/FormatTestCSharp.cpp
@@ -527,14 +527,14 @@
 
   verifyFormat(R"(//
 Shape[] shapes = new[] {
-new Circle {
-Radius = 2.7281,
-Colour = Colours.Red,
-},
-new Square {
-Side = 101.1,
-Colour = Colours.Yellow,
-},
+  new Circle {
+Radius = 2.7281,
+Colour = Colours.Red,
+  },
+  new Square {
+Side = 101.1,
+Colour = Colours.Yellow,
+  },
 };)",
Style);
 
@@ -542,8 +542,8 @@
   verifyFormat(R"(//
 Shape[] shapes = new[] { new Circle { Radius = 2.7281, Colour = Colours.Red },
  new Square {
- Side = 101.1,
- Colour = Colours.Yellow,
+   Side = 101.1,
+   Colour = Colours.Yellow,
  } };)",
Style);
 
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1677,7 +1677,10 @@
   }
   break;
 case tok::l_square:
-  tryToParseLambda();
+  if (Style.isCSharp())
+parseSquare();
+  else
+tryToParseLambda();
   break;
 case tok::l_paren:
   parseParens();
Index: clang/lib/Format/FormatToken.h
===
--- clang/li

[PATCH] D75732: [ASTImporter] Added visibility check for variable templates.

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

balazske created this revision.
Herald added subscribers: cfe-commits, martong, teemperor, gamesh411, 
Szelethus, dkrupp.
Herald added a reviewer: a.sidorin.
Herald added a reviewer: shafik.
Herald added a project: clang.

ASTImporter makes now difference between variable templates
with same name in different translation units if not visible
outside.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D75732

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

Index: clang/unittests/AST/ASTImporterVisibilityTest.cpp
===
--- clang/unittests/AST/ASTImporterVisibilityTest.cpp
+++ clang/unittests/AST/ASTImporterVisibilityTest.cpp
@@ -49,6 +49,12 @@
 return functionTemplateDecl(hasName("f"));
   }
 };
+struct GetVarTemplPattern {
+  using DeclTy = VarTemplateDecl;
+  BindableMatcher operator()() {
+return namedDecl(hasName("v"), has(templateTypeParmDecl()));
+  }
+};
 struct GetClassTemplPattern {
   using DeclTy = ClassTemplateDecl;
   BindableMatcher operator()() { return classTemplateDecl(hasName("X")); }
@@ -80,6 +86,10 @@
 const auto *ExternFT = "template  void f();";
 const auto *StaticFT = "template  static void f();";
 const auto *AnonFT = "namespace { template  void f(); }";
+// VarTemplateDecl:
+const auto *ExternVT = "template  extern int v;";
+const auto *StaticVT = "template  static int v;";
+const auto *AnonVT = "namespace { template  extern int v; }";
 // ClassTemplateDecl:
 const auto *ExternCT = "template  class X;";
 const auto *AnonCT = "namespace { template  class X; }";
@@ -130,6 +140,8 @@
 using ImportScopedEnumsVisibilityChain = ImportVisibilityChain;
 using ImportFunctionTemplatesVisibilityChain =
 ImportVisibilityChain;
+using ImportVariableTemplatesVisibilityChain =
+ImportVisibilityChain;
 using ImportClassTemplatesVisibilityChain =
 ImportVisibilityChain;
 
@@ -153,6 +165,10 @@
 TEST_P(ImportFunctionTemplatesVisibilityChain, ImportChain) {
   TypedTest_ImportChain();
 }
+// Value-parameterized test for variable templates.
+TEST_P(ImportVariableTemplatesVisibilityChain, ImportChain) {
+  TypedTest_ImportChain();
+}
 // Value-parameterized test for class templates.
 TEST_P(ImportClassTemplatesVisibilityChain, ImportChain) {
   TypedTest_ImportChain();
@@ -190,6 +206,11 @@
 ::testing::Combine(DefaultTestValuesForRunOptions,
::testing::Values(ExternFT, StaticFT,
  AnonFT)), );
+INSTANTIATE_TEST_CASE_P(ParameterizedTests,
+ImportVariableTemplatesVisibilityChain,
+::testing::Combine(DefaultTestValuesForRunOptions,
+   ::testing::Values(ExternVT,
+ AnonVT)), );
 INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportClassTemplatesVisibilityChain,
 ::testing::Combine(DefaultTestValuesForRunOptions,
::testing::Values(ExternCT,
@@ -306,6 +327,7 @@
 using ImportScopedEnumsVisibility = ImportVisibility;
 using ImportTypedefNameVisibility = ImportVisibility;
 using ImportFunctionTemplatesVisibility = ImportVisibility;
+using ImportVariableTemplatesVisibility = ImportVisibility;
 using ImportClassTemplatesVisibility = ImportVisibility;
 
 // FunctionDecl.
@@ -356,6 +378,13 @@
 TEST_P(ImportFunctionTemplatesVisibility, ImportAfterImport) {
   TypedTest_ImportAfterImport();
 }
+// VarTemplateDecl.
+TEST_P(ImportVariableTemplatesVisibility, ImportAfter) {
+  TypedTest_ImportAfter();
+}
+TEST_P(ImportVariableTemplatesVisibility, ImportAfterImport) {
+  TypedTest_ImportAfterImport();
+}
 // ClassTemplateDecl.
 TEST_P(ImportClassTemplatesVisibility, ImportAfter) { TypedTest_ImportAfter(); }
 TEST_P(ImportClassTemplatesVisibility, ImportAfterImport) {
@@ -462,6 +491,20 @@
 std::make_tuple(AnonFT, ExternFT, ExpectUnlinkedDeclChain),
 std::make_tuple(AnonFT, StaticFT, ExpectUnlinkedDeclChain),
 std::make_tuple(AnonFT, AnonFT, ExpectUnlinkedDeclChain))), );
+INSTANTIATE_TEST_CASE_P(
+ParameterizedTests, ImportVariableTemplatesVisibility,
+::testing::Combine(
+DefaultTestValuesForRunOptions,
+::testing::Values(
+std::make_tuple(ExternVT, ExternVT, ExpectLinkedDeclChain),
+std::make_tuple(ExternVT, StaticVT, ExpectUnlinkedDeclChain),
+std::make_tuple(ExternVT, AnonVT, ExpectUnlinkedDeclChain),
+std::make_tuple(StaticVT, ExternVT, ExpectUnlinkedDeclChain),
+std::make_tuple(StaticVT, StaticVT, ExpectUnlinkedDeclChain),
+std::make_tuple(StaticVT, AnonVT, ExpectUnlinkedDeclChain),
+std::make_tuple(AnonVT, ExternVT, ExpectUnlinkedDeclChain),
+std::make_tuple(AnonVT, StaticVT, ExpectUnlinkedDec

[PATCH] D75734: [Sema][SVE] Reject atomic 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, jfb, rkruppe, tschuett.
Herald added a reviewer: rengolin.
Herald added a project: clang.
rsandifo-arm added a parent revision: D75573: [Sema][SVE] Reject 
aligned/_Alignas for sizeless types.

It would be difficult to guarantee atomicity for sizeless types,
so the SVE ACLE makes atomic sizeless types invalid.  As it happens,
we already rejected them before the patch, but for the wrong reason:

  error: _Atomic cannot be applied to type 'svint8_t' (aka '__SVInt8_t')
  which is not trivially copyable

The SVE types should be treated as trivially copyable; a later
patch fixes that.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D75734

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaType.cpp
  clang/test/Sema/sizeless-1.c
  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
@@ -98,6 +98,7 @@
   const volatile svint8_t const_volatile_int8 = local_int8; // expected-note 
{{declared const here}}
   const volatile svint8_t uninit_const_volatile_int8;   // expected-error 
{{default initialization of an object of const type 'const volatile svint8_t'}}
 
+  _Atomic svint8_t atomic_int8;  // expected-error {{_Atomic cannot be 
applied to sizeless type 'svint8_t'}}
   __restrict svint8_t restrict_int8; // expected-error {{requires a pointer or 
reference}}
 
   bool test_int8 = init_int8; // expected-error {{cannot initialize a variable 
of type 'bool' with an lvalue of type 'svint8_t'}}
Index: clang/test/Sema/sizeless-1.c
===
--- clang/test/Sema/sizeless-1.c
+++ clang/test/Sema/sizeless-1.c
@@ -84,6 +84,7 @@
   const volatile svint8_t const_volatile_int8 = local_int8; // expected-note 
{{declared const here}}
   const volatile svint8_t uninit_const_volatile_int8;
 
+  _Atomic svint8_t atomic_int8;  // expected-error {{_Atomic cannot be 
applied to sizeless type 'svint8_t'}}
   __restrict svint8_t restrict_int8; // expected-error {{requires a pointer or 
reference}}
 
   _Bool test_int8 = init_int8; // expected-error {{initializing '_Bool' with 
an expression of incompatible type 'svint8_t'}}
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -8564,9 +8564,11 @@
   DisallowedKind = 4;
 else if (T.hasQualifiers())
   DisallowedKind = 5;
+else if (T->isSizelessType())
+  DisallowedKind = 6;
 else if (!T.isTriviallyCopyableType(Context))
   // Some other non-trivially-copyable type (probably a C++ class)
-  DisallowedKind = 6;
+  DisallowedKind = 7;
 
 if (DisallowedKind != -1) {
   Diag(Loc, diag::err_atomic_specifier_bad_type) << DisallowedKind << T;
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -5912,7 +5912,7 @@
   "incomplete result type %0 in function definition">;
 def err_atomic_specifier_bad_type : Error<
   "_Atomic cannot be applied to "
-  "%select{incomplete |array |function |reference |atomic |qualified |}0type "
+  "%select{incomplete |array |function |reference |atomic |qualified |sizeless 
|}0type "
   "%1 %select{||which is not trivially copyable}0">;
 
 // Expressions.


Index: clang/test/SemaCXX/sizeless-1.cpp
===
--- clang/test/SemaCXX/sizeless-1.cpp
+++ clang/test/SemaCXX/sizeless-1.cpp
@@ -98,6 +98,7 @@
   const volatile svint8_t const_volatile_int8 = local_int8; // expected-note {{declared const here}}
   const volatile svint8_t uninit_const_volatile_int8;   // expected-error {{default initialization of an object of const type 'const volatile svint8_t'}}
 
+  _Atomic svint8_t atomic_int8;  // expected-error {{_Atomic cannot be applied to sizeless type 'svint8_t'}}
   __restrict svint8_t restrict_int8; // expected-error {{requires a pointer or reference}}
 
   bool test_int8 = init_int8; // expected-error {{cannot initialize a variable of type 'bool' with an lvalue of type 'svint8_t'}}
Index: clang/test/Sema/sizeless-1.c
===
--- clang/test/Sema/sizeless-1.c
+++ clang/test/Sema/sizeless-1.c
@@ -84,6 +84,7 @@
   const volatile svint8_t const_volatile_int8 = local_int8; // expected-note {{declared const here}}
   const volatile svint8_t uninit_const_volatile_int8;
 
+  _Atomic svint8_t atomic_int8;  // expected-error {{_Atomic cannot be applied to sizeless type 'svint8_t'}}
   

[clang-tools-extra] c86f794 - [clangd][VSCode] Force VSCode to use the ranking provided by clangd.

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2020-03-06T13:34:25+01:00
New Revision: c86f794bd555a272f0f74a0b0a48f158e84b26b4

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

LOG: [clangd][VSCode] Force VSCode to use the ranking provided by clangd.

Summary:
Clangd's approach is to provide lots of completions, and let ranking sort them
out. This relies on various important signals (Quality.h), without which the
large completion lists are extremely spammy.

Even with a completion result exactly at the cursor, vscode looks backwards and
tries to match the presumed partial-identifier against filterText, and uses
the result to rank, with sortText only used as a tiebreak.
By prepending the partial-identifier to the filterText, we can force the match
to be perfect and so give sortText full control of the ranking.

Full sad story: https://github.com/microsoft/language-server-protocol/issues/898

It's possible to do this on the server side too of course, and switch it on
with an initialization option. But it's a little easier in the extension, it
will get the fix to users of old clangd versions, and other editors

Reviewers: hokein

Reviewed By: hokein

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

Tags: #clang

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

Added: 


Modified: 
clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts

Removed: 




diff  --git a/clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts 
b/clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts
index 4d2e04bce26a..3d8ed2d65564 100644
--- a/clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts
+++ b/clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts
@@ -98,7 +98,33 @@ export function activate(context: vscode.ExtensionContext) {
 },
 initializationOptions: { clangdFileStatus: true },
 // Do not switch to output window when clangd returns output
-revealOutputChannelOn: vscodelc.RevealOutputChannelOn.Never
+revealOutputChannelOn: vscodelc.RevealOutputChannelOn.Never,
+
+// We hack up the completion items a bit to prevent VSCode from 
re-ranking them
+// and throwing away all our delicious signals like type information.
+//
+// VSCode sorts by (fuzzymatch(prefix, item.filterText), item.sortText)
+// By adding the prefix to the beginning of the filterText, we get a 
perfect
+// fuzzymatch score for every item.
+// The sortText (which reflects clangd ranking) breaks the tie.
+//
+// We also have to mark the list as incomplete to force retrieving new 
rankings.
+// See https://github.com/microsoft/language-server-protocol/issues/898
+middleware: {
+  provideCompletionItem: async (document, position, context, token, 
next) => {
+// Get the incomplete identifier before the cursor.
+let word = document.getWordRangeAtPosition(position);
+let prefix = word && document.getText(new vscode.Range(word.start, 
position));
+
+let list = await next(document, position, context, token);
+let items = (Array.isArray(list) ? list : list.items).map(item => {
+  if (prefix)
+item.filterText = prefix + "_" + item.filterText;
+  return item;
+})
+return new vscode.CompletionList(items, /*isIncomplete=*/true);
+  }
+},
 };
 
   const clangdClient = new ClangdLanguageClient('Clang Language Server',



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

[clang] 6ef953c - [OpenCL] Align vload and vstore builtins

[Sven van Haastregt via cfe-commits]

Author: Sven van Haastregt
Date: 2020-03-06T12:45:28Z
New Revision: 6ef953c2d649cdc4df631c6cfdd54690f4914f4c

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

LOG: [OpenCL] Align vload and vstore builtins

Various vload and vstore builtins were missing or misdefined in the
TableGen description.  Align the OpenCL vload* and vstore* builtins of
the `-fdeclare-opencl-builtins` option to those of `opencl-c.h`.

Reviewed-by: Stuart Brady 

Added: 


Modified: 
clang/lib/Sema/OpenCLBuiltins.td

Removed: 




diff  --git a/clang/lib/Sema/OpenCLBuiltins.td 
b/clang/lib/Sema/OpenCLBuiltins.td
index 4ccb6b5fd49d..876409848246 100644
--- a/clang/lib/Sema/OpenCLBuiltins.td
+++ b/clang/lib/Sema/OpenCLBuiltins.td
@@ -742,17 +742,17 @@ let MaxVersion = CL20 in {
 def : Builtin, Size, 
PointerType, AS>]>;
   }
   foreach name = ["vstore" # VSize] in {
-def : Builtin, Size, 
PointerType, AS>]>;
-def : Builtin, Size, 
PointerType, AS>]>;
-def : Builtin, Size, 
PointerType, AS>]>;
-def : Builtin, Size, 
PointerType, AS>]>;
-def : Builtin, Size, 
PointerType, AS>]>;
-def : Builtin, Size, 
PointerType, AS>]>;
-def : Builtin, Size, 
PointerType, AS>]>;
-def : Builtin, Size, 
PointerType, AS>]>;
-def : Builtin, Size, 
PointerType, AS>]>;
-def : Builtin, Size, 
PointerType, AS>]>;
-def : Builtin, Size, 
PointerType, AS>]>;
+def : Builtin, Size, 
PointerType]>;
+def : Builtin, Size, 
PointerType]>;
+def : Builtin, Size, 
PointerType]>;
+def : Builtin, Size, 
PointerType]>;
+def : Builtin, Size, 
PointerType]>;
+def : Builtin, Size, 
PointerType]>;
+def : Builtin, Size, 
PointerType]>;
+def : Builtin, Size, 
PointerType]>;
+def : Builtin, Size, 
PointerType]>;
+def : Builtin, Size, 
PointerType]>;
+def : Builtin, Size, 
PointerType]>;
   }
   foreach name = ["vloada_half" # VSize] in {
 def : Builtin, Size, 
PointerType, AS>]>;
@@ -784,17 +784,17 @@ let MinVersion = CL20 in {
   def : Builtin, Size, 
PointerType, GenericAS>]>;
 }
 foreach name = ["vstore" # VSize] in {
-  def : Builtin, Size, 
PointerType, GenericAS>]>;
-  def : Builtin, Size, 
PointerType, GenericAS>]>;
-  def : Builtin, Size, 
PointerType, GenericAS>]>;
-  def : Builtin, Size, 
PointerType, GenericAS>]>;
-  def : Builtin, Size, 
PointerType, GenericAS>]>;
-  def : Builtin, Size, 
PointerType, GenericAS>]>;
-  def : Builtin, Size, 
PointerType, GenericAS>]>;
-  def : Builtin, Size, 
PointerType, GenericAS>]>;
-  def : Builtin, Size, 
PointerType, GenericAS>]>;
-  def : Builtin, Size, 
PointerType, GenericAS>]>;
-  def : Builtin, Size, 
PointerType, GenericAS>]>;
+  def : Builtin, Size, 
PointerType]>;
+  def : Builtin, Size, 
PointerType]>;
+  def : Builtin, Size, 
PointerType]>;
+  def : Builtin, Size, 
PointerType]>;
+  def : Builtin, Size, 
PointerType]>;
+  def : Builtin, Size, 
PointerType]>;
+  def : Builtin, Size, 
PointerType]>;
+  def : Builtin, Size, 
PointerType]>;
+  def : Builtin, Size, 
PointerType]>;
+  def : Builtin, Size, 
PointerType]>;
+  def : Builtin, Size, 
PointerType]>;
 }
 foreach name = ["vloada_half" # VSize] in {
   def : Builtin, Size, 
PointerType, GenericAS>]>;
@@ -825,24 +825,21 @@ foreach VSize = [2, 3, 4, 8, 16] in {
   foreach name = ["vloada_half" # VSize] in {
 def : Builtin, Size, 
PointerType, ConstantAS>]>;
   }
-  foreach rnd = ["", "_rte", "_rtz", "_rtp", "_rtn"] in {
-foreach name = ["vstorea_half" # VSize # rnd] in {
-  def : Builtin, Size, 
PointerType]>;
-  def : Builtin, Size, 
PointerType]>;
-}
-  }
 }
 let MaxVersion = CL20 in {
   foreach AS = [GlobalAS, LocalAS, PrivateAS] in {
 def : Builtin<"vload_half", [Float, Size, PointerType, 
AS>]>;
+def : Builtin<"vloada_half", [Float, Size, PointerType, 
AS>]>;
 foreach VSize = [2, 3, 4, 8, 16] in {
   foreach name = ["vload_half" # VSize] in {
 def : Builtin, Size, 
PointerType, AS>]>;
   }
 }
 foreach rnd = ["", "_rte", "_rtz", "_rtp", "_rtn"] in {
-  def : Builtin<"vstore_half" # rnd, [Void, Float, Size, PointerType]>;
-  def : Builtin<"vstore_half" # rnd, [Void, Double, Size, 
PointerType]>;
+  foreach name = ["vstore_half" # rnd, "vstorea_half" # rnd] in {
+def : Builtin]>;
+def : Builtin]>;
+  }
   foreach VSize = [2, 3, 4, 8, 16] in {
 foreach name = ["vstore_half" # VSize # rnd] in {
   def : Builtin, Size, 
PointerType]>;
@@ -855,14 +852,17 @@ let MaxVersion = CL20 in {
 let MinVersion = CL20 in {
   foreach AS = 

[PATCH] D75735: [clangd] WIP: warn the user about missing compile_commands.json

[Sam McCall via Phabricator via cfe-commits]

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D75735

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

Index: clang-tools-extra/clangd/GlobalCompilationDatabase.h
===
--- clang-tools-extra/clangd/GlobalCompilationDatabase.h
+++ clang-tools-extra/clangd/GlobalCompilationDatabase.h
@@ -86,6 +86,7 @@
 std::string Path; // Not case-folded.
 std::unique_ptr CDB = nullptr;
 bool SentBroadcast = false;
+bool Diagnosed = false;
   };
   CachedCDB &getCDBInDirLocked(PathRef File) const;
 
@@ -93,6 +94,8 @@
 PathRef FileName;
 // Whether this lookup should trigger discovery of the CDB found.
 bool ShouldBroadcast = false;
+// If no CDB is found, attempt diagnosis of the directories searched.
+bool ShouldDiagnose = false;
   };
   struct CDBLookupResult {
 tooling::CompilationDatabase *CDB = nullptr;
@@ -100,6 +103,9 @@
   };
   llvm::Optional lookupCDB(CDBLookupRequest Request) const;
 
+  // Examines a directory where a CDB was not found, to suggest fixes.
+  bool diagnose(llvm::StringRef Path) const;
+
   // Performs broadcast on governed files.
   void broadcastCDB(CDBLookupResult Res) const;
 
Index: clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
===
--- clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
+++ clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
@@ -16,6 +16,7 @@
 #include "llvm/ADT/None.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/FileUtilities.h"
@@ -71,6 +72,7 @@
   CDBLookupRequest Req;
   Req.FileName = File;
   Req.ShouldBroadcast = true;
+  Req.ShouldDiagnose = true;
 
   auto Res = lookupCDB(Req);
   if (!Res) {
@@ -129,10 +131,18 @@
 CDBLookupRequest Request) const {
   assert(llvm::sys::path::is_absolute(Request.FileName) &&
  "path must be absolute");
+  std::string CanonicalName = removeDots(Request.FileName);
 
   bool ShouldBroadcast = false;
   CDBLookupResult Result;
 
+  llvm::SmallVector DiagnoseDirs;
+  auto DiagnoseBeforeReturn = llvm::make_scope_exit([&]{
+for (PathRef Dir : DiagnoseDirs)
+  if (diagnose(Dir))
+break;
+  });
+
   {
 std::lock_guard Lock(Mutex);
 CachedCDB *Entry = nullptr;
@@ -142,16 +152,22 @@
   // Traverse the canonical version to prevent false positives. i.e.:
   // src/build/../a.cc can detect a CDB in /src/build if not canonicalized.
   // FIXME(sammccall): this loop is hot, use a union-find-like structure.
-  actOnAllParentDirectories(removeDots(Request.FileName),
-[&](PathRef Path) {
-  Entry = &getCDBInDirLocked(Path);
-  return Entry->CDB != nullptr;
-});
+  actOnAllParentDirectories(CanonicalName, [&](PathRef Path) {
+Entry = &getCDBInDirLocked(Path);
+if (Request.ShouldDiagnose && !Entry->CDB && !Entry->Diagnosed) {
+  Entry->Diagnosed = true;
+  DiagnoseDirs.push_back(Path);
+}
+return Entry->CDB != nullptr;
+  });
 }
 
 if (!Entry || !Entry->CDB)
   return llvm::None;
 
+// Don't diagnose if we found a CDB in the end.
+DiagnoseDirs.clear();
+
 // Mark CDB as broadcasted to make sure discovery is performed once.
 if (Request.ShouldBroadcast && !Entry->SentBroadcast) {
   Entry->SentBroadcast = true;
@@ -231,6 +247,29 @@
   return Res->PI;
 }
 
+bool DirectoryBasedGlobalCompilationDatabase::diagnose(PathRef Dir) const {
+  vlog("Diagnosing missing CDB in {0}", Dir);
+
+  namespace fs = llvm::sys::fs;
+  namespace path = llvm::sys::path;
+  llvm::SmallString<256> Path = Dir;
+  auto Exists = [&](llvm::StringRef Filename){
+auto Size = Path.size();
+path::append(Path, Filename);
+bool Ret = fs::exists(Path);
+Path.resize(Size);
+return Ret;
+  };
+  if (Exists("CMakeLists.txt")) {
+Path.resize(path::parent_path(Path).size());
+if (!Exists("CMakeLists.txt")) {
+  elog("Missing compile_commands.json in {0}", Dir);
+  return true;
+}
+  }
+  return false;
+}
+
 OverlayCDB::OverlayCDB(const GlobalCompilationDatabase *Base,
std::vector FallbackFlags,
tooling::ArgumentsAdjuster Adjuster)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D75737: [Sema][SVE] Don't allow fields to have sizeless type

[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: D75736: [Sema][SVE] Don't allow static or 
thread-local variables to have sizeless type.
rsandifo-arm added a child revision: D75738: [Sema][SVE] Reject by-copy capture 
of sizeless types.

The SVE ACLE doesn't allow fields to have sizeless type.  At the moment
clang accepts things like:

  struct s { __SVInt8_t x; } y;

but trying to code-generate it leads to LLVM asserts like:

  llvm/include/llvm/Support/TypeSize.h:126: uint64_t 
llvm::TypeSize::getFixedSize() const: Assertion `!IsScalable && "Request for a 
fixed size on a scalable object"' failed.

This patch adds an associated clang diagnostic.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D75737

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaLambda.cpp
  clang/test/Sema/sizeless-1.c
  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
@@ -249,6 +249,20 @@
   return count;
 }
 
+struct sized_struct {
+  int f1;
+  svint8_t f2; // expected-error {{field has sizeless type 'svint8_t'}}
+  svint8_t f3 : 2; // expected-error {{field has sizeless type 'svint8_t'}}
+  svint8_t : 3;// expected-error {{field has sizeless type 'svint8_t'}}
+};
+
+union sized_union {
+  int f1;
+  svint8_t f2; // expected-error {{field has sizeless type 'svint8_t'}}
+  svint8_t f3 : 2; // expected-error {{field has sizeless type 'svint8_t'}}
+  svint8_t : 3;// expected-error {{field has sizeless type 'svint8_t'}}
+};
+
 void pass_int8_ref(svint8_t &); // expected-note {{not viable}}
 
 svint8_t &return_int8_ref();
@@ -256,6 +270,11 @@
 svint8_t &&return_int8_rvalue_ref();
 #endif
 
+template 
+struct s_template {
+  T y; // expected-error {{field has sizeless type '__SVInt8_t'}}
+};
+
 template 
 struct s_ptr_template {
   s_ptr_template();
@@ -344,6 +363,9 @@
   local_int8 = svint8_t();
   local_int8 = svint16_t(); // expected-error {{assigning to 'svint8_t' (aka '__SVInt8_t') from incompatible type 'svint16_t'}}
 
+  s_template st_int;
+  s_template st_svint8; // expected-note {{in instantiation}}
+
   s_ptr_template st_ptr_int;
   s_ptr_template st_ptr_svint8;
 
Index: clang/test/Sema/sizeless-1.c
===
--- clang/test/Sema/sizeless-1.c
+++ clang/test/Sema/sizeless-1.c
@@ -230,6 +230,20 @@
   return count;
 }
 
+struct sized_struct {
+  int f1;
+  svint8_t f2; // expected-error {{field has sizeless type 'svint8_t'}}
+  svint8_t f3 : 2; // expected-error {{field has sizeless type 'svint8_t'}}
+  svint8_t : 3;// expected-error {{field has sizeless type 'svint8_t'}}
+};
+
+union sized_union {
+  int f1;
+  svint8_t f2; // expected-error {{field has sizeless type 'svint8_t'}}
+  svint8_t f3 : 2; // expected-error {{field has sizeless type 'svint8_t'}}
+  svint8_t : 3;// expected-error {{field has sizeless type 'svint8_t'}}
+};
+
 #if __STDC_VERSION__ >= 201112L
 void test_generic(void) {
   svint8_t local_int8;
Index: clang/lib/Sema/SemaLambda.cpp
===
--- clang/lib/Sema/SemaLambda.cpp
+++ clang/lib/Sema/SemaLambda.cpp
@@ -1629,7 +1629,8 @@
   // If the variable being captured has an invalid type, mark the class as
   // invalid as well.
   if (!FieldType->isDependentType()) {
-if (RequireCompleteType(Loc, FieldType, diag::err_field_incomplete)) {
+if (RequireCompleteSizedType(Loc, FieldType,
+ diag::err_field_incomplete_or_sizeless)) {
   RD->setInvalidDecl();
   Field->setInvalidDecl();
 } else {
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -5247,8 +5247,8 @@
   Chain.push_back(Anon);
 
   RecordDecl *RecordDef = Record->getDefinition();
-  if (RequireCompleteType(Anon->getLocation(), RecTy,
-  diag::err_field_incomplete) ||
+  if (RequireCompleteSizedType(Anon->getLocation(), RecTy,
+   diag::err_field_incomplete_or_sizeless) ||
   InjectAnonymousStructOrUnionMembers(*this, S, CurContext, RecordDef,
   AS_none, Chain)) {
 Anon->setInvalidDecl();
@@ -16129,8 +16129,9 @@
   // C99 6.7.2.1p4 - verify the field type.
   // C++ 9.6p3: A bit-field shall have integral or enumeration type.
   if (!FieldTy->isDependentType() && !FieldTy->isIntegralOrEnumerationType()) {
-// Handle incomplete types with specific error.
-if (RequireComple

[PATCH] D70086: [ConstExprPreter] Implemented control flow statements

[Nandor Licker via Phabricator via cfe-commits]

nand updated this revision to Diff 248701.
nand added a comment.

rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70086

Files:
  clang/lib/AST/Interp/ByteCodeEmitter.cpp
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/lib/AST/Interp/ByteCodeExprGen.h
  clang/lib/AST/Interp/ByteCodeStmtGen.cpp
  clang/lib/AST/Interp/ByteCodeStmtGen.h
  clang/lib/AST/Interp/Context.cpp
  clang/lib/AST/Interp/Context.h
  clang/lib/AST/Interp/EvalEmitter.cpp
  clang/lib/AST/Interp/InterpLoop.cpp
  clang/lib/AST/Interp/Opcodes.td
  clang/lib/AST/Interp/Opcodes/Comparison.h

Index: clang/lib/AST/Interp/Opcodes/Comparison.h
===
--- clang/lib/AST/Interp/Opcodes/Comparison.h
+++ clang/lib/AST/Interp/Opcodes/Comparison.h
@@ -148,5 +148,16 @@
   return false;
 }
 
+template 
+bool InRange(InterpState &S, CodePtr OpPC) {
+  using T = typename PrimConv::T;
+  const T &RHS = S.Stk.pop();
+  const T &LHS = S.Stk.pop();
+  const T &Value = S.Stk.pop();
+
+  S.Stk.push(LHS <= Value && Value <= RHS);
+  return true;
+}
+
 #endif
 
Index: clang/lib/AST/Interp/Opcodes.td
===
--- clang/lib/AST/Interp/Opcodes.td
+++ clang/lib/AST/Interp/Opcodes.td
@@ -75,7 +75,11 @@
 }
 
 def FPTypeClass : TypeClass {
-  let Types = IntTypeClass.Types;
+  let Types = [];
+}
+
+def IntFPTypeClass : TypeClass {
+  let Types = !listconcat(IntTypeClass.Types, FPTypeClass.Types);
 }
 
 def AluFPRealFPTypeClass : TypeClass {
@@ -334,6 +338,16 @@
 def GT : ComparisonOpcode;
 def GE : ComparisonOpcode;
 
+//===--===//
+// Range test.
+//===--===//
+
+// [Real, Real, Real] -> [Bool]
+def InRange : Opcode {
+  let Types = [IntFPTypeClass];
+  let HasGroup = 1;
+}
+
 //===--===//
 // Stack management.
 //===--===//
Index: clang/lib/AST/Interp/InterpLoop.cpp
===
--- clang/lib/AST/Interp/InterpLoop.cpp
+++ clang/lib/AST/Interp/InterpLoop.cpp
@@ -108,6 +108,8 @@
 return false;
   if (S.checkingPotentialConstantExpression())
 return false;
+  if (!F->isConstexpr())
+return false;
 
   // Adjust the state.
   S.CallStackDepth++;
Index: clang/lib/AST/Interp/EvalEmitter.cpp
===
--- clang/lib/AST/Interp/EvalEmitter.cpp
+++ clang/lib/AST/Interp/EvalEmitter.cpp
@@ -177,6 +177,8 @@
 return false;
   if (S.checkingPotentialConstantExpression())
 return false;
+  if (!F->isConstexpr())
+return false;
   S.Current = new InterpFrame(S, F, S.Current, OpPC, std::move(This));
   return Interpret(S, Result);
 }
Index: clang/lib/AST/Interp/Context.h
===
--- clang/lib/AST/Interp/Context.h
+++ clang/lib/AST/Interp/Context.h
@@ -70,13 +70,6 @@
   /// Classifies an expression.
   llvm::Optional classify(QualType T);
 
-private:
-  /// Runs a function.
-  bool Run(State &Parent, Function *Func, APValue &Result);
-
-  /// Checks a result fromt the interpreter.
-  bool Check(State &Parent, llvm::Expected &&R);
-
 private:
   /// Current compilation context.
   ASTContext &Ctx;
Index: clang/lib/AST/Interp/Context.cpp
===
--- clang/lib/AST/Interp/Context.cpp
+++ clang/lib/AST/Interp/Context.cpp
@@ -27,6 +27,8 @@
 Context::~Context() {}
 
 bool Context::isPotentialConstantExpr(State &Parent, const FunctionDecl *FD) {
+  // Try to compile the function. This either produces an error message (if this
+  // is the first attempt to compile) or returns a dummy function with no body.
   Function *Func = P->getFunction(FD);
   if (!Func) {
 ByteCodeStmtGen C(*this, *P, Parent);
@@ -40,22 +42,43 @@
 }
   }
 
+  // If function has no body, it is definitely not constexpr.
   if (!Func->isConstexpr())
 return false;
 
-  APValue Dummy;
-  return Run(Parent, Func, Dummy);
+  // Run the function in a dummy context.
+  APValue DummyResult;
+  InterpState State(Parent, *P, Stk, *this);
+  State.Current = new InterpFrame(State, Func, nullptr, {}, {});
+  if (Interpret(State, DummyResult))
+return true;
+  Stk.clear();
+  return false;
 }
 
 bool Context::evaluateAsRValue(State &Parent, const Expr *E, APValue &Result) {
   ByteCodeExprGen C(*this, *P, Parent, Stk, Result);
-  return Check(Parent, C.interpretExpr(E));
+  if (auto Flag = C.interpretExpr(E)) {
+return *Flag;
+  } else {
+handleAllErrors(Flag.takeError(), [&Parent](ByteCodeGenError &Err) {
+  Parent.FFDiag(Err.getLoc(), diag::err_experimental_clang_interp_failed);
+  

[PATCH] D75738: [Sema][SVE] Reject by-copy capture of 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 project: clang.
rsandifo-arm added a parent revision: D75737: [Sema][SVE] Don't allow fields to 
have sizeless type.

Since fields can't have sizeless type, it also doesn't make sense
to capture sizeless types by value in lambda expressions.  This patch
makes sure that we diagnose that and that we use "sizeless type" rather
"incomplete type" in the associated message.  (Both are correct, but
"sizeless type" is more specific and hopefully more user-friendly.)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D75738

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaExpr.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
@@ -496,6 +496,7 @@
   local_int8 = ([]() -> svint8_t { return svint8_t(); })();
   auto fn1 = [&local_int8](svint8_t x) { local_int8 = x; };
   auto fn2 = [&local_int8](svint8_t *ptr) { *ptr = local_int8; };
+  auto fn3 = [local_int8](svint8_t *ptr) { *ptr = local_int8; }; // 
expected-error {{by-copy capture of variable 'local_int8' with sizeless type 
'svint8_t'}}
 
   for (auto x : local_int8) { // expected-error {{no viable 'begin' function 
available}}
   }
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -16341,9 +16341,10 @@
 // Make sure that by-copy captures are of a complete and non-abstract type.
 if (!Invalid && BuildAndDiagnose) {
   if (!CaptureType->isDependentType() &&
-  S.RequireCompleteType(Loc, CaptureType,
-diag::err_capture_of_incomplete_type,
-Var->getDeclName()))
+  S.RequireCompleteSizedType(
+  Loc, CaptureType,
+  diag::err_capture_of_incomplete_or_sizeless_type,
+  Var->getDeclName()))
 Invalid = true;
   else if (S.RequireNonAbstractType(Loc, CaptureType,
 diag::err_capture_of_abstract_type))
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -1474,8 +1474,8 @@
 def err_array_of_abstract_type : Error<"array of abstract class type %0">;
 def err_capture_of_abstract_type : Error<
   "by-copy capture of value of abstract type %0">;
-def err_capture_of_incomplete_type : Error<
-  "by-copy capture of variable %0 with incomplete type %1">;
+def err_capture_of_incomplete_or_sizeless_type : Error<
+  "by-copy capture of variable %0 with %select{incomplete|sizeless}1 type %2">;
 def err_capture_default_non_local : Error<
   "non-local lambda expression cannot have a capture-default">;
 


Index: clang/test/SemaCXX/sizeless-1.cpp
===
--- clang/test/SemaCXX/sizeless-1.cpp
+++ clang/test/SemaCXX/sizeless-1.cpp
@@ -496,6 +496,7 @@
   local_int8 = ([]() -> svint8_t { return svint8_t(); })();
   auto fn1 = [&local_int8](svint8_t x) { local_int8 = x; };
   auto fn2 = [&local_int8](svint8_t *ptr) { *ptr = local_int8; };
+  auto fn3 = [local_int8](svint8_t *ptr) { *ptr = local_int8; }; // expected-error {{by-copy capture of variable 'local_int8' with sizeless type 'svint8_t'}}
 
   for (auto x : local_int8) { // expected-error {{no viable 'begin' function available}}
   }
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -16341,9 +16341,10 @@
 // Make sure that by-copy captures are of a complete and non-abstract type.
 if (!Invalid && BuildAndDiagnose) {
   if (!CaptureType->isDependentType() &&
-  S.RequireCompleteType(Loc, CaptureType,
-diag::err_capture_of_incomplete_type,
-Var->getDeclName()))
+  S.RequireCompleteSizedType(
+  Loc, CaptureType,
+  diag::err_capture_of_incomplete_or_sizeless_type,
+  Var->getDeclName()))
 Invalid = true;
   else if (S.RequireNonAbstractType(Loc, CaptureType,
 diag::err_capture_of_abstract_type))
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -1474,8 +1474,8 @@
 def err_array_of_abstract_type : Error<"array of abstract class type %0"

[PATCH] D75736: [Sema][SVE] Don't allow static or thread-local variables to have sizeless type

[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 project: clang.
rsandifo-arm added a parent revision: D75734: [Sema][SVE] Reject atomic 
sizeless types.
rsandifo-arm added a child revision: D75737: [Sema][SVE] Don't allow fields to 
have sizeless type.

clang accepts a TU containing just:

  __SVInt8_t x;

However, sizeless types are not allowed to have static or thread-local
storage duration and trying to code-generate the TU triggers an LLVM
fatal error:

  Globals cannot contain scalable vectors
  * @x
  fatal error: error in backend: Broken module found, compilation aborted!

This patch adds an associated clang diagnostic.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D75736

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDecl.cpp
  clang/test/Sema/sizeless-1.c
  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
@@ -10,6 +10,10 @@
 typedef __SVInt8_t svint8_t;
 typedef __SVInt16_t svint16_t;
 
+svint8_t global_int8;  // expected-error {{non-local variable with 
sizeless type 'svint8_t'}}
+extern svint8_t extern_int8;   // expected-error {{non-local variable with 
sizeless type 'svint8_t'}}
+static svint8_t static_int8;   // expected-error {{non-local variable with 
sizeless type 'svint8_t'}}
+__thread svint8_t thread_int8; // expected-error {{non-local variable with 
sizeless type 'svint8_t'}}
 svint8_t *global_int8_ptr;
 extern svint8_t *extern_int8_ptr;
 static svint8_t *static_int8_ptr;
@@ -58,6 +62,8 @@
 struct incomplete_struct *incomplete_ptr;
 
 void func(int sel) {
+  static svint8_t static_int8; // expected-error {{non-local variable with 
sizeless type 'svint8_t'}}
+
   svint8_t local_int8;
   svint16_t local_int16;
 
Index: clang/test/Sema/sizeless-1.c
===
--- clang/test/Sema/sizeless-1.c
+++ clang/test/Sema/sizeless-1.c
@@ -5,6 +5,10 @@
 typedef __SVInt8_t svint8_t;
 typedef __SVInt16_t svint16_t;
 
+svint8_t global_int8;  // expected-error {{non-local variable with 
sizeless type 'svint8_t'}}
+extern svint8_t extern_int8;   // expected-error {{non-local variable with 
sizeless type 'svint8_t'}}
+static svint8_t static_int8;   // expected-error {{non-local variable with 
sizeless type 'svint8_t'}}
+__thread svint8_t thread_int8; // expected-error {{non-local variable with 
sizeless type 'svint8_t'}}
 svint8_t *global_int8_ptr;
 extern svint8_t *extern_int8_ptr;
 static svint8_t *static_int8_ptr;
@@ -48,6 +52,8 @@
 struct incomplete_struct *incomplete_ptr;
 
 void func(int sel) {
+  static svint8_t static_int8; // expected-error {{non-local variable with 
sizeless type 'svint8_t'}}
+
   svint8_t local_int8;
   svint16_t local_int16;
 
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -7939,6 +7939,12 @@
 return;
   }
 
+  if (!NewVD->hasLocalStorage() && T->isSizelessType()) {
+Diag(NewVD->getLocation(), diag::err_sizeless_nonlocal) << T;
+NewVD->setInvalidDecl();
+return;
+  }
+
   if (isVM && NewVD->hasAttr()) {
 Diag(NewVD->getLocation(), diag::err_block_on_vm);
 NewVD->setInvalidDecl();
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -9132,6 +9132,8 @@
   "__block attribute not allowed, only allowed on local variables">;
 def err_block_on_vm : Error<
   "__block attribute not allowed on declaration with a variably modified 
type">;
+def err_sizeless_nonlocal : Error<
+  "non-local variable with sizeless type %0">;
 
 def err_vec_builtin_non_vector : Error<
  "first two arguments to %0 must be vectors">;


Index: clang/test/SemaCXX/sizeless-1.cpp
===
--- clang/test/SemaCXX/sizeless-1.cpp
+++ clang/test/SemaCXX/sizeless-1.cpp
@@ -10,6 +10,10 @@
 typedef __SVInt8_t svint8_t;
 typedef __SVInt16_t svint16_t;
 
+svint8_t global_int8;  // expected-error {{non-local variable with sizeless type 'svint8_t'}}
+extern svint8_t extern_int8;   // expected-error {{non-local variable with sizeless type 'svint8_t'}}
+static svint8_t static_int8;   // expected-error {{non-local variable with sizeless type 'svint8_t'}}
+__thread svint8_t thread_int8; // expected-error {{non-local variable with sizeless type 'svint8_t'}}
 svint8_t *global_int8_ptr;
 extern svint8_t *extern_int8_ptr;
 static svint8_t *static_int8_ptr;
@@ -58,6 +62,8 @@
 struct incomplete_struct *incomplete_ptr;
 
 vo

[PATCH] D75621: [clang-tidy] Use ; as separator for HeaderFileExtensions

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.cpp:43-45
+llvm::errs()
+<< "Using ',' as a file extension delimiter is deprecated. Please "
+   "switch your configuration to use ';'.\n";

njames93 wrote:
> lebedev.ri wrote:
> > Can this spam be avoided?
> > It's going to be really intrusive, and it is not always possible to
> > just perform the migration (think: using more than one clang-tidy version.)
> > The deprecation message should be in the docs/releasenotes.
> That seems like a good idea. Would take a good many versions before the comma 
> could safely be removed without fallout so people would get the warning too 
> often and not be able to change it
FWIW, I was on the fence about this being a chatty diagnostic as well. I 
eventually figured that it was fine because it's going to `stderr` and it 
definitely gets the job done of letting people know. Most folks don't read 
release notes, so putting the information there is a good idea, but not really 
sufficient for warning about deprecation.

I don't think we have a better solution at hand and the point @lebedev.ri 
brings up about older versions is a valid one, so I agree, let's move this into 
the release notes rather than spamming the command line.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75621



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

[PATCH] D75623: [clangd][VSCode] Force VSCode to use the ranking provided by clangd.

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGc86f794bd555: [clangd][VSCode] Force VSCode to use the 
ranking provided by clangd. (authored by sammccall).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75623

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


Index: clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts
===
--- clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts
+++ clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts
@@ -98,7 +98,33 @@
 },
 initializationOptions: { clangdFileStatus: true },
 // Do not switch to output window when clangd returns output
-revealOutputChannelOn: vscodelc.RevealOutputChannelOn.Never
+revealOutputChannelOn: vscodelc.RevealOutputChannelOn.Never,
+
+// We hack up the completion items a bit to prevent VSCode from 
re-ranking them
+// and throwing away all our delicious signals like type information.
+//
+// VSCode sorts by (fuzzymatch(prefix, item.filterText), item.sortText)
+// By adding the prefix to the beginning of the filterText, we get a 
perfect
+// fuzzymatch score for every item.
+// The sortText (which reflects clangd ranking) breaks the tie.
+//
+// We also have to mark the list as incomplete to force retrieving new 
rankings.
+// See https://github.com/microsoft/language-server-protocol/issues/898
+middleware: {
+  provideCompletionItem: async (document, position, context, token, 
next) => {
+// Get the incomplete identifier before the cursor.
+let word = document.getWordRangeAtPosition(position);
+let prefix = word && document.getText(new vscode.Range(word.start, 
position));
+
+let list = await next(document, position, context, token);
+let items = (Array.isArray(list) ? list : list.items).map(item => {
+  if (prefix)
+item.filterText = prefix + "_" + item.filterText;
+  return item;
+})
+return new vscode.CompletionList(items, /*isIncomplete=*/true);
+  }
+},
 };
 
   const clangdClient = new ClangdLanguageClient('Clang Language Server',


Index: clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts
===
--- clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts
+++ clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts
@@ -98,7 +98,33 @@
 },
 initializationOptions: { clangdFileStatus: true },
 // Do not switch to output window when clangd returns output
-revealOutputChannelOn: vscodelc.RevealOutputChannelOn.Never
+revealOutputChannelOn: vscodelc.RevealOutputChannelOn.Never,
+
+// We hack up the completion items a bit to prevent VSCode from re-ranking them
+// and throwing away all our delicious signals like type information.
+//
+// VSCode sorts by (fuzzymatch(prefix, item.filterText), item.sortText)
+// By adding the prefix to the beginning of the filterText, we get a perfect
+// fuzzymatch score for every item.
+// The sortText (which reflects clangd ranking) breaks the tie.
+//
+// We also have to mark the list as incomplete to force retrieving new rankings.
+// See https://github.com/microsoft/language-server-protocol/issues/898
+middleware: {
+  provideCompletionItem: async (document, position, context, token, next) => {
+// Get the incomplete identifier before the cursor.
+let word = document.getWordRangeAtPosition(position);
+let prefix = word && document.getText(new vscode.Range(word.start, position));
+
+let list = await next(document, position, context, token);
+let items = (Array.isArray(list) ? list : list.items).map(item => {
+  if (prefix)
+item.filterText = prefix + "_" + item.filterText;
+  return item;
+})
+return new vscode.CompletionList(items, /*isIncomplete=*/true);
+  }
+},
 };
 
   const clangdClient = new ClangdLanguageClient('Clang Language Server',
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D75563: [clang][Parse] properly parse asm-qualifiers, asm inline

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/Parse/ParseStmtAsm.cpp:704
+  AQ = DeclSpec::AQ_goto;
+else {
+  if (EndLoc.isValid())

nickdesaulniers wrote:
> aaron.ballman wrote:
> > I would expect a diagnostic if the unknown token is anything other than a 
> > left paren.
> That currently is is handled by the caller, see `if (Tok.isNot(tok::l_paren)) 
> {` in `Parser::ParseAsmStatement` (line 762 of 
> clang/lib/Parse/ParseStmtAsm.cpp).
My suggestion is to move it out of the caller and into the helper. This 
function is expected to parse the asm qualifiers, so it stands to reason if it 
gets something other than an asm qualifier, it should diagnose. (We probably 
won't need to re-use this parsing logic in other contexts, but that's why it's 
better to have the diagnostic here rather than assume the caller will do it.)



Comment at: clang/lib/Parse/ParseStmtAsm.cpp:725
 /// [GNU] gnu-asm-statement:
-/// 'asm' type-qualifier[opt] '(' asm-argument ')' ';'
+/// 'asm' asm-qualifier[opt] '(' asm-argument ')' ';'
 ///

nickdesaulniers wrote:
> I guess this should be `asm-qualifier-list[opt]`?
Yup, good catch!



Comment at: clang/test/Parser/asm-qualifiers.c:12-14
+  asm noodle(""); // expected-error {{expected '(' after 'asm'}}
+  asm goto noodle("" foo); // expected-error {{expected '(' after 'asm'}}
+  asm volatile noodle inline(""); // expected-error {{expected '(' after 
'asm'}}

I think these diagnostics should be improved as none of them suggest the 
underlying problem. As they read now, it sounds like you can write 
`asm(volatile noodle inline(""))` to appease the diagnostic.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75563



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

[PATCH] D75731: [clang-format] C# does not indent braced initializers as continuations

[Jonathan B Coe via Phabricator via cfe-commits]

jbcoe updated this revision to Diff 248712.

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

https://reviews.llvm.org/D75731

Files:
  clang/lib/Format/FormatToken.h
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/FormatTestCSharp.cpp


Index: clang/unittests/Format/FormatTestCSharp.cpp
===
--- clang/unittests/Format/FormatTestCSharp.cpp
+++ clang/unittests/Format/FormatTestCSharp.cpp
@@ -527,31 +527,31 @@
 
   verifyFormat(R"(//
 Shape[] shapes = new[] {
-new Circle {
-Radius = 2.7281,
-Colour = Colours.Red,
-},
-new Square {
-Side = 101.1,
-Colour = Colours.Yellow,
-},
+  new Circle {
+Radius = 2.7281,
+Colour = Colours.Red,
+  },
+  new Square {
+Side = 101.1,
+Colour = Colours.Yellow,
+  },
 };)",
Style);
 
   // Omitted final `,`s will change the formatting.
   verifyFormat(R"(//
-Shape[] shapes = new[] { new Circle { Radius = 2.7281, Colour = Colours.Red },
- new Square {
- Side = 101.1,
- Colour = Colours.Yellow,
- } };)",
+Shape[] shapes =
+new[] { new Circle { Radius = 2.7281, Colour = Colours.Red }, new Square {
+ Side = 101.1,
+ Colour = Colours.Yellow,
+   } };)",
Style);
 
   // Lambdas can be supplied as initialiser arguments.
   verifyFormat(R"(//
 private Transformer _transformer = new X.Y {
-Filler = (Shape shape) => { return new Transform.Fill(shape, RED); },
-Scaler = (Shape shape) => { return new Transform.Resize(shape, 0.1); },
+  Filler = (Shape shape) => { return new Transform.Fill(shape, RED); },
+  Scaler = (Shape shape) => { return new Transform.Resize(shape, 0.1); },
 };)",
Style);
 }
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1677,7 +1677,10 @@
   }
   break;
 case tok::l_square:
-  tryToParseLambda();
+  if (Style.isCSharp())
+parseSquare();
+  else
+tryToParseLambda();
   break;
 case tok::l_paren:
   parseParens();
Index: clang/lib/Format/FormatToken.h
===
--- clang/lib/Format/FormatToken.h
+++ clang/lib/Format/FormatToken.h
@@ -509,6 +509,9 @@
   /// Returns \c true if this tokens starts a block-type list, i.e. a
   /// list that should be indented with a block indent.
   bool opensBlockOrBlockTypeList(const FormatStyle &Style) const {
+// C# Does not indent object initialisers as continuations.
+if (is(tok::l_brace) && BlockKind == BK_BracedInit && Style.isCSharp())
+  return true;
 if (is(TT_TemplateString) && opensScope())
   return true;
 return is(TT_ArrayInitializerLSquare) || is(TT_ProtoExtensionLSquare) ||


Index: clang/unittests/Format/FormatTestCSharp.cpp
===
--- clang/unittests/Format/FormatTestCSharp.cpp
+++ clang/unittests/Format/FormatTestCSharp.cpp
@@ -527,31 +527,31 @@
 
   verifyFormat(R"(//
 Shape[] shapes = new[] {
-new Circle {
-Radius = 2.7281,
-Colour = Colours.Red,
-},
-new Square {
-Side = 101.1,
-Colour = Colours.Yellow,
-},
+  new Circle {
+Radius = 2.7281,
+Colour = Colours.Red,
+  },
+  new Square {
+Side = 101.1,
+Colour = Colours.Yellow,
+  },
 };)",
Style);
 
   // Omitted final `,`s will change the formatting.
   verifyFormat(R"(//
-Shape[] shapes = new[] { new Circle { Radius = 2.7281, Colour = Colours.Red },
- new Square {
- Side = 101.1,
- Colour = Colours.Yellow,
- } };)",
+Shape[] shapes =
+new[] { new Circle { Radius = 2.7281, Colour = Colours.Red }, new Square {
+ Side = 101.1,
+ Colour = Colours.Yellow,
+   } };)",
Style);
 
   // Lambdas can be supplied as initialiser arguments.
   verifyFormat(R"(//
 private Transformer _transformer = new X.Y {
-Filler = (Shape shape) => { return new Transform.Fill(shape, RED); },
-Scaler = (Shape shape) => { return new Transform.Resize(shape, 0.1); },
+  Filler = (Shape shape) => { return new Transform.Fill(shape, RED); },
+  Scaler = (Shape shape) => { return new Transform.Resize(shape, 0.1); },
 };)",
Style);
 }
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1677,7 +1677,10 @@
   }
   break;
 case tok::l_square:
-  tryToParseLambda();
+  if (S

[PATCH] D74692: [clang-tidy][bugprone-use-after-move] Warn on std::move for consts

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp:383
   SourceLocation MoveLoc = MovingCall->getExprLoc();
+  const bool MoveArgIsConst = MoveArg->getType().isConstQualified();
 

Drop the top-level `const` and make it an integer type so that you don't need 
to use the `?:` below. The implicit conversion will do the right thing in terms 
of the integer value.



Comment at: clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp:391-392
+if (Lambda) {
+  for (auto Capture = Lambda->capture_begin(), End = Lambda->capture_end();
+   Capture != End; ++Capture) {
+if (MoveArg->getDecl() == Capture->getCapturedVar()) {

Range-based for loop over `captures()`?



Comment at: clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp:394
+if (MoveArg->getDecl() == Capture->getCapturedVar()) {
+  const bool IsExplicitCapture = Capture->isExplicit();
+  Check->diag(

Same suggestions here as above.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp:342
+  // CHECK-NOTES: [[@LINE-5]]:20: note: variable 'a' implicitly captured 
const here
 };
   }

One more test case to try out (it might be a FIXME because I imagine this 
requires flow control to get right):
```
A a;
std::move(a);

auto lambda = [=] {
  a.foo(); // Use of 'a' after it was moved
}
```


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

https://reviews.llvm.org/D74692



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

[PATCH] D74735: [analyzer] Add support for CXXInheritedCtorInitExpr.

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

@steakhal

There is a fix for the crash: https://reviews.llvm.org/D75678
Let's hope that patch get's in soon.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74735



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

[clang] 33bb32b - [Sema] Reword -Wrange-loop-analysis warning messages

[Aaron Puchert via cfe-commits]

Author: Aaron Puchert
Date: 2020-03-06T14:57:01+01:00
New Revision: 33bb32bbc674a16973c59c9a6dc119387843a0f0

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

LOG: [Sema] Reword -Wrange-loop-analysis warning messages

Summary:
The messages for two of the warnings are misleading:
* warn_for_range_const_reference_copy suggests that the initialization
  of the loop variable results in a copy. But that's not always true,
  we just know that some conversion happens, potentially invoking a
  constructor or conversion operator. The constructor might copy, as in
  the example that lead to this message [1], but it might also not.
  However, the constructed object is bound to a reference, which is
  potentially misleading, so we rewrite the message to emphasize that.
  We also make sure that we print the reference type into the warning
  message to clarify that this warning only appears when operator*
  returns a reference.
* warn_for_range_variable_always_copy suggests that a reference type
  loop variable initialized from a temporary "is always a copy". But
  we don't know this, the range might just return temporary objects
  which aren't copies of anything. (Assuming RVO a copy constructor
  might never have been called.)

The message for warn_for_range_copy is a bit repetitive: the type of a
VarDecl and its initialization Expr are the same up to cv-qualifiers,
because Sema will insert implicit casts or constructor calls to make
them match.

[1] https://bugs.llvm.org/show_bug.cgi?id=32823

Reviewers: aaron.ballman, Mordante, rtrieu

Reviewed By: aaron.ballman

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

Added: 


Modified: 
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaStmt.cpp
clang/test/SemaCXX/warn-range-loop-analysis-trivially-copyable.cpp
clang/test/SemaCXX/warn-range-loop-analysis.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index e6155d5d0e10..cb3dd24a44cf 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -2424,20 +2424,19 @@ def note_for_range_invalid_iterator : Note <
   "in implicit call to 'operator%select{!=|*|++}0' for iterator of type %1">;
 def note_for_range_begin_end : Note<
   "selected '%select{begin|end}0' %select{function|template }1%2 with iterator 
type %3">;
-def warn_for_range_const_reference_copy : Warning<
+def warn_for_range_const_ref_binds_temp_built_from_ref : Warning<
   "loop variable %0 "
-  "%
diff {has type $ but is initialized with type $"
-  "|is initialized with a value of a 
diff erent type}1,2 resulting in a copy">,
+  "%
diff {of type $ binds to a temporary constructed from type $"
+  "|binds to a temporary constructed from a 
diff erent type}1,2">,
   InGroup, DefaultIgnore;
 def note_use_type_or_non_reference : Note<
-  "use non-reference type %0 to keep the copy or type %1 to prevent copying">;
-def warn_for_range_variable_always_copy : Warning<
-  "loop variable %0 is always a copy because the range of type %1 does not "
-  "return a reference">,
+  "use non-reference type %0 to make construction explicit or type %1 to 
prevent copying">;
+def warn_for_range_ref_binds_ret_temp : Warning<
+  "loop variable %0 binds to a temporary value produced by a range of type 
%1">,
   InGroup, DefaultIgnore;
 def note_use_non_reference_type : Note<"use non-reference type %0">;
 def warn_for_range_copy : Warning<
-  "loop variable %0 of type %1 creates a copy from type %2">,
+  "loop variable %0 creates a copy from type %1">,
   InGroup, DefaultIgnore;
 def note_use_reference_type : Note<"use reference type %0 to prevent copying">;
 def err_objc_for_range_init_stmt : Error<

diff  --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp
index ff6481006280..2104add400e0 100644
--- a/clang/lib/Sema/SemaStmt.cpp
+++ b/clang/lib/Sema/SemaStmt.cpp
@@ -2741,22 +2741,24 @@ static void 
DiagnoseForRangeReferenceVariableCopies(Sema &SemaRef,
 E = E->IgnoreImpCasts();
   }
 
-  bool ReturnsReference = false;
+  QualType ReferenceReturnType;
   if (isa(E)) {
-ReturnsReference = true;
+ReferenceReturnType = SemaRef.Context.getLValueReferenceType(E->getType());
   } else {
 const CXXOperatorCallExpr *Call = cast(E);
 const FunctionDecl *FD = Call->getDirectCallee();
 QualType ReturnType = FD->getReturnType();
-ReturnsReference = ReturnType->isReferenceType();
+if (ReturnType->isReferenceType())
+  ReferenceReturnType = ReturnType;
   }
 
-  if (ReturnsReference) {
+  if (!ReferenceReturnType.isNull()) {
 // Loop variable creates a temporary.  Suggest either to go with
 // non-reference loop variable to indicate a 

[PATCH] D75740: [ASTImporter] Corrected import of repeated friend declarations.

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

balazske created this revision.
Herald added subscribers: cfe-commits, martong, teemperor, gamesh411, 
Szelethus, dkrupp.
Herald added a reviewer: a.sidorin.
Herald added a reviewer: shafik.
Herald added a project: clang.

Import declarations in correct order if a class contains
multiple redundant friend (type or decl) declarations.
If the order is incorrect this could cause false structural
equivalences and wrong declaration chains after import.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D75740

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

Index: clang/unittests/AST/StructuralEquivalenceTest.cpp
===
--- clang/unittests/AST/StructuralEquivalenceTest.cpp
+++ clang/unittests/AST/StructuralEquivalenceTest.cpp
@@ -797,6 +797,20 @@
   EXPECT_FALSE(testStructuralMatch(t));
 }
 
+TEST_F(StructuralEquivalenceRecordTest, SameFriendMultipleTimes) {
+  auto t = makeNamedDecls("struct foo{ friend class X; };",
+  "struct foo{ friend class X; friend class X; };",
+  Lang_CXX);
+  EXPECT_FALSE(testStructuralMatch(t));
+}
+
+TEST_F(StructuralEquivalenceRecordTest, SameFriendsDifferentOrder) {
+  auto t = makeNamedDecls("struct foo{ friend class X; friend class Y; };",
+  "struct foo{ friend class Y; friend class X; };",
+  Lang_CXX);
+  EXPECT_FALSE(testStructuralMatch(t));
+}
+
 struct StructuralEquivalenceLambdaTest : StructuralEquivalenceTest {};
 
 TEST_F(StructuralEquivalenceLambdaTest, LambdaClassesWithDifferentMethods) {
Index: clang/unittests/AST/ASTImporterTest.cpp
===
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -4005,6 +4005,56 @@
   EXPECT_EQ(ImportedFwd, ImportedDef->getPreviousDecl());
 }
 
+TEST_P(ImportFriendClasses, ImportOfRepeatedFriendType) {
+  auto Code =
+  R"(
+  class Container {
+friend class X;
+friend class X;
+  };
+  )";
+  Decl *ToTu = getToTuDecl(Code, Lang_CXX);
+  Decl *FromTu = getTuDecl(Code, Lang_CXX, "from.cc");
+
+  auto *ToFriend1 = FirstDeclMatcher().match(ToTu, friendDecl());
+  auto *ToFriend2 = LastDeclMatcher().match(ToTu, friendDecl());
+  auto *FromFriend1 =
+  FirstDeclMatcher().match(FromTu, friendDecl());
+  auto *FromFriend2 = LastDeclMatcher().match(FromTu, friendDecl());
+
+  FriendDecl *ToImportedFriend1 = Import(FromFriend1, Lang_CXX);
+  FriendDecl *ToImportedFriend2 = Import(FromFriend2, Lang_CXX);
+
+  EXPECT_NE(ToImportedFriend1, ToImportedFriend2);
+  EXPECT_EQ(ToFriend1, ToImportedFriend1);
+  EXPECT_EQ(ToFriend2, ToImportedFriend2);
+}
+
+TEST_P(ImportFriendClasses, ImportOfRepeatedFriendDecl) {
+  auto Code =
+  R"(
+  class Container {
+friend void f();
+friend void f();
+  };
+  )";
+  Decl *ToTu = getToTuDecl(Code, Lang_CXX);
+  Decl *FromTu = getTuDecl(Code, Lang_CXX, "from.cc");
+
+  auto *ToFriend1 = FirstDeclMatcher().match(ToTu, friendDecl());
+  auto *ToFriend2 = LastDeclMatcher().match(ToTu, friendDecl());
+  auto *FromFriend1 =
+  FirstDeclMatcher().match(FromTu, friendDecl());
+  auto *FromFriend2 = LastDeclMatcher().match(FromTu, friendDecl());
+
+  FriendDecl *ToImportedFriend1 = Import(FromFriend1, Lang_CXX);
+  FriendDecl *ToImportedFriend2 = Import(FromFriend2, Lang_CXX);
+
+  EXPECT_NE(ToImportedFriend1, ToImportedFriend2);
+  EXPECT_EQ(ToFriend1, ToImportedFriend1);
+  EXPECT_EQ(ToFriend2, ToImportedFriend2);
+}
+
 TEST_P(ASTImporterOptionSpecificTestBase, FriendFunInClassTemplate) {
   auto *Code = R"(
   template 
Index: clang/lib/AST/ASTImporter.cpp
===
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -3632,6 +3632,38 @@
   return ToIndirectField;
 }
 
+static std::tuple
+getFriendCountAndPosition(FriendDecl *FD) {
+  unsigned int FriendCount = 0;
+  llvm::Optional FriendPosition;
+  auto *RD = cast(FD->getLexicalDeclContext());
+  if (FD->getFriendType()) {
+QualType TypeOfFriend = FD->getFriendType()->getType().getCanonicalType();
+for (FriendDecl *FoundFriend : RD->friends()) {
+  if (FoundFriend == FD) {
+FriendPosition = FriendCount;
+++FriendCount;
+  } else if (FoundFriend->getFriendType() &&
+ FoundFriend->getFriendType()->getType().getCanonicalType() ==
+ TypeOfFriend)
+++FriendCount;
+}
+  } else {
+const Decl *CanDeclOfFriend = FD->getFriendDecl()->getCanonicalDecl();
+for (FriendDecl *FoundFriend : RD->friends()) {
+  if (FoundFriend == FD) {
+FriendPosition = FriendCount;
+++FriendCount;
+  } else if (FoundFriend->getFriendDecl() &&
+ FoundFriend->getFriendDecl()->getCa

[PATCH] D75479: [clangd] go-to-def on names in comments etc that are used nearby.

[Sam McCall via Phabricator via cfe-commits]

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

> I'll make a proposal on https://github.com/clangd/clangd/issues/241 for a bit 
> more visibility.

OK, I wrote a bunch of stuff there (twice, laptop crashed, grr...)

I'll put this briefly on hold to see what you/others think there - e.g. the 
real-identifier bit is best decided with the final plan in mind.




Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:871
+TEST(LocateSymbol, NearbyIdentifier) {
+  const char *Tests[] = {
+R"cpp(

nridge wrote:
> Tangentially related, but what do you think about the issue I raised in [this 
> mailing list 
> thread](https://lists.llvm.org/pipermail/clangd-dev/2019-November/000579.html)
>  about testcase style?
Posted a reply on the thread. TL;DR:
 - I hate that too
 - the style brings several benefits
 - I don't know what would be better that's available
 - but we could probably build something


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75479



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

[PATCH] D75739: [clangd][vscode] Enable dot-to-arrow fixes in clangd completion.

[Haojian Wu via Phabricator via cfe-commits]

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

The previous issue is that the item was filtered out by vscode, because
the prefix (which contains ".") are not matched against the filterText.

This patch works around it by adjusting the item filterText, inspired by
https://reviews.llvm.org/D75623.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D75739

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


Index: clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts
===
--- clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts
+++ clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts
@@ -65,6 +65,14 @@
   }
 }
 
+class EnableEditsNearCursorFeature implements vscodelc.StaticFeature {
+  initialize() { }
+  fillClientCapabilities(capabilities: vscodelc.ClientCapabilities): void {
+const extendedCompletionCapabilities: any = 
capabilities.textDocument.completion;
+extendedCompletionCapabilities.editsNearCursor = true;
+  }
+}
+
 /**
  *  this method is called when your extension is activate
  *  your extension is activated the very first time the command is executed
@@ -112,12 +120,27 @@
 // See https://github.com/microsoft/language-server-protocol/issues/898
 middleware: {
   provideCompletionItem: async (document, position, context, token, 
next) => {
-// Get the incomplete identifier before the cursor.
-let word = document.getWordRangeAtPosition(position);
-let prefix = word && document.getText(new vscode.Range(word.start, 
position));
-
 let list = await next(document, position, context, token);
 let items = (Array.isArray(list) ? list : list.items).map(item => {
+  // ^ is the position.
+  //  Example 1: std::u_p^
+  //  Example 2: a.f^
+  //
+  // When run code completion in VSCode, what happens:
+  // - clangd returns a completion item with range [[u_p]] and 
filterText "unique_ptr"
+  // - vscode extracts the word (prefix we called) from the range 
, which is "u_p"
+  // - vscode fuzzy-matches the filterText "unique_ptr" against 
the prefix "u_p"
+  //
+  // For example 2, the completion item range is [[.f]], filterText
+  // is "foo", insertText is "->foo". ".f" is the prefix used to do
+  // fuzzymatch, and the filterText "foo" is not matched, therefore
+  // the item is filtered out and not shown in the UI.
+  //
+  // Adding the prefix to the filterText will prevent VSCode 
filtering
+  // it out.
+
+  // Gets the prefix used by vscode when doing fuzzymatch.
+  let prefix = document.getText(new vscode.Range(item.range.start, 
position))
   if (prefix)
 item.filterText = prefix + "_" + item.filterText;
   return item;
@@ -137,6 +160,7 @@
 vscode.Disposable.from(semanticHighlightingFeature));
 clangdClient.registerFeature(semanticHighlightingFeature);
   }
+  clangdClient.registerFeature(new EnableEditsNearCursorFeature);
   console.log('Clang Language Server is now active!');
   context.subscriptions.push(clangdClient.start());
   context.subscriptions.push(vscode.commands.registerCommand(


Index: clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts
===
--- clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts
+++ clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts
@@ -65,6 +65,14 @@
   }
 }
 
+class EnableEditsNearCursorFeature implements vscodelc.StaticFeature {
+  initialize() { }
+  fillClientCapabilities(capabilities: vscodelc.ClientCapabilities): void {
+const extendedCompletionCapabilities: any = capabilities.textDocument.completion;
+extendedCompletionCapabilities.editsNearCursor = true;
+  }
+}
+
 /**
  *  this method is called when your extension is activate
  *  your extension is activated the very first time the command is executed
@@ -112,12 +120,27 @@
 // See https://github.com/microsoft/language-server-protocol/issues/898
 middleware: {
   provideCompletionItem: async (document, position, context, token, next) => {
-// Get the incomplete identifier before the cursor.
-let word = document.getWordRangeAtPosition(position);
-let prefix = word && document.getText(new vscode.Range(word.start, position));
-
 let list = await next(document, position, context, token);
 let items = (Array.isArray(list) ? list : list.items).map(item => {
+  // ^ is th

[PATCH] D75698: [analyzer][WIP] Suppress bug reports where a tracked expression's latest value change was a result of an invalidation

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

> While invalidation is a fundamental part of static analysis, it is 
> unfortunately not an under-approximation (resulting in fewer but more precise 
> paths of execution)

+1 for saying this out :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75698



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

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

[Gabor Marton via Phabricator via cfe-commits]

martong added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:92-125
+class MakeRetVal {
+  const CallExpr *CE = nullptr;
+  std::unique_ptr RetVal;
+  SymbolRef RetSym;
+
+public:
+  MakeRetVal(const CallEvent &Call, CheckerContext &C)

Szelethus wrote:
> balazske wrote:
> > Szelethus wrote:
> > > balazske wrote:
> > > > Szelethus wrote:
> > > > > Do you have other patches that really crave the need for this class? 
> > > > > Why isn't `CallEvent::getReturnValue` sufficient? This is a 
> > > > > legitimate question, I really don't know. :)
> > > > This is an "interesting" solution for the problem that there is need 
> > > > for a function with 3 return values. The constructor performs the task 
> > > > of the function: Create a conjured value (and get the various objects 
> > > > for it). The output values are RetVal and RetSym, and the success 
> > > > state, and the call expr that is computed here anyway. It could be 
> > > > computed independently but if the value was retrieved once it is better 
> > > > to store it for later use. (I did not check how costly that operation 
> > > > is.)
> > > > 
> > > > I had some experience that using only `getReturnValue` and make 
> > > > constraints on that does not work as intended, and the reason can be 
> > > > that we need to bind a value for the call expr otherwise it is an 
> > > > unknown (undefined?) value (and not the conjured symbol)?
> > > I suspect that `getReturnValue` might only work in `postCall`, but I'm 
> > > not sure.
> > > 
> > > I think instead of this class, a function returning a `std::tuple` would 
> > > be nicer, with `std::tie` on the call site. You seem to use all 3 returns 
> > > values in the functions that instantiate `MakeRetVal` anyways :).
> > > 
> > > In `StdLibraryFunctionsChecker`'s `evalCall`, the return value is 
> > > [[https://github.com/llvm/llvm-project/blob/master/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp#L403|explicitly
> > >  constructed]], and further constraints on it are only imposed in 
> > > `postCall`. I wonder why that is. @martong, any idea why we don't `apply` 
> > > the constraints for pure functions in `evalCall?`
> > The return value case is not as simple because the `DefinedSVal` has no 
> > default constructor, but it is not as bad to return only the `RetVal` and 
> > have a `CE` argument.
> I like the current solution very much!
> In StdLibraryFunctionsChecker's evalCall, the return value is explicitly 
> constructed, and further constraints on it are only imposed in postCall. I 
> wonder why that is. @martong, any idea why we don't apply the constraints for 
> pure functions in evalCall?

We could apply them in evalCall technically.
I think the reason why we don't do that is the matter of implementation, and 
more importantly this way we are consequent with the traditional Hoare logic: 
{Pre}C{Post} as {Post} is done in postCall.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75356



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

[PATCH] D75613: [Sema] Reword -Wrange-loop-analysis warning messages

[Aaron Puchert via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG33bb32bbc674: [Sema] Reword -Wrange-loop-analysis warning 
messages (authored by aaronpuchert).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75613

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaStmt.cpp
  clang/test/SemaCXX/warn-range-loop-analysis-trivially-copyable.cpp
  clang/test/SemaCXX/warn-range-loop-analysis.cpp

Index: clang/test/SemaCXX/warn-range-loop-analysis.cpp
===
--- clang/test/SemaCXX/warn-range-loop-analysis.cpp
+++ clang/test/SemaCXX/warn-range-loop-analysis.cpp
@@ -71,17 +71,17 @@
   Container bar_container;
 
   for (const int &x : int_non_ref_container) {}
-  // expected-warning@-1 {{loop variable 'x' is always a copy because the range of type 'Container' does not return a reference}}
+  // expected-warning@-1 {{loop variable 'x' binds to a temporary value produced by a range of type 'Container'}}
   // expected-note@-2 {{use non-reference type 'int'}}
   // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:18-[[@LINE-3]]:19}:""
 
   for (const double &x : int_container) {}
-  // expected-warning@-1 {{loop variable 'x' has type 'const double &' but is initialized with type 'int' resulting in a copy}}
-  // expected-note@-2 {{use non-reference type 'double' to keep the copy or type 'const int &' to prevent copying}}
+  // expected-warning@-1 {{loop variable 'x' of type 'const double &' binds to a temporary constructed from type 'int &'}}
+  // expected-note@-2 {{use non-reference type 'double' to make construction explicit or type 'const int &' to prevent copying}}
   // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:21-[[@LINE-3]]:22}:""
 
   for (const Bar x : bar_container) {}
-  // expected-warning@-1 {{loop variable 'x' of type 'const Bar' creates a copy from type 'const Bar'}}
+  // expected-warning@-1 {{loop variable 'x' creates a copy from type 'const Bar'}}
   // expected-note@-2 {{use reference type 'const Bar &' to prevent copying}}
   // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:18-[[@LINE-3]]:18}:"&"
 }
@@ -92,7 +92,7 @@
   for (const int &&x : A) {}
   // No warning, rvalue-reference to the temporary
   for (const int &x : A) {}
-  // expected-warning@-1 {{always a copy}}
+  // expected-warning@-1 {{binds to a temporary value produced by a range}}
   // expected-note@-2 {{'int'}}
   // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:18-[[@LINE-3]]:19}:""
   for (const int x : A) {}
@@ -107,7 +107,7 @@
   for (const double &&x : A) {}
   // No warning, rvalue-reference to the temporary
   for (const double &x : A) {}
-  // expected-warning@-1 {{always a copy}}
+  // expected-warning@-1 {{binds to a temporary value produced by a range}}
   // expected-note@-2 {{'double'}}
   // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:21-[[@LINE-3]]:22}:""
   for (const double x : A) {}
@@ -122,7 +122,7 @@
   for (const Bar &&x : A) {}
   // No warning, rvalue-reference to the temporary
   for (const Bar &x : A) {}
-  // expected-warning@-1 {{always a copy}}
+  // expected-warning@-1 {{binds to a temporary value produced by a range}}
   // expected-note@-2 {{'Bar'}}
   // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:18-[[@LINE-3]]:19}:""
   for (const Bar x : A) {}
@@ -152,16 +152,16 @@
   // No warning
 
   for (const double &&x : B) {}
-  // expected-warning@-1 {{resulting in a copy}}
+  // expected-warning@-1 {{binds to a temporary constructed from}}
   // expected-note-re@-2 {{'double'{{.*}}'const int &'}}
   // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:21-[[@LINE-3]]:23}:""
   for (const double &x : B) {}
-  // expected-warning@-1 {{resulting in a copy}}
+  // expected-warning@-1 {{binds to a temporary constructed from}}
   // expected-note-re@-2 {{'double'{{.*}}'const int &'}}
   // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:21-[[@LINE-3]]:22}:""
   for (const double x : B) {}
   for (double &&x : B) {}
-  // expected-warning@-1 {{resulting in a copy}}
+  // expected-warning@-1 {{binds to a temporary constructed from}}
   // expected-note-re@-2 {{'double'{{.*}}'const int &'}}
   // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:15-[[@LINE-3]]:17}:""
   //for (double &x : B) {}
@@ -170,16 +170,16 @@
   // No warning
 
   for (const Bar &&x : B) {}
-  // expected-warning@-1 {{resulting in a copy}}
+  // expected-warning@-1 {{binds to a temporary constructed from}}
   // expected-note@-2 {{'Bar'}}
   // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:18-[[@LINE-3]]:20}:""
   for (const Bar &x : B) {}
-  // expected-warning@-1 {{resulting in a copy}}
+  // expected-warning@-1 {{binds to a temporary constructed from}}
   // expected-note@-2 {{'Bar'}}
   // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:18-[[@LINE-3]]:19}:""
   for (const Bar x : B) {}
   for (Bar &&x : B) {}
-  // expected-warning@-1 {{resulting in a copy}}
+  // expected-warning@-1 {{binds to a temporary constructed from}}
   // expected-note@-2 {{'Bar

[PATCH] D75731: [clang-format] C# does not indent braced initializers as continuations

[Jonathan B Coe via Phabricator via cfe-commits]

jbcoe updated this revision to Diff 248719.
jbcoe added a comment.

Remove comma to tidy up ugly format test and actually test what is needed.


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

https://reviews.llvm.org/D75731

Files:
  clang/lib/Format/FormatToken.h
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/FormatTestCSharp.cpp


Index: clang/unittests/Format/FormatTestCSharp.cpp
===
--- clang/unittests/Format/FormatTestCSharp.cpp
+++ clang/unittests/Format/FormatTestCSharp.cpp
@@ -527,31 +527,28 @@
 
   verifyFormat(R"(//
 Shape[] shapes = new[] {
-new Circle {
-Radius = 2.7281,
-Colour = Colours.Red,
-},
-new Square {
-Side = 101.1,
-Colour = Colours.Yellow,
-},
+  new Circle {
+Radius = 2.7281,
+Colour = Colours.Red,
+  },
+  new Square {
+Side = 101.1,
+Colour = Colours.Yellow,
+  },
 };)",
Style);
 
   // Omitted final `,`s will change the formatting.
   verifyFormat(R"(//
 Shape[] shapes = new[] { new Circle { Radius = 2.7281, Colour = Colours.Red },
- new Square {
- Side = 101.1,
- Colour = Colours.Yellow,
- } };)",
+ new Square { Side = 101.1, Colour = Colours.Yellow } 
};)",
Style);
 
   // Lambdas can be supplied as initialiser arguments.
   verifyFormat(R"(//
 private Transformer _transformer = new X.Y {
-Filler = (Shape shape) => { return new Transform.Fill(shape, RED); },
-Scaler = (Shape shape) => { return new Transform.Resize(shape, 0.1); },
+  Filler = (Shape shape) => { return new Transform.Fill(shape, RED); },
+  Scaler = (Shape shape) => { return new Transform.Resize(shape, 0.1); },
 };)",
Style);
 }
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1677,7 +1677,10 @@
   }
   break;
 case tok::l_square:
-  tryToParseLambda();
+  if (Style.isCSharp())
+parseSquare();
+  else
+tryToParseLambda();
   break;
 case tok::l_paren:
   parseParens();
Index: clang/lib/Format/FormatToken.h
===
--- clang/lib/Format/FormatToken.h
+++ clang/lib/Format/FormatToken.h
@@ -509,6 +509,9 @@
   /// Returns \c true if this tokens starts a block-type list, i.e. a
   /// list that should be indented with a block indent.
   bool opensBlockOrBlockTypeList(const FormatStyle &Style) const {
+// C# Does not indent object initialisers as continuations.
+if (is(tok::l_brace) && BlockKind == BK_BracedInit && Style.isCSharp())
+  return true;
 if (is(TT_TemplateString) && opensScope())
   return true;
 return is(TT_ArrayInitializerLSquare) || is(TT_ProtoExtensionLSquare) ||


Index: clang/unittests/Format/FormatTestCSharp.cpp
===
--- clang/unittests/Format/FormatTestCSharp.cpp
+++ clang/unittests/Format/FormatTestCSharp.cpp
@@ -527,31 +527,28 @@
 
   verifyFormat(R"(//
 Shape[] shapes = new[] {
-new Circle {
-Radius = 2.7281,
-Colour = Colours.Red,
-},
-new Square {
-Side = 101.1,
-Colour = Colours.Yellow,
-},
+  new Circle {
+Radius = 2.7281,
+Colour = Colours.Red,
+  },
+  new Square {
+Side = 101.1,
+Colour = Colours.Yellow,
+  },
 };)",
Style);
 
   // Omitted final `,`s will change the formatting.
   verifyFormat(R"(//
 Shape[] shapes = new[] { new Circle { Radius = 2.7281, Colour = Colours.Red },
- new Square {
- Side = 101.1,
- Colour = Colours.Yellow,
- } };)",
+ new Square { Side = 101.1, Colour = Colours.Yellow } };)",
Style);
 
   // Lambdas can be supplied as initialiser arguments.
   verifyFormat(R"(//
 private Transformer _transformer = new X.Y {
-Filler = (Shape shape) => { return new Transform.Fill(shape, RED); },
-Scaler = (Shape shape) => { return new Transform.Resize(shape, 0.1); },
+  Filler = (Shape shape) => { return new Transform.Fill(shape, RED); },
+  Scaler = (Shape shape) => { return new Transform.Resize(shape, 0.1); },
 };)",
Style);
 }
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1677,7 +1677,10 @@
   }
   break;
 case tok::l_square:
-  tryToParseLambda();
+  if (Style.isCSharp())
+parseSquare();
+  else
+tryToParseLambda();
   break;
   

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

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

Szelethus added a comment.

In D75356#1909193 , @balazske wrote:

> To avoid problems I created a new version of this diff too that follows after 
> the other new ones:
>  https://reviews.llvm.org/D75682
>  (Adding a new diff can make inline comment positions even more inexact 
> specially if the diff has many differences from an older one?)


I'd strongly prefer to finish this and move on after that -- we have the 
discussion here, and a great looking patch with only a few things to address.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75356



___
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.

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

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

Let's halt this for now -- we have so many revisions going on seemingly doing 
the same thing, its becoming really confusing. Please finish D75356 
 before proceeding.


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] D75163: [analyzer][StreamChecker] Adding precall and refactoring.

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

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

Okay, very well, then lets abandon this patch. Please keep in mind that 
phabricator can mark whether a revision was mentioned somewhere else if you 
only copy the revision ID, not the entire link: D75163 
 vs. https://reviews.llvm.org/D75163.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75163



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

[PATCH] D75612: [Analyzer][StreamChecker] Adding PreCall and refactoring (NFC).

[Gabor Marton via Phabricator via cfe-commits]

martong added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:321
+ProgramStateRef
+StreamChecker::ensureStreamNonNull(SVal StreamVal, CheckerContext &C,
+   ProgramStateRef State) const {

Just a quick note here. We are going to be able to do similar arg checks but in 
a generic way, once apiModeling.StdLibraryFunctionsChecker is clever enough. 
https://reviews.llvm.org/D75063
Sooner or later we will have a summary for these file handling functions, and 
that time the non-null argument constraint violation will be reported by either 
of the two checkers. Then we would be able to remove the nonnull check from 
here if we make StdLibraryFunctionsChecker a dependency.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75612



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

[PATCH] D75614: [Analyzer][StreamChecker] Check for opened stream before operations.

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

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

Amazing, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75614



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

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

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

Szelethus added a comment.

Could you please fix up the dependencies of this revision?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75356



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

[PATCH] D72326: [clang-format] Rebased on master: Add option to specify explicit config file

[Thibault North via Phabricator via cfe-commits]

tnorth added a comment.

Thanks @MyDeveloperDay . I guess more approvals are needed at this point?


Repository:
  rC Clang

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

https://reviews.llvm.org/D72326



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

[PATCH] D74361: [Clang] Undef attribute for global variables

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/Sema/SemaDecl.cpp:2718
+  // attribute then there are two different definitions.
+  // In C++, prefer the standard diagnostics
+  if (!S.getLangOpts().CPlusPlus) {

Missing a full stop at the end of the sentence. The comment can probably be 
re-flowed with the preceding sentence too.



Comment at: clang/lib/Sema/SemaDecl.cpp:11949
 
+  // The LoaderUninitialized attribute acts as a definition (of undef)
+  if (VDecl->hasAttr()) {

Missing full stop.



Comment at: clang/lib/Sema/SemaDecl.cpp:12377
+  }
+  if (Var->getStorageClass() == SC_Extern) {
+Diag(Var->getLocation(), diag::err_loader_uninitialized_extern);

Should this either be calling `VarDecl::hasExternalStorage()` or looking at the 
linkage of the variable, rather than at the storage class written in the source?



Comment at: clang/test/CodeGen/attr-loader-uninitialized.c:4
+// CHECK: @tentative_attr_first = weak global i32 undef, align 4
+int tentative_attr_first __attribute__((loader_uninitialized));
+int tentative_attr_first;

I thought `err_loader_uninitialized_extern` says that the variable cannot have 
external linkage?



Comment at: clang/test/CodeGenCXX/attr-loader-uninitialized.cpp:4
+// CHECK: @defn = global i32 undef
+int defn  [[clang::loader_uninitialized]];
+

I thought `err_loader_uninitialized_extern` says that the variable cannot have 
external linkage?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74361



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

[PATCH] D70265: [clang-tidy] Add CppCoreGuidelines I.2 "Avoid non-const global variables" check

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidNonConstGlobalVariablesCheck.cpp:55-71
+  if (const auto *Reference =
+  Result.Nodes.getNodeAs("reference_to_non-const")) {
+diag(Reference->getLocation(),
+ "variable %0 provides global access to non-const type, consider "
+ "making the referenced data const")
+<< Reference; // FIXME: Add fix-it hint to Reference
+return;

I think these cases should be combined to avoid duplication.
```
if (const auto *VD = Result.Nodes.getNodeAs("whatever")) {
  diag(VD->getLocation(), "variable %0 provides global access to a non-const 
object; considering making the %select{referenced|pointed-to}1 data 'const'") 
<< VD << VD->getType()->isReferenceType();
  return;
}
```
the matcher needs to be changed to bind to the same id for both cases.

Note, this rewords the diagnostic slightly to avoid type/object confusion (the 
variable provides access to an object of a non-const type, not to the type 
itself).



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-avoid-non-const-global-variables.rst:20
+
+  char * c_ptr1 = &c;  // Warns!
+  char *const c_const_ptr = &c;  // Warns!

This isn't legal code.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-avoid-non-const-global-variables.rst:33
+char h = 0;
+  }
+

This isn't legal code either.

You may want to run the example through the compiler to catch these sort of 
things.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70265



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

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

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

balazske added a comment.

I have "mirrored" all 3 changes in this stack to the new series in D75682 
. Probably it is possible to reuse these 
revisions instead but I do not know if it will not confuse phabricator somehow 
(and how  phabricator behaves in such "tricky" cases, there is not a usable 
documentation for it). The D75682  is the one 
that should be used now, it is the same change as this one plus the ferror and 
feof functions and tests. The new part with `ferror` and `feof` can be done in 
a new change but these belong logically into this change to make the error 
handling complete and testable (this change in current form is not good for 
tests).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75356



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

[PATCH] D75163: [analyzer][StreamChecker] Adding precall and refactoring.

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

balazske abandoned this revision.
balazske added a comment.

The patch was split into D75612  and D75614 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75163



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

[PATCH] D75739: [clangd][vscode] Enable dot-to-arrow fixes in clangd completion.

[Sam McCall via Phabricator via cfe-commits]

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

So the core of this is a better version of my patch, thanks!
It does work around the problem with dot-to-arrow, which is useful and we 
should definitely enable that.

However I actually think the dot-to-arrow problem is our bug at this point. LSP 
is vague but the discussion on language-server-protocol#898 very sensibly links 
filterText to textEdit.range. If we're sending `filterText = "foo", 
textEdit.range = ".f"` then I don't think that *is* a match.
We get into this mess because we merge two edits together, I think to avoid 
LSP's vague prohibition about edits related to the cursor. My vote would be:

1. check this patch in, but without claiming it as the correct and carefully 
architected fix for dot-to-arrow
2. try turning off range-merging, and test it in a couple of clients (including 
vscode *without* this patch)
3. if it works, make that change in clangd




Comment at: clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts:117
 // fuzzymatch score for every item.
 // The sortText (which reflects clangd ranking) breaks the tie.
 //

"This also prevents VSCode from filtering out any results due to differences in 
how fuzzy filtering is applied."



Comment at: clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts:125
 let items = (Array.isArray(list) ? list : list.items).map(item => {
+  // ^ is the position.
+  //  Example 1: std::u_p^

While the detailed examples/debugging instructions are nice for me, I don't 
think keeping them in the code here is worthwhile - I'd suggest just adding the 
one line to the comment above and removing this block.

For the dot-to-arrow example specifically it might be worth filing a clangd bug 
and linking to it in the comment above.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75739



___
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 created this revision.
pbaran added a reviewer: alexfh.
pbaran added a project: clang-tools-extra.
Herald added subscribers: kbarton, xazax.hun, nemanjai.
Herald added a project: clang.
pbaran updated this revision to Diff 248729.
pbaran added a comment.
Herald added a subscriber: wuzish.

Removed unintendedly added comment


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


https://reviews.llvm.org/D75745

Files:
  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
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-special-member-functions-allow-missing-move-when-copy-is-deleted.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-special-member-functions-allow-missing-move-when-copy-is-deleted.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-special-member-functions-allow-missing-move-when-copy-is-deleted.cpp
@@ -0,0 +1,49 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-special-member-functions %t -- -config="{CheckOptions: [{key: cppcoreguidelines-special-member-functions.AllowMissingMoveFunctionsWhenCopyIsDeleted, value: 1}]}" --
+
+class DefinesEverything {
+  DefinesEverything(const DefinesEverything &);
+  DefinesEverything(DefinesEverything &&);
+  DefinesEverything &operator=(const DefinesEverything &);
+  DefinesEverything &operator=(DefinesEverything &&);
+  ~DefinesEverything();
+};
+
+class DefinesNothing {
+};
+
+class DeletedCopyCtorAndOperator {
+  ~DeletedCopyCtorAndOperator() = default;
+  DeletedCopyCtorAndOperator(const DeletedCopyCtorAndOperator &) = delete;
+  DeletedCopyCtorAndOperator &operator=(const DeletedCopyCtorAndOperator &) = delete;
+};
+
+// CHECK-MESSAGES: [[@LINE+1]]:7: warning: class 'DefaultedCopyCtorAndOperator' defines a default destructor, a copy constructor and a copy assignment operator but does not define a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]
+class DefaultedCopyCtorAndOperator {
+  ~DefaultedCopyCtorAndOperator() = default;
+  DefaultedCopyCtorAndOperator(const DefaultedCopyCtorAndOperator &) = default;
+  DefaultedCopyCtorAndOperator &operator=(const DefaultedCopyCtorAndOperator &) = default;
+};
+
+// CHECK-MESSAGES: [[@LINE+1]]:7: warning: class 'DefinedCopyCtorAndOperator' defines a default destructor, a copy constructor and a copy assignment operator but does not define a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]
+class DefinedCopyCtorAndOperator {
+  ~DefinedCopyCtorAndOperator() = default;
+  DefinedCopyCtorAndOperator(const DefinedCopyCtorAndOperator &);
+  DefinedCopyCtorAndOperator &operator=(const DefinedCopyCtorAndOperator &);
+};
+
+// CHECK-MESSAGES: [[@LINE+1]]:7: warning: class 'MissingCopyCtor' defines a default destructor and a copy assignment operator but does not define a copy constructor, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]
+class MissingCopyCtor {
+  ~MissingCopyCtor() = default;
+  MissingCopyCtor &operator=(const MissingCopyCtor &) = delete;
+};
+
+// CHECK-MESSAGES: [[@LINE+1]]:7:  warning: class 'MissingCopyOperator' defines a default destructor and a copy constructor but does not define a copy assignment operator, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]
+class MissingCopyOperator {
+  ~MissingCopyOperator() = default;
+  MissingCopyOperator(const MissingCopyOperator &) = delete;
+};
+
+// CHECK-MESSAGES: [[@LINE+1]]:7:  warning: class 'MissingAll' defines a default destructor but does not define a copy constructor, a copy assignment operator, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]
+class MissingAll {
+  ~MissingAll() = default;
+};
Index: clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-special-member-functions.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-special-member-functions.rst
+++ clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-special-member-functions.rst
@@ -46,4 +46,19 @@
A(const A&);
A& operator=(const A&);
~A();
- }
+ };
+
+.. option:: AllowMissingMoveFunctionsWhenCopyIsDeleted
+
+   When set to `1` (default is `0`), this check doesn't flag classes which define deleted copy
+   operations but don't define move operations. This flags is related to Google C++ Style Guide
+   https://google.github.io/styleguide/

[PATCH] D75569: [clang-tidy] New check for methods marked __attribute__((unavailable)) that do not override a method from a superclass.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/objc-method-unavailable-not-override.rst:20
+
+Suggests a fix to remove the method declaration entirely.

You should also document the user-facing option for the check.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75569



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

[PATCH] D75747: [clang-format] Correct indentation for `[key] = value,` entries in C++ object initialisers

[Jonathan B Coe via Phabricator via cfe-commits]

jbcoe created this revision.
jbcoe added a reviewer: krasimir.
jbcoe added a project: clang-format.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
jbcoe added a comment.

Requires https://reviews.llvm.org/D75731 to be merged first.


Do not use continuation indent for '[' in blocks in C# code.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D75747

Files:
  clang/lib/Format/ContinuationIndenter.cpp
  clang/unittests/Format/FormatTestCSharp.cpp


Index: clang/unittests/Format/FormatTestCSharp.cpp
===
--- clang/unittests/Format/FormatTestCSharp.cpp
+++ clang/unittests/Format/FormatTestCSharp.cpp
@@ -549,6 +549,15 @@
 private Transformer _transformer = new X.Y {
   Filler = (Shape shape) => { return new Transform.Fill(shape, RED); },
   Scaler = (Shape shape) => { return new Transform.Resize(shape, 0.1); },
+};)",
+   Style);
+
+  // Dictionary initialisation.
+  verifyFormat(R"(//
+var myDict = new Dictionary {
+  ["name"] = _donald,
+  ["age"] = Convert.ToString(DateTime.Today.Year - 1934),
+  ["type"] = _duck,
 };)",
Style);
 }
Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -1047,6 +1047,8 @@
   if (NextNonComment->is(TT_ArraySubscriptLSquare)) {
 if (State.Stack.back().StartOfArraySubscripts != 0)
   return State.Stack.back().StartOfArraySubscripts;
+else if (Style.isCSharp()) // C# allows `["key"] = value` inside object 
initializers.
+  return State.Stack.back().Indent;
 return ContinuationIndent;
   }
 


Index: clang/unittests/Format/FormatTestCSharp.cpp
===
--- clang/unittests/Format/FormatTestCSharp.cpp
+++ clang/unittests/Format/FormatTestCSharp.cpp
@@ -549,6 +549,15 @@
 private Transformer _transformer = new X.Y {
   Filler = (Shape shape) => { return new Transform.Fill(shape, RED); },
   Scaler = (Shape shape) => { return new Transform.Resize(shape, 0.1); },
+};)",
+   Style);
+
+  // Dictionary initialisation.
+  verifyFormat(R"(//
+var myDict = new Dictionary {
+  ["name"] = _donald,
+  ["age"] = Convert.ToString(DateTime.Today.Year - 1934),
+  ["type"] = _duck,
 };)",
Style);
 }
Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -1047,6 +1047,8 @@
   if (NextNonComment->is(TT_ArraySubscriptLSquare)) {
 if (State.Stack.back().StartOfArraySubscripts != 0)
   return State.Stack.back().StartOfArraySubscripts;
+else if (Style.isCSharp()) // C# allows `["key"] = value` inside object initializers.
+  return State.Stack.back().Indent;
 return ContinuationIndent;
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D75574: RFC: Implement objc_direct_protocol attribute to remove protocol metadata

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added reviewers: aaron.ballman, rjmccall, erik.pilkington, 
MadCoder.
aaron.ballman added a comment.
Herald added a subscriber: dexonsmith.

Adding some more knowledgeable reviewers for comments on your RFC. I pointed 
out a few minor nits, but I'll hold off on a technical review until the 
ObjC-specific details are worked out and there is buy-in on the feature.




Comment at: clang/include/clang/AST/DeclObjC.h:2197
 
+  /// True if the protocol is tagged as objc_static_protocol
+  bool isStaticProtocol() const;

Comments should be properly punctuated (missing a full stop at the end of the 
sentence), here and elsewhere.



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:2609
+ const ParsedAttr &AL) {
+  if (S.getLangOpts().ObjCRuntime.allowsStaticProtocols()) {
+handleSimpleAttribute(S, D, AL);

This should be implemented via a custom `LangOpt` in `Attr.td`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75574



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

[PATCH] D75171: [Analyzer] Fix for incorrect use of container and iterator checkers

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

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

You seem to have uploaded the wrong diff :)

In D75171#1908259 , 
@baloghadamsoftware wrote:

> This is the so called "correct" solution. However, it does not even compile 
> because `getAnalyzerOptions()` and `getASTContext()` are non-const functions 
> of `CheckerManager`, but `shouldRegister`//XXX//`()` functions get it as 
> const reference.


That doesn't sound too drastic, why don't you make those methods const?

> Even if I use `const_cast` (not in this patch, just for testing) it does not 
> work, it does not prevent the crash: when trying to register 
> `IteratorModeling` which depends on `ContainerModeling` both checkers are 
> registered after `shouldRegisterContainerModeling()` function returning false.

Huh, that sounds interesting. I just tried it locally and it works like a 
charm. Would you mind me commandeering this patch to demonstrate?


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

https://reviews.llvm.org/D75171



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

[PATCH] D75747: [clang-format] Correct indentation for `[key] = value,` entries in C++ object initialisers

[Jonathan B Coe via Phabricator via cfe-commits]

jbcoe added a comment.

Requires https://reviews.llvm.org/D75731 to be merged first.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75747



___
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 updated this revision to Diff 248729.
pbaran added a comment.
Herald added a subscriber: wuzish.

Removed unintendedly added comment


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

https://reviews.llvm.org/D75745

Files:
  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
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-special-member-functions-allow-missing-move-when-copy-is-deleted.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-special-member-functions-allow-missing-move-when-copy-is-deleted.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-special-member-functions-allow-missing-move-when-copy-is-deleted.cpp
@@ -0,0 +1,49 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-special-member-functions %t -- -config="{CheckOptions: [{key: cppcoreguidelines-special-member-functions.AllowMissingMoveFunctionsWhenCopyIsDeleted, value: 1}]}" --
+
+class DefinesEverything {
+  DefinesEverything(const DefinesEverything &);
+  DefinesEverything(DefinesEverything &&);
+  DefinesEverything &operator=(const DefinesEverything &);
+  DefinesEverything &operator=(DefinesEverything &&);
+  ~DefinesEverything();
+};
+
+class DefinesNothing {
+};
+
+class DeletedCopyCtorAndOperator {
+  ~DeletedCopyCtorAndOperator() = default;
+  DeletedCopyCtorAndOperator(const DeletedCopyCtorAndOperator &) = delete;
+  DeletedCopyCtorAndOperator &operator=(const DeletedCopyCtorAndOperator &) = delete;
+};
+
+// CHECK-MESSAGES: [[@LINE+1]]:7: warning: class 'DefaultedCopyCtorAndOperator' defines a default destructor, a copy constructor and a copy assignment operator but does not define a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]
+class DefaultedCopyCtorAndOperator {
+  ~DefaultedCopyCtorAndOperator() = default;
+  DefaultedCopyCtorAndOperator(const DefaultedCopyCtorAndOperator &) = default;
+  DefaultedCopyCtorAndOperator &operator=(const DefaultedCopyCtorAndOperator &) = default;
+};
+
+// CHECK-MESSAGES: [[@LINE+1]]:7: warning: class 'DefinedCopyCtorAndOperator' defines a default destructor, a copy constructor and a copy assignment operator but does not define a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]
+class DefinedCopyCtorAndOperator {
+  ~DefinedCopyCtorAndOperator() = default;
+  DefinedCopyCtorAndOperator(const DefinedCopyCtorAndOperator &);
+  DefinedCopyCtorAndOperator &operator=(const DefinedCopyCtorAndOperator &);
+};
+
+// CHECK-MESSAGES: [[@LINE+1]]:7: warning: class 'MissingCopyCtor' defines a default destructor and a copy assignment operator but does not define a copy constructor, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]
+class MissingCopyCtor {
+  ~MissingCopyCtor() = default;
+  MissingCopyCtor &operator=(const MissingCopyCtor &) = delete;
+};
+
+// CHECK-MESSAGES: [[@LINE+1]]:7:  warning: class 'MissingCopyOperator' defines a default destructor and a copy constructor but does not define a copy assignment operator, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]
+class MissingCopyOperator {
+  ~MissingCopyOperator() = default;
+  MissingCopyOperator(const MissingCopyOperator &) = delete;
+};
+
+// CHECK-MESSAGES: [[@LINE+1]]:7:  warning: class 'MissingAll' defines a default destructor but does not define a copy constructor, a copy assignment operator, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]
+class MissingAll {
+  ~MissingAll() = default;
+};
Index: clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-special-member-functions.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-special-member-functions.rst
+++ clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-special-member-functions.rst
@@ -46,4 +46,19 @@
A(const A&);
A& operator=(const A&);
~A();
- }
+ };
+
+.. option:: AllowMissingMoveFunctionsWhenCopyIsDeleted
+
+   When set to `1` (default is `0`), this check doesn't flag classes which define deleted copy
+   operations but don't define move operations. This flags is related to Google C++ Style Guide
+   https://google.github.io/styleguide/cppguide.html#Copyable_Movable_Types. With this option enabled, the 
+   following class won't be flagged:
+   
+   .. code-block:: c++
+   
+ struct A {
+   A(const A&) = delete;
+   A& operator=(const A&) = delete;
+   ~A();
+ };
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h

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

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

Szelethus added a comment.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75356



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

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

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

Szelethus added a comment.

In D75356#1909610 , @balazske wrote:

> The D75682  is the one that should be used 
> now,


If this patch is supposed to be a followup to D75682 
, could you please mark it as such? I find 
these revisions difficult to navigate.

> I have "mirrored" all 3 changes in this stack to the new series in D75682 
> . Probably it is possible to reuse these 
> revisions instead but I do not know if it will not confuse phabricator 
> somehow (and how  phabricator behaves in such "tricky" cases, there is not a 
> usable documentation for it).

Since this is the patch where we held the discussion about error states, I 
think it would be better for this revision land first, that would also solve 
the problem of inlines being all over the place. It doesn't really matter 
whether we're introducing error states first through `feof` and `ferror`, or 
the admittedly quirky `fseek`. WDYT?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75356



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

[PATCH] D75700: [NFC] Let mangler accept GlobalDecl

[Yaxun Liu via Phabricator via cfe-commits]

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



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:1028
 else
-  MC.mangleName(ND, Out);
+  MC.mangleName(GD, Out);
   } else {

rjmccall wrote:
> What would be necessary in order for this to turn into just `mangleName(GD, 
> Out)`?   I suspect there are a lot of subtle assumptions between the 
> different mangling methods today that could probably be broken down a bit and 
> we'd end up with something much cleaner.
It seems to be just API changes. I will try changing them.


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

https://reviews.llvm.org/D75700



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

[PATCH] D75749: [clang-tidy] extend bugprone-signed-char-misuse check.

[Tamás Zolnai via Phabricator via cfe-commits]

ztamas created this revision.
Herald added subscribers: cfe-commits, xazax.hun.
Herald added a project: clang.
ztamas added a reviewer: aaron.ballman.
ztamas added a comment.

I run the check on LLVM code and found only one catch of suspicious comparison:

  clang-tidy -p=/home/zolnai/clang/build 
/home/zolnai/clang/llvm-project/clang/lib/Lex/Lexer.cpp
  /home/zolnai/clang/llvm-project/clang/lib/Lex/Lexer.cpp:1293:39: warning: 
comparison between signed and unsigned char [bugprone-signed-char-misuse]
if ((C == '\n' || C == '\r') && C != PrevC)


ztamas added a comment.

In the LibreOffice codebase, I found 8 catches:
https://gist.github.com/tzolnai/2b22492c0cf79f5dba577f6a878657e3

All catches seem valid to me.


Cover a new use case when using a 'signed char' as an integer
might lead to issue with non-ASCII characters. Comparing
a 'signed char' with an 'unsigned char' using equality / unequality
operator produces an unexpected result for non-ASCII characters.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D75749

Files:
  clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.h
  clang-tools-extra/docs/clang-tidy/checks/bugprone-signed-char-misuse.rst
  clang-tools-extra/test/clang-tidy/checkers/bugprone-signed-char-misuse.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-signed-char-misuse.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-signed-char-misuse.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-signed-char-misuse.cpp
@@ -62,6 +62,34 @@
   return NCharacter;
 }
 
+int SignedUnsignedCharEquality(signed char SCharacter) {
+  unsigned char USCharacter = 'a';
+  if (SCharacter == USCharacter) // CHECK-MESSAGES: [[@LINE]]:7: warning: comparison between signed and unsigned char [bugprone-signed-char-misuse]
+return 1;
+  return 0;
+}
+
+int SignedUnsignedCharUneqiality(signed char SCharacter) {
+  unsigned char USCharacter = 'a';
+  if (SCharacter != USCharacter) // CHECK-MESSAGES: [[@LINE]]:7: warning: comparison between signed and unsigned char [bugprone-signed-char-misuse]
+return 1;
+  return 0;
+}
+
+int CompareWithNonAsciiConstant(unsigned char USCharacter) {
+  const signed char SCharacter = -5;
+  if (USCharacter == SCharacter) // CHECK-MESSAGES: [[@LINE]]:7: warning: comparison between signed and unsigned char [bugprone-signed-char-misuse]
+return 1;
+  return 0;
+}
+
+int CompareWithUnsignedNonAsciiConstant(signed char SCharacter) {
+  const unsigned char USCharacter = 128;
+  if (USCharacter == SCharacter) // CHECK-MESSAGES: [[@LINE]]:7: warning: comparison between signed and unsigned char [bugprone-signed-char-misuse]
+return 1;
+  return 0;
+}
+
 ///
 /// Test cases correctly ignored by the check.
 
@@ -121,3 +149,61 @@
 
   return USCharacter;
 }
+
+int FixComparisonWithSignedCharCast(signed char SCharacter) {
+  unsigned char USCharacter = 'a';
+  if (SCharacter == static_cast(USCharacter)) // no warning
+return 1;
+  return 0;
+}
+
+int FixComparisonWithUnSignedCharCast(signed char SCharacter) {
+  unsigned char USCharacter = 'a';
+  if (static_cast(SCharacter) == USCharacter) // no warning
+return 1;
+  return 0;
+}
+
+// Make sure we don't catch other type of char comparison.
+int SameCharTypeComparison(signed char SCharacter) {
+  signed char SCharacter2 = 'a';
+  if (SCharacter == SCharacter2) // no warning
+return 1;
+  return 0;
+}
+
+// Make sure we don't catch other type of char comparison.
+int SameCharTypeComparison2(unsigned char USCharacter) {
+  unsigned char USCharacter2 = 'a';
+  if (USCharacter == USCharacter2) // no warning
+return 1;
+  return 0;
+}
+
+// Make sure we don't catch integer - char comparison.
+int CharIntComparison(signed char SCharacter) {
+  int ICharacter = 10;
+  if (SCharacter == ICharacter) // no warning
+return 1;
+  return 0;
+}
+
+int CompareWithAsciiLiteral(unsigned char USCharacter) {
+  if (USCharacter == 'x') // no warning
+return 1;
+  return 0;
+}
+
+int CompareWithAsciiConstant(unsigned char USCharacter) {
+  const signed char SCharacter = 'a';
+  if (USCharacter == SCharacter) // no warning
+return 1;
+  return 0;
+}
+
+int CompareWithUnsignedAsciiConstant(signed char SCharacter) {
+  const unsigned char USCharacter = 'a';
+  if (USCharacter == SCharacter) // no warning
+return 1;
+  return 0;
+}
\ No newline at end of file
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone-signed-char-misuse.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/bugprone-signed-char-misuse.rst
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone-signed-char-misuse.rst
@@ -3,27 +3,38 @@
 bugprone-signed-char-misuse
 ===
 
-Finds ``signed char`` -> integer

[PATCH] D75749: [clang-tidy] extend bugprone-signed-char-misuse check.

[Tamás Zolnai via Phabricator via cfe-commits]

ztamas added a comment.

I run the check on LLVM code and found only one catch of suspicious comparison:

  clang-tidy -p=/home/zolnai/clang/build 
/home/zolnai/clang/llvm-project/clang/lib/Lex/Lexer.cpp
  /home/zolnai/clang/llvm-project/clang/lib/Lex/Lexer.cpp:1293:39: warning: 
comparison between signed and unsigned char [bugprone-signed-char-misuse]
if ((C == '\n' || C == '\r') && C != PrevC)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75749



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

[PATCH] D75171: [Analyzer] Fix for incorrect use of container and iterator checkers

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

Szelethus added a comment.

Actually, this is the diff:

  diff --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td 
b/clang/include/clang/Basic/DiagnosticDriverKinds.td
  index ecd871e36ee..1f2c8c50a01 100644
  --- a/clang/include/clang/Basic/DiagnosticDriverKinds.td
  +++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td
  @@ -341,6 +341,8 @@ def err_analyzer_checker_option_unknown : Error<
 "checker '%0' has no option called '%1'">;
   def err_analyzer_checker_option_invalid_input : Error<
 "invalid input for checker option '%0', that expects %1">;
  +def err_analyzer_checker_incompatible_analyzer_option : Error<
  +  "checker cannot be enabled with analyzer option '%0' == %1">;
   
   def err_drv_invalid_hvx_length : Error<
 "-mhvx-length is not supported without a -mhvx/-mhvx= flag">;
  diff --git a/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h 
b/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
  index a3a85bfac13..680cb92e1ff 100644
  --- a/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
  +++ b/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
  @@ -169,8 +169,8 @@ public:
 void finishedCheckerRegistration();
   
 const LangOptions &getLangOpts() const { return LangOpts; }
  -  AnalyzerOptions &getAnalyzerOptions() { return AOptions; }
  -  ASTContext &getASTContext() {
  +  AnalyzerOptions &getAnalyzerOptions() const { return AOptions; }
  +  ASTContext &getASTContext() const {
   assert(Context);
   return *Context;
 }
  diff --git a/clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp 
b/clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp
  index 16a913e761c..f739417a37b 100644
  --- a/clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp
  +++ b/clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp
  @@ -12,6 +12,7 @@
   
   #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
   #include "clang/AST/DeclTemplate.h"
  +#include "clang/Driver/DriverDiagnostic.h"
   #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
   #include "clang/StaticAnalyzer/Core/Checker.h"
   #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
  @@ -1036,5 +1037,15 @@ void ento::registerContainerModeling(CheckerManager 
&mgr) {
   }
   
   bool ento::shouldRegisterContainerModeling(const CheckerManager &mgr) {
  +  if (!mgr.getLangOpts().CPlusPlus)
  +return false;
  +
  +  if (!mgr.getAnalyzerOptions().ShouldAggressivelySimplifyBinaryOperation) {
  +mgr.getASTContext().getDiagnostics().Report(
  +diag::err_analyzer_checker_incompatible_analyzer_option)
  +  << "aggressive-binary-operation-simplification" << "false";
  +return false;
  +  }
  +
 return true;
   }
  diff --git a/clang/test/Analysis/checker-dependencies.c 
b/clang/test/Analysis/checker-dependencies.c
  index 6c8583adb35..54d585561d1 100644
  --- a/clang/test/Analysis/checker-dependencies.c
  +++ b/clang/test/Analysis/checker-dependencies.c
  @@ -18,3 +18,11 @@
   
   // CHECK-IMPLICITLY-DISABLED-NOT: osx.cocoa.RetainCountBase
   // CHECK-IMPLICITLY-DISABLED-NOT: osx.cocoa.RetainCount
  +
  +// RUN: %clang_analyze_cc1 %s \
  +// RUN:   -analyzer-checker=cplusplus.IteratorModeling \
  +// RUN:   -analyzer-config aggressive-binary-operation-simplification=false \
  +// RUN:   -analyzer-list-enabled-checkers \
  +// RUN:   2>&1 | FileCheck %s -check-prefix=DEPENDENCY-SHOULD-NOT-REGISTER
  +
  +// DEPENDENCY-SHOULD-NOT-REGISTER: 
'aggressive-binary-operation-simplification' == false


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

https://reviews.llvm.org/D75171



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

[PATCH] D70172: [CUDA][HIP][OpenMP] Emit deferred diagnostics by a post-parsing AST travese

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

Ping


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

https://reviews.llvm.org/D70172



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

[PATCH] D68165: [analyzer][MallocChecker][NFC] Split checkPostCall up, deploy CallDescriptionMap

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

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

In D68165#1902702 , @Charusso wrote:

> I wish for a third map, something like `ReallocationMap`. Other than that it 
> is a great direction, I love it. Thanks!


Hah, that is a neat idea.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68165



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

[PATCH] D75749: [clang-tidy] extend bugprone-signed-char-misuse check.

[Tamás Zolnai via Phabricator via cfe-commits]

ztamas added a comment.

In the LibreOffice codebase, I found 8 catches:
https://gist.github.com/tzolnai/2b22492c0cf79f5dba577f6a878657e3

All catches seem valid to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75749



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

[clang] 2eff1c3 - [clang-format] Extend AllowShortLoopsOnASingleLine to do ... while loops.

[Mitchell Balan via cfe-commits]

Author: Mitchell Balan
Date: 2020-03-06T11:13:23-05:00
New Revision: 2eff1c3ce48ef529064e2dc006d57c37da0b4d84

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

LOG: [clang-format] Extend AllowShortLoopsOnASingleLine to do ... while loops.
Summary:
If AllowShortLoopsOnASingleLine is enabled,

  do
a++;
  while (true);

becomes

  do a++;
  while (true);

Reviewers: MyDeveloperDay, mitchell-stellar

Reviewed by: mitchell-stellar

Contributed by: DaanDeMeyer

Subscribers: cfe-commits

Tags: #clang, #clang-format

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

Added: 


Modified: 
clang/lib/Format/UnwrappedLineFormatter.cpp
clang/unittests/Format/FormatTest.cpp

Removed: 




diff  --git a/clang/lib/Format/UnwrappedLineFormatter.cpp 
b/clang/lib/Format/UnwrappedLineFormatter.cpp
index da9ee41da71b..84ccbec2150d 100644
--- a/clang/lib/Format/UnwrappedLineFormatter.cpp
+++ b/clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -411,7 +411,7 @@ class LineJoiner {
  ? tryMergeSimpleControlStatement(I, E, Limit)
  : 0;
 }
-if (TheLine->First->isOneOf(tok::kw_for, tok::kw_while)) {
+if (TheLine->First->isOneOf(tok::kw_for, tok::kw_while, tok::kw_do)) {
   return Style.AllowShortLoopsOnASingleLine
  ? tryMergeSimpleControlStatement(I, E, Limit)
  : 0;
@@ -514,7 +514,10 @@ class LineJoiner {
   return 0;
 Limit = limitConsideringMacros(I + 1, E, Limit);
 AnnotatedLine &Line = **I;
-if (Line.Last->isNot(tok::r_paren))
+if (!Line.First->is(tok::kw_do) && Line.Last->isNot(tok::r_paren))
+  return 0;
+// Only merge do while if do is the only statement on the line.
+if (Line.First->is(tok::kw_do) && !Line.Last->is(tok::kw_do))
   return 0;
 if (1 + I[1]->Last->TotalLength > Limit)
   return 0;

diff  --git a/clang/unittests/Format/FormatTest.cpp 
b/clang/unittests/Format/FormatTest.cpp
index cb052ecaf111..a6f35ba27069 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -556,6 +556,30 @@ TEST_F(FormatTest, FormatLoopsWithoutCompoundStatement) {
   verifyFormat("for (;;) /* still don't merge */\n"
"  continue;",
AllowsMergedLoops);
+  verifyFormat("do a++;\n"
+   "while (true);",
+   AllowsMergedLoops);
+  verifyFormat("do /* Don't merge */\n"
+   "  a++;\n"
+   "while (true);",
+   AllowsMergedLoops);
+  verifyFormat("do // Don't merge\n"
+   "  a++;\n"
+   "while (true);",
+   AllowsMergedLoops);
+  verifyFormat("do\n"
+   "  // Don't merge\n"
+   "  a++;\n"
+   "while (true);",
+   AllowsMergedLoops);
+  // Without braces labels are interpreted 
diff erently.
+  verifyFormat("{\n"
+   "  do\n"
+   "  label:\n"
+   "a++;\n"
+   "  while (true);\n"
+   "}",
+   AllowsMergedLoops);
 }
 
 TEST_F(FormatTest, FormatShortBracedStatements) {



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

[PATCH] D72326: [clang-format] Rebased on master: Add option to specify explicit config file

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar added a comment.

It's not more approval that is needed, it's just that someone with commit 
access (assuming you do not) needs to find the time to commit this. For what 
it's worth, I'm getting a patch rejection for the 4th block in 
lib/Format/Format.cpp. It seems the contents of `LoadConfigFile` need to be 
updated to reflect the most recent changes, so please rebase against master 
when you can.


Repository:
  rC Clang

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

https://reviews.llvm.org/D72326



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

[PATCH] D75752: [Sema] Move pointer to int cast warnings under -Wmicrosoft-cast

[Arthur Eubanks via Phabricator via cfe-commits]

aeubanks created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
aeubanks edited the summary of this revision.
aeubanks added reviewers: thakis, rnk.
aeubanks retitled this revision from "Move warnings added in 
https://reviews.llvm.org/D75708 under -Wmicrosoft-cast" to "[Sema] Move pointer 
to int cast warnings under -Wmicrosoft-cast".
aeubanks edited the summary of this revision.

Microsoft extensions should be under a -Wmicrosoft-foo group.
Added in https://reviews.llvm.org/D75708.

There shouldn't be a reason to separate out void pointers since this is C++.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D75752

Files:
  clang/lib/Sema/SemaCast.cpp


Index: clang/lib/Sema/SemaCast.cpp
===
--- clang/lib/Sema/SemaCast.cpp
+++ clang/lib/Sema/SemaCast.cpp
@@ -2209,10 +2209,8 @@
   bool MicrosoftException =
   Self.getLangOpts().MicrosoftExt && !DestType->isBooleanType();
   if (MicrosoftException) {
-unsigned Diag = SrcType->isVoidPointerType()
-? diag::warn_void_pointer_to_int_cast
-: diag::warn_pointer_to_int_cast;
-Self.Diag(OpRange.getBegin(), Diag) << SrcType << DestType << OpRange;
+Self.Diag(OpRange.getBegin(), diag::ext_ms_pointer_to_int_cast)
+<< SrcType << DestType << OpRange;
   } else {
 msg = diag::err_bad_reinterpret_cast_small_int;
 return TC_Failed;


Index: clang/lib/Sema/SemaCast.cpp
===
--- clang/lib/Sema/SemaCast.cpp
+++ clang/lib/Sema/SemaCast.cpp
@@ -2209,10 +2209,8 @@
   bool MicrosoftException =
   Self.getLangOpts().MicrosoftExt && !DestType->isBooleanType();
   if (MicrosoftException) {
-unsigned Diag = SrcType->isVoidPointerType()
-? diag::warn_void_pointer_to_int_cast
-: diag::warn_pointer_to_int_cast;
-Self.Diag(OpRange.getBegin(), Diag) << SrcType << DestType << OpRange;
+Self.Diag(OpRange.getBegin(), diag::ext_ms_pointer_to_int_cast)
+<< SrcType << DestType << OpRange;
   } else {
 msg = diag::err_bad_reinterpret_cast_small_int;
 return TC_Failed;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D75022: clang-format: Extend AllowShortLoopsOnASingleLine to do ... while loops.

[Mitchell via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG2eff1c3ce48e: [clang-format] Extend 
AllowShortLoopsOnASingleLine to do ... while loops. (authored by 
mitchell-stellar).

Changed prior to commit:
  https://reviews.llvm.org/D75022?vs=246512&id=248748#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75022

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -556,6 +556,30 @@
   verifyFormat("for (;;) /* still don't merge */\n"
"  continue;",
AllowsMergedLoops);
+  verifyFormat("do a++;\n"
+   "while (true);",
+   AllowsMergedLoops);
+  verifyFormat("do /* Don't merge */\n"
+   "  a++;\n"
+   "while (true);",
+   AllowsMergedLoops);
+  verifyFormat("do // Don't merge\n"
+   "  a++;\n"
+   "while (true);",
+   AllowsMergedLoops);
+  verifyFormat("do\n"
+   "  // Don't merge\n"
+   "  a++;\n"
+   "while (true);",
+   AllowsMergedLoops);
+  // Without braces labels are interpreted differently.
+  verifyFormat("{\n"
+   "  do\n"
+   "  label:\n"
+   "a++;\n"
+   "  while (true);\n"
+   "}",
+   AllowsMergedLoops);
 }
 
 TEST_F(FormatTest, FormatShortBracedStatements) {
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -411,7 +411,7 @@
  ? tryMergeSimpleControlStatement(I, E, Limit)
  : 0;
 }
-if (TheLine->First->isOneOf(tok::kw_for, tok::kw_while)) {
+if (TheLine->First->isOneOf(tok::kw_for, tok::kw_while, tok::kw_do)) {
   return Style.AllowShortLoopsOnASingleLine
  ? tryMergeSimpleControlStatement(I, E, Limit)
  : 0;
@@ -514,7 +514,10 @@
   return 0;
 Limit = limitConsideringMacros(I + 1, E, Limit);
 AnnotatedLine &Line = **I;
-if (Line.Last->isNot(tok::r_paren))
+if (!Line.First->is(tok::kw_do) && Line.Last->isNot(tok::r_paren))
+  return 0;
+// Only merge do while if do is the only statement on the line.
+if (Line.First->is(tok::kw_do) && !Line.Last->is(tok::kw_do))
   return 0;
 if (1 + I[1]->Last->TotalLength > Limit)
   return 0;


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -556,6 +556,30 @@
   verifyFormat("for (;;) /* still don't merge */\n"
"  continue;",
AllowsMergedLoops);
+  verifyFormat("do a++;\n"
+   "while (true);",
+   AllowsMergedLoops);
+  verifyFormat("do /* Don't merge */\n"
+   "  a++;\n"
+   "while (true);",
+   AllowsMergedLoops);
+  verifyFormat("do // Don't merge\n"
+   "  a++;\n"
+   "while (true);",
+   AllowsMergedLoops);
+  verifyFormat("do\n"
+   "  // Don't merge\n"
+   "  a++;\n"
+   "while (true);",
+   AllowsMergedLoops);
+  // Without braces labels are interpreted differently.
+  verifyFormat("{\n"
+   "  do\n"
+   "  label:\n"
+   "a++;\n"
+   "  while (true);\n"
+   "}",
+   AllowsMergedLoops);
 }
 
 TEST_F(FormatTest, FormatShortBracedStatements) {
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -411,7 +411,7 @@
  ? tryMergeSimpleControlStatement(I, E, Limit)
  : 0;
 }
-if (TheLine->First->isOneOf(tok::kw_for, tok::kw_while)) {
+if (TheLine->First->isOneOf(tok::kw_for, tok::kw_while, tok::kw_do)) {
   return Style.AllowShortLoopsOnASingleLine
  ? tryMergeSimpleControlStatement(I, E, Limit)
  : 0;
@@ -514,7 +514,10 @@
   return 0;
 Limit = limitConsideringMacros(I + 1, E, Limit);
 AnnotatedLine &Line = **I;
-if (Line.Last->isNot(tok::r_paren))
+if (!Line.First->is(tok::kw_do) && Line.Last->isNot(tok::r_paren))
+  return 0;
+// Only merge do while if do is the only statement on the line.
+if (Line.First->is(tok::kw_do) && !Line.Last->is(tok::kw_do))
   return 0;
 if (1 + I[1]->Last->TotalL

[PATCH] D75209: [OPENMP][NVPTX]Fix PR45003: add support for complex functions.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75209



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

[PATCH] D74144: [OPENMP50]Add basic support for array-shaping operation.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

Ping 3


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74144



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

[clang] 5dadf57 - [OPENMP50]Add 'depobj' modifier in 'depend' clauses.

[Alexey Bataev via cfe-commits]

Author: Alexey Bataev
Date: 2020-03-06T11:44:57-05:00
New Revision: 5dadf577d59b53110f2a0084dc9ef9f974116955

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

LOG: [OPENMP50]Add 'depobj' modifier in 'depend' clauses.

Added basic support (parsing/sema/serialization) for depobj dependency
kind in depend clauses.

Added: 


Modified: 
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Basic/OpenMPKinds.def
clang/lib/CodeGen/CGOpenMPRuntime.cpp
clang/lib/Parse/ParseOpenMP.cpp
clang/lib/Sema/SemaOpenMP.cpp
clang/test/OpenMP/depobj_codegen.cpp
clang/test/OpenMP/depobj_messages.cpp
clang/test/OpenMP/ordered_messages.cpp
clang/test/OpenMP/target_depend_messages.cpp
clang/test/OpenMP/target_enter_data_depend_messages.cpp
clang/test/OpenMP/target_exit_data_depend_messages.cpp
clang/test/OpenMP/target_parallel_depend_messages.cpp
clang/test/OpenMP/target_parallel_for_depend_messages.cpp
clang/test/OpenMP/target_parallel_for_simd_depend_messages.cpp
clang/test/OpenMP/target_simd_depend_messages.cpp
clang/test/OpenMP/target_teams_depend_messages.cpp
clang/test/OpenMP/target_teams_distribute_depend_messages.cpp
clang/test/OpenMP/target_teams_distribute_parallel_for_depend_messages.cpp

clang/test/OpenMP/target_teams_distribute_parallel_for_simd_depend_messages.cpp
clang/test/OpenMP/target_teams_distribute_simd_depend_messages.cpp
clang/test/OpenMP/target_update_depend_messages.cpp
clang/test/OpenMP/task_ast_print.cpp
clang/test/OpenMP/task_depend_messages.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index cb3dd24a44cf..181341fed780 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -9633,7 +9633,7 @@ def err_omp_expected_var_name_member_expr : Error<
 def err_omp_expected_var_name_member_expr_or_array_item : Error<
   "expected variable name%select{|, data member of current class}0, array 
element or array section">;
 def err_omp_expected_addressable_lvalue_or_array_item : Error<
-  "expected addressable lvalue expression, array element or array section">;
+  "expected addressable lvalue expression, array element or array 
section%select{| of non 'omp_depend_t' type}0">;
 def err_omp_expected_named_var_member_or_array_expression: Error<
   "expected expression containing only member accesses and/or array sections 
based on named variables">;
 def err_omp_bit_fields_forbidden_in_clause : Error<

diff  --git a/clang/include/clang/Basic/OpenMPKinds.def 
b/clang/include/clang/Basic/OpenMPKinds.def
index dbc0d1cec2c7..70f962427e36 100644
--- a/clang/include/clang/Basic/OpenMPKinds.def
+++ b/clang/include/clang/Basic/OpenMPKinds.def
@@ -384,6 +384,7 @@ OPENMP_DEPEND_KIND(in)
 OPENMP_DEPEND_KIND(out)
 OPENMP_DEPEND_KIND(inout)
 OPENMP_DEPEND_KIND(mutexinoutset)
+OPENMP_DEPEND_KIND(depobj)
 OPENMP_DEPEND_KIND(source)
 OPENMP_DEPEND_KIND(sink)
 

diff  --git a/clang/lib/CodeGen/CGOpenMPRuntime.cpp 
b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
index 620f6ea3e654..e16dfbcddbe2 100644
--- a/clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -5214,6 +5214,7 @@ static RTLDependenceKindTy 
translateDependencyKind(OpenMPDependClauseKind K) {
 break;
   case OMPC_DEPEND_source:
   case OMPC_DEPEND_sink:
+  case OMPC_DEPEND_depobj:
   case OMPC_DEPEND_unknown:
 llvm_unreachable("Unknown task dependence type");
   }

diff  --git a/clang/lib/Parse/ParseOpenMP.cpp b/clang/lib/Parse/ParseOpenMP.cpp
index 27b88b331199..1da884f657a0 100644
--- a/clang/lib/Parse/ParseOpenMP.cpp
+++ b/clang/lib/Parse/ParseOpenMP.cpp
@@ -3198,9 +3198,7 @@ bool Parser::ParseOpenMPVarList(OpenMPDirectiveKind DKind,
   Data.RLoc = Tok.getLocation();
   if (!T.consumeClose())
 Data.RLoc = T.getCloseLocation();
-  return (Kind == OMPC_depend && Data.ExtraModifier != OMPC_DEPEND_unknown &&
-  Vars.empty()) ||
- (Kind != OMPC_depend && Kind != OMPC_map && Vars.empty()) ||
+  return (Kind != OMPC_depend && Kind != OMPC_map && Vars.empty()) ||
  (MustHaveTail && !Data.TailExpr) || InvalidReductionId ||
  IsInvalidMapperModifier;
 }

diff  --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp
index d42a9f45cf8c..c0fb56c47371 100644
--- a/clang/lib/Sema/SemaOpenMP.cpp
+++ b/clang/lib/Sema/SemaOpenMP.cpp
@@ -12247,8 +12247,9 @@ OMPClause 
*Sema::ActOnOpenMPUpdateClause(OpenMPDependClauseKind Kind,
  SourceLocation LParenLoc,
  SourceLocation EndLoc) {
   if (Kind == OMPC_DEPEND_unknown || Kind == OMPC_DEPEND_source ||
- 

[PATCH] D75621: [clang-tidy] Use ; as separator for HeaderFileExtensions

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs updated this revision to Diff 248755.
jroelofs added a comment.

- Don't spam the deprecation message. Move that to release notes.
- Drop unnecessary `const`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75621

Files:
  clang-tools-extra/clang-tidy/bugprone/DynamicStaticInitializersCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/DynamicStaticInitializersCheck.h
  clang-tools-extra/clang-tidy/google/GlobalNamesInHeadersCheck.cpp
  clang-tools-extra/clang-tidy/google/GlobalNamesInHeadersCheck.h
  clang-tools-extra/clang-tidy/google/UnnamedNamespaceInHeaderCheck.cpp
  clang-tools-extra/clang-tidy/google/UnnamedNamespaceInHeaderCheck.h
  clang-tools-extra/clang-tidy/llvm/HeaderGuardCheck.h
  clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.cpp
  clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.h
  clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.cpp
  clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h
  clang-tools-extra/clang-tidy/utils/HeaderGuard.h
  clang-tools-extra/docs/ReleaseNotes.rst

Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -125,6 +125,11 @@
   check now detects in class initializers and constructor initializers which
   are deemed to be redundant.
 
+- Checks supporting the ``HeaderFileExtensions`` flag now support ``;`` as a
+  delimiter in addition to ``,``, with the latter being deprecated as of this
+  release. This simplifies how one specifies the options on the command line:
+  ``--config="{CheckOptions: [{ key: HeaderFileExtensions, value: h;;hpp;hxx }]}"``
+
 Renamed checks
 ^^
 
Index: clang-tools-extra/clang-tidy/utils/HeaderGuard.h
===
--- clang-tools-extra/clang-tidy/utils/HeaderGuard.h
+++ clang-tools-extra/clang-tidy/utils/HeaderGuard.h
@@ -18,11 +18,12 @@
 
 /// Finds and fixes header guards.
 /// The check supports these options:
-///   - `HeaderFileExtensions`: a comma-separated list of filename extensions of
-/// header files (The filename extension should not contain "." prefix).
-/// ",h,hh,hpp,hxx" by default.
+///   - `HeaderFileExtensions`: a semicolon-separated list of filename
+/// extensions of header files (The filename extension should not contain
+/// "." prefix). ";h;hh;hpp;hxx" by default.
+///
 /// For extension-less header files, using an empty string or leaving an
-/// empty string between "," if there are other filename extensions.
+/// empty string between ";" if there are other filename extensions.
 class HeaderGuardCheck : public ClangTidyCheck {
 public:
   HeaderGuardCheck(StringRef Name, ClangTidyContext *Context)
@@ -30,7 +31,8 @@
 RawStringHeaderFileExtensions(Options.getLocalOrGlobal(
 "HeaderFileExtensions", utils::defaultHeaderFileExtensions())) {
 utils::parseFileExtensions(RawStringHeaderFileExtensions,
-   HeaderFileExtensions, ',');
+   HeaderFileExtensions,
+   utils::defaultFileExtensionDelimiters());
   }
   void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
Preprocessor *ModuleExpanderPP) override;
Index: clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h
===
--- clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h
+++ clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h
@@ -34,11 +34,16 @@
 
 /// Returns recommended default value for the list of header file
 /// extensions.
-inline StringRef defaultHeaderFileExtensions() { return ",h,hh,hpp,hxx"; }
+inline StringRef defaultHeaderFileExtensions() { return ";h;hh;hpp;hxx"; }
+
+/// Returns recommended default value for the list of file extension
+/// delimiters.
+inline StringRef defaultFileExtensionDelimiters() { return ",;"; }
 
 /// Parses header file extensions from a semicolon-separated list.
 bool parseFileExtensions(StringRef AllFileExtensions,
- FileExtensionsSet &FileExtensions, char Delimiter);
+ FileExtensionsSet &FileExtensions,
+ StringRef Delimiters);
 
 /// Decides whether a file has one of the specified file extensions.
 bool isFileExtension(StringRef FileName,
Index: clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.cpp
===
--- clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.cpp
+++ clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.cpp
@@ -9,6 +9,7 @@
 #include "FileExtensionsUtils.h"
 #include "clang/Basic/CharInfo.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Support/raw_ostream.h"
 
 namespace clang {

[PATCH] D75665: [analyzer] On-demand parsing capability for CTU

[Gabor Marton via Phabricator via cfe-commits]

martong added inline comments.



Comment at: clang/lib/CrossTU/CrossTranslationUnit.cpp:365
 
+  llvm::SmallString<256> AbsPath = CTUDir;
+  llvm::sys::path::append(AbsPath, Identifier);

Could we check somehow if `CTUDir` is indeed an absolute path? If not then 
probably we should bail out with an error. Though I am not sure if there is an 
easy and portable way in llvm:: to check for beeing an absolute path.



Comment at: clang/lib/CrossTU/CrossTranslationUnit.cpp:375
+/// Load the AST from a source-file, which is supposed to be located inside the
+/// compilation database \p OnDemandParsingCommands. The compilation database
+/// can contain the path of the file under the key "file" as an absolute path,

There is no variable named as `OnDemandParsingCommands`, perhaps you made a 
rename but forgot to rename in the comments?



Comment at: clang/lib/CrossTU/CrossTranslationUnit.cpp:384
+/// the invocation inside ClangTool is always made with an absolute path. \p
+/// ASTSourcePath is assumed to be the lookup-name of the file, which comes 
from
+/// the Index. The Index is built by the \p clang-extdef-mapping tool, which is

There is no variable named as `ASTSourcePath`, perhaps you made a rename but 
forgot to rename in the comments?



Comment at: clang/lib/CrossTU/CrossTranslationUnit.cpp:413
+  Files.push_back(std::string(Identifier));
+  ClangTool Tool(*CompileCommands, Files, CI.getPCHContainerOperations());
+

martong wrote:
> martong wrote:
> > Seems like `Tool` has it's lifetime ended when `load()` finishes. But we 
> > have long living `ASTUnits` whose lifetime are longer then the `Tool` which 
> > created them. Is this not a problem?
> `CI.getPCHContainerOperations()` is probably not needed, because we are not 
> going to exercise the ASTReader, so this could be a nullptr (default param 
> value).
If `CI.getPCHContainerOperations()` is not needed then we can remove the `CI` 
member of `ASTOnDemandLoader`.



Comment at: clang/lib/CrossTU/CrossTranslationUnit.cpp:443
+/// be the absolute path of the file.
+*SourceFilePath = Identifier.str();
+

Perhaps we should make sure that `Identifier` is indeed an absolute path and if 
not then we should emit an error. If a user do not use CodeChecker to generate 
the ExternalDefMapping it may contain relative paths. See e.g.: 
https://clang.llvm.org/docs/analyzer/user-docs/CrossTranslationUnit.html#manual-ctu-analysis



Comment at: clang/lib/CrossTU/CrossTranslationUnit.cpp:457
+return llvm::make_error(
+index_error_code::ambiguous_compile_commands_database);
+

Could we have a lit test for this case? I.e having a compile_commands.json with 
two ambiguous entries for the same file and then we should expect the compiler 
diag?
(Note, I am asking this because it is not immediate for me from the code that 
this will ever fire.)



Comment at: clang/test/Analysis/ctu-main.c:53
   clang_analyzer_eval(res == 6); // expected-warning{{TRUE}}
+  // Call something with uninitialized from the same function in which the 
implicit was called.
+  // This is necessary to reproduce a special bug in NoStoreFuncVisitor.

Is this hunk related?



Comment at: clang/test/Analysis/ctu-on-demand-parsing.c:3
 // RUN: mkdir -p %t/ctudir2
-// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu \
-// RUN:   -emit-pch -o %t/ctudir2/ctu-other.c.ast %S/Inputs/ctu-other.c
-// RUN: cp %S/Inputs/ctu-other.c.externalDefMap.txt 
%t/ctudir2/externalDefMap.txt
-// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -fsyntax-only -std=c89 -analyze 
\
+// RUN: echo '[{"directory":"%S/Inputs","command":"clang -x c -std=c89 -c 
ctu-other.c","file":"ctu-other.c"}]' | sed -e 's/\\//g' > 
%t/ctudir2/compile_commands.json
+// RUN: %clang_extdef_map %S/Inputs/ctu-other.c > %t/ctudir2/externalDefMap.txt

Why do we need `sed` here? I don't see any `\` in the compile commands json. Is 
it because of Windows builds? Could you please explain in a comment maybe in 
the previous line.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75665



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

[PATCH] D75621: [clang-tidy] Use ; as separator for HeaderFileExtensions

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs updated this revision to Diff 248757.
jroelofs added a comment.

- Drop dead include.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75621

Files:
  clang-tools-extra/clang-tidy/bugprone/DynamicStaticInitializersCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/DynamicStaticInitializersCheck.h
  clang-tools-extra/clang-tidy/google/GlobalNamesInHeadersCheck.cpp
  clang-tools-extra/clang-tidy/google/GlobalNamesInHeadersCheck.h
  clang-tools-extra/clang-tidy/google/UnnamedNamespaceInHeaderCheck.cpp
  clang-tools-extra/clang-tidy/google/UnnamedNamespaceInHeaderCheck.h
  clang-tools-extra/clang-tidy/llvm/HeaderGuardCheck.h
  clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.cpp
  clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.h
  clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.cpp
  clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h
  clang-tools-extra/clang-tidy/utils/HeaderGuard.h
  clang-tools-extra/docs/ReleaseNotes.rst

Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -125,6 +125,11 @@
   check now detects in class initializers and constructor initializers which
   are deemed to be redundant.
 
+- Checks supporting the ``HeaderFileExtensions`` flag now support ``;`` as a
+  delimiter in addition to ``,``, with the latter being deprecated as of this
+  release. This simplifies how one specifies the options on the command line:
+  ``--config="{CheckOptions: [{ key: HeaderFileExtensions, value: h;;hpp;hxx }]}"``
+
 Renamed checks
 ^^
 
Index: clang-tools-extra/clang-tidy/utils/HeaderGuard.h
===
--- clang-tools-extra/clang-tidy/utils/HeaderGuard.h
+++ clang-tools-extra/clang-tidy/utils/HeaderGuard.h
@@ -18,11 +18,12 @@
 
 /// Finds and fixes header guards.
 /// The check supports these options:
-///   - `HeaderFileExtensions`: a comma-separated list of filename extensions of
-/// header files (The filename extension should not contain "." prefix).
-/// ",h,hh,hpp,hxx" by default.
+///   - `HeaderFileExtensions`: a semicolon-separated list of filename
+/// extensions of header files (The filename extension should not contain
+/// "." prefix). ";h;hh;hpp;hxx" by default.
+///
 /// For extension-less header files, using an empty string or leaving an
-/// empty string between "," if there are other filename extensions.
+/// empty string between ";" if there are other filename extensions.
 class HeaderGuardCheck : public ClangTidyCheck {
 public:
   HeaderGuardCheck(StringRef Name, ClangTidyContext *Context)
@@ -30,7 +31,8 @@
 RawStringHeaderFileExtensions(Options.getLocalOrGlobal(
 "HeaderFileExtensions", utils::defaultHeaderFileExtensions())) {
 utils::parseFileExtensions(RawStringHeaderFileExtensions,
-   HeaderFileExtensions, ',');
+   HeaderFileExtensions,
+   utils::defaultFileExtensionDelimiters());
   }
   void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
Preprocessor *ModuleExpanderPP) override;
Index: clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h
===
--- clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h
+++ clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h
@@ -34,11 +34,16 @@
 
 /// Returns recommended default value for the list of header file
 /// extensions.
-inline StringRef defaultHeaderFileExtensions() { return ",h,hh,hpp,hxx"; }
+inline StringRef defaultHeaderFileExtensions() { return ";h;hh;hpp;hxx"; }
+
+/// Returns recommended default value for the list of file extension
+/// delimiters.
+inline StringRef defaultFileExtensionDelimiters() { return ",;"; }
 
 /// Parses header file extensions from a semicolon-separated list.
 bool parseFileExtensions(StringRef AllFileExtensions,
- FileExtensionsSet &FileExtensions, char Delimiter);
+ FileExtensionsSet &FileExtensions,
+ StringRef Delimiters);
 
 /// Decides whether a file has one of the specified file extensions.
 bool isFileExtension(StringRef FileName,
Index: clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.cpp
===
--- clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.cpp
+++ clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.cpp
@@ -33,9 +33,16 @@
 }
 
 bool parseFileExtensions(StringRef AllFileExtensions,
- FileExtensionsSet &FileExtensions, char Delimiter) {
+ FileExtensionsSet &FileExtensions,
+ Strin

[PATCH] D74144: [OPENMP50]Add basic support for array-shaping operation.

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added inline comments.



Comment at: clang/lib/Parse/ParseExpr.cpp:2891
   ArgExprs);
 }
+  } else if (!getLangOpts().ObjC && getLangOpts().OpenMP >= 50 &&

Out of curiosity, why do we prevent Objective C here? I doubt anyone runs it in 
OpenMP mode but it would be kinda cool.



Comment at: clang/lib/Sema/SemaExpr.cpp:4744
+  // Delay analysis of the types/expressions if instantiation/specialixzation 
is
+  // required.
+  if (!BaseTy->isAnyPointerType() &&

Typo above.



Comment at: clang/lib/Sema/SemaExpr.cpp:4750
+return OMPArrayShapingExpr::Create(Context, Context.DependentTy, Base,
+   LParenLoc, RParenLoc, Dims, Brackets);
+  if (!BaseTy->isAnyPointerType())

Does this trigger as well if BaseTy is a pointer type but dependent? I mean, 
why do we need the first check (`!BaseTy->isAnyPointerType() &&`)?



Comment at: clang/test/OpenMP/parallel_reduction_messages.c:8
+#pragma omp parallel reduction(+ : incomplete, ([10])p) // expected-error {{a 
reduction list item with incomplete type 'int []'}} omp45-error {{expected 
expression}} omp50-error {{expected variable name, array element or array 
section}}
   ;
 }

Why is there a 50 error here?



Comment at: clang/test/OpenMP/task_ast_print.cpp:153
+#pragma omp task untied mergeable depend(out:argv[:a][1], 
(arr)[0:],([argc][10])argv) if(task: argc > 0) priority(f)
+  // CHECK-NEXT: #pragma omp task untied mergeable depend(out : 
argv[:a][1],(arr)[0:],([argc][10])argv) if(task: argc > 0) priority(f)
   a = 2;

We do not complain that argc is non-constant. Is that on purpose?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74144



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

[PATCH] D74935: [LangRef][AliasAnalysis] Clarify `noalias` affects only modified objects

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

In D74935#1909054 , @jeroen.dobbelaere 
wrote:

> In D74935#1908665 , @jdoerfert wrote:
>
> > I think we conflate two things here:
> >
> > 1. The modifications to the LangRef which should be in accordance with the 
> > C standard (at least I haven't seen you contradict the new wording 
> > directly).
>
>
> imho, the proposed wording is still confusing, and does not handle the case 
> with the extra indirections.
>  Unless the 'by any means' was meant to also include the  '.. Every  access  
> that modifies X shall be considered also to modify P,for the purposes of this 
> subclause. .. ' from the restrict specification.
>  If that is the idea, we should mention it explicitly.


I would say that once we get modeling for indirect restrict we can adapt the 
lang ref accordingly. For now there is only have outer level restrict/noalias.

>> 2. The extended `noalias` deduction D73428 .
>> 
>>   If you look at the commit message in D73428 
>> , it says that `noalias` is not just 
>> derived for all `readonly` arguments. In your example we access `unknown` 
>> memory, e.g., `**pA=42`, so case (1) cannot be applied. For case (2) we need 
>> to show that the loads of `pA` and `pB` do not alias, which we cannot.
> 
> That sounds good. Is there also a testcase (similar to D74935#1907100 
>  or D74935#1907939 
>  ) that explicitly checks that 
> 'noalias' is not deduced ?

I'll add one :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74935



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

[PATCH] D75723: [X86] Make intrinsics _BitScan* not limited to Windows

[Craig Topper via Phabricator via cfe-commits]

craig.topper added inline comments.



Comment at: clang/include/clang/Basic/BuiltinsX86.def:1904
+// BITSCAN
+TARGET_BUILTIN(_BitScanForward, "UcUNi*UNi", "n", "")
+TARGET_BUILTIN(_BitScanReverse, "UcUNi*UNi", "n", "")

skan wrote:
> craig.topper wrote:
> > The N specifier here is sort of MSVC mode specific. I need to think about 
> > this.
> > 
> > This also makes this available without including a header file which isn't 
> > good if it doesn't start with __builtin.
> I think we can define a new builtin `__builtin_ia32_BitScanForward` in 
> BuiltinsX86.def. And when macro `_MSC_VER` is not defined, define 
> `_BitScanForward` in ia32intrin.h by calling the 
> `__builtin_ia32_BitScanForward`. Then we can avoid the N specifier and the 
> name issue. Do you think it's appropriate?
Instead of new builtins, can we use  __builtin_clz, __builtin_clzl, 
__builin_ctz, __builtin_ctzl?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75723



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

[clang] 865456d - [Concepts] Add null check for TemplateTypeParmType::getDecl() in GetContainedInventedTypeParmVisitor

[Saar Raz via cfe-commits]

Author: Saar Raz
Date: 2020-03-06T19:32:10+02:00
New Revision: 865456d589e093582acaafd17d58ad1c0cce66af

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

LOG: [Concepts] Add null check for TemplateTypeParmType::getDecl() in 
GetContainedInventedTypeParmVisitor

GetContainedInventedTypeParmVisitor would not account for the case where 
TemplateTypeParmType::getDecl() is
nullptr, causing bug #45102.

Add the nullptr check.

Added: 


Modified: 
clang/lib/Sema/SemaTemplateInstantiate.cpp
clang/test/SemaTemplate/instantiate-abbreviated-template.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp 
b/clang/lib/Sema/SemaTemplateInstantiate.cpp
index 3ae2822a9803..b37b4bbba783 100644
--- a/clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -2162,7 +2162,7 @@ namespace {
 // The deduced type itself.
 TemplateTypeParmDecl *VisitTemplateTypeParmType(
 const TemplateTypeParmType *T) {
-  if (!T->getDecl()->isImplicit())
+  if (!T->getDecl() || !T->getDecl()->isImplicit())
 return nullptr;
   return T->getDecl();
 }

diff  --git a/clang/test/SemaTemplate/instantiate-abbreviated-template.cpp 
b/clang/test/SemaTemplate/instantiate-abbreviated-template.cpp
index 99801115626f..1f2171a25ebb 100644
--- a/clang/test/SemaTemplate/instantiate-abbreviated-template.cpp
+++ b/clang/test/SemaTemplate/instantiate-abbreviated-template.cpp
@@ -31,3 +31,15 @@ struct G {
 
 using gf1 = decltype(G::foo1('a', 1, 2, 3, 4)); // 
expected-error{{no matching function}}
 using gf2 = decltype(G::foo2('a', 1, 2)); // expected-error{{no 
matching function}}
+
+
+// Regression (bug #45102): check that instantiation works where there is no
+// TemplateTypeParmDecl
+template  using id = T;
+
+template 
+constexpr void g() {
+  id f;
+}
+
+static_assert((g(), true));
\ No newline at end of file



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

[PATCH] D74973: [analyzer] StdLibraryFunctionsChecker refactor w/ inheritance

[Gabor Marton via Phabricator via cfe-commits]

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

In D74973#1900852 , @Szelethus wrote:

> I have some high level questions, you have spent far more time with this code 
> and I'm happy to be proven wrong! :)
>
> In D74973#1889188 , @martong wrote:
>
> > > Is really more kind of constraint needed than range constraint?
> >
> > Yes, there are other constraints I am planning to implement:
> >
> > - Size requirements E.g.: asctime_s(char *buf, rsize_t bufsz, const struct 
> > tm *time_ptr); `buf` size must be at least `bufsz`.
> > - Not-null
> > - Not-uninitalized
> > - Not-tainted
>
>
> Are we really sure that we need to express that with constraints? Can't we 
> just change the name of `ValueRange` (or encapsulate it in another class) and 
> add more fields to it, such as taintedness or initializedness? Is there an 
> incentive to keep `ValueRange` lean?


Yes there is: separation of concerns, i.e. ValueRange should handle Values with 
int ranges. One class should handle only one well established responsibility.

> This doesn't look too bad:
> 
>   auto Getc = [&]() {
> return Summary(ArgTypes{Irrelevant}, RetType{IntTy}, NoEvalCall)
> .Case(
> {ReturnValueDescription(RangeConstraints(WithinRange, {{EOFv, 
> EOFv}, {0, UCharMax}},
> Tainted, Non_Uninitialized});
>   };
> 
> 
> 
> 
>>> A non-null can be represented as range constraint too.
>> 
>> Actually, to implement that we should have a branch in all 
>> `ValueRange::apply*` functions that handles `Loc` SVals. Unfortunately, a 
>> pointer cannot be handled as `NonLoc`, and the current Range based 
>> implementation handles `NonLoc`s only.
> 
> So, why didn't we take that route instead? Marking a pointer non-null seems 
> to be a less invasive change.

The answer is the same here. I think we should not mix too much implementation 
of different cases into one monumental function/class.

> 
> 
>>> The compare constraint is used only for the return value for which a 
>>> special `ReturnConstraint` can be used to handle the return value not like 
>>> a normal argument (and then the `Ret` special value is not needed).
>> 
>> The Compare constraint is already forced into a Range "concept" whereas it 
>> has nothing to do with ranges. By handling compare constraints separately, 
>> we attach a single responsibility to each constraint class, instead of 
>> having a monolithic god constraint class. Take a look at this coerced data 
>> representation that we have today in ValueRange:
>> 
>>   BinaryOperator::Opcode getOpcode() const {
>> assert(Kind == ComparesToArgument);
>> assert(Args.size() == 1);
>> BinaryOperator::Opcode Op =
>> static_cast(Args[0].first);
>> assert(BinaryOperator::isComparisonOp(Op) &&
>>"Only comparison ops are supported for ComparesToArgument");
>> return Op;
>>   }
>>
>> 
>> Subclasses are a good way to get rid of this not-so-intuitive structure and 
>> assertions.
> 
> Having more fields feels like another possible solution.

Yes, having fields is another approach of having an enum and a union (i.e. 
tagged union). A tagged union is a variant basically. So adding more fields 
would finally be very similar to a variant based solution. And that is useful 
only if we know the set of classes beforehand and we want to gradually 
implement new operations on them.
In our case we know that we need 3 operations: apply, negate, warning and no 
more. And we want to gradually add new classes: NonNull, NotUninitialized, ...




Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:151
+
+  using ValueConstraintPtr = std::shared_ptr;
+  /// The complete list of constraints that defines a single branch.

Szelethus wrote:
> gamesh411 wrote:
> > martong wrote:
> > > Note here, we need a copyable, polymorphic and default initializable type 
> > > (vector needs that). A raw pointer were good, however, we cannot default 
> > > initialize that. unique_ptr makes the Summary class non-copyable, 
> > > therefore not an option.
> > > Releasing the copyablitly requirement would render the initialization of 
> > > the Summary map infeasible.
> > > Perhaps we could come up with a [[ 
> > > https://www.youtube.com/watch?v=bIhUE5uUFOA | type erasure technique 
> > > without inheritance ]] once we consider the shared_ptr as restriction, 
> > > but for now that seems to be overkill.
> > std::variant (with std::monostate for the default constructibility) would 
> > also be an option  (if c++17 were supported). But this is not really an 
> > issue, i agree with that.
> Ugh, we've historically been very hostile towards virtual functions. We don't 
> mind them that much when they don't have to run a lot, like during bug report 
> construction, but as a core part of the analysis, I'm not sure what the 
> current stance 

[PATCH] D75758: [Sema] Add -Wpointer-to-enum-cast and -Wvoid-pointer-to-enum-cast

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance created this revision.
nathanchance added reviewers: Mordante, rjmccall.
Herald added a project: clang.

GCC does not warn on casts from pointers to enumerators, while clang
currently does: https://godbolt.org/z/3DFDVG

This causes a bunch of extra warnings in the Linux kernel, where
certain structs contain a void pointer to avoid using a gigantic
union for all of the various types of driver data, such as
versions.

Add a diagnostic that allows certain projects like the kernel to
disable the warning just for enums, which allowst those projects to
keep full compatibility with GCC but keeps the intention of treating
casts to integers and enumerators the same by default so that other
projects have the opportunity to catch issues not noticed before (or
follow suite and disable the warning).

Link: https://github.com/ClangBuiltLinux/linux/issues/887


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D75758

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaCast.cpp
  clang/test/Sema/cast.c


Index: clang/test/Sema/cast.c
===
--- clang/test/Sema/cast.c
+++ clang/test/Sema/cast.c
@@ -186,3 +186,23 @@
 void *intToPointerCast3() {
   return (void*)(1 + 3);
 }
+
+void voidPointerToEnumCast(VoidPtr v) {
+  (void)(X) v; // expected-warning{{cast to smaller integer type 'X' from 
'VoidPtr' (aka 'void *')}}
+  // Test that casts to void* can be controlled separately
+  // from other -Wpointer-to-enum-cast warnings.
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wvoid-pointer-to-enum-cast"
+  (void)(X) v; // no-warning
+#pragma clang diagnostic pop
+}
+
+void pointerToEnumCast(CharPtr v) {
+  (void)(X) v; // expected-warning{{cast to smaller integer type 'X' from 
'CharPtr' (aka 'char *')}}
+  // Test that casts to void* can be controlled separately
+  // from other -Wpointer-to-enum-cast warnings.
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wvoid-pointer-to-enum-cast"
+  (void)(X) v; // expected-warning{{cast to smaller integer type 'X' from 
'CharPtr' (aka 'char *')}}
+#pragma clang diagnostic pop
+}
Index: clang/lib/Sema/SemaCast.cpp
===
--- clang/lib/Sema/SemaCast.cpp
+++ clang/lib/Sema/SemaCast.cpp
@@ -2777,11 +2777,18 @@
   // If the result cannot be represented in the integer type, the behavior
   // is undefined. The result need not be in the range of values of any
   // integer type.
-  unsigned Diag = Self.getLangOpts().MicrosoftExt
-  ? diag::ext_ms_pointer_to_int_cast
-  : SrcType->isVoidPointerType()
-? diag::warn_void_pointer_to_int_cast
-: diag::warn_pointer_to_int_cast;
+  unsigned Diag;
+  if (Self.getLangOpts().MicrosoftExt)
+Diag = diag::ext_ms_pointer_to_int_cast;
+  else if (SrcType->isVoidPointerType())
+if (DestType->isEnumeralType())
+  Diag = diag::warn_void_pointer_to_enum_cast;
+else
+  Diag = diag::warn_void_pointer_to_int_cast;
+  else if (DestType->isEnumeralType())
+Diag = diag::warn_pointer_to_enum_cast;
+  else
+Diag = diag::warn_pointer_to_int_cast;
   Self.Diag(OpRange.getBegin(), Diag) << SrcType << DestType << OpRange;
 }
   }
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3669,9 +3669,15 @@
 def warn_pointer_to_int_cast : Warning<
   "cast to smaller integer type %1 from %0">,
   InGroup;
+def warn_pointer_to_enum_cast : Warning<
+  "cast to smaller integer type %1 from %0">,
+  InGroup;
 def warn_void_pointer_to_int_cast : Warning<
   "cast to smaller integer type %1 from %0">,
   InGroup;
+def warn_void_pointer_to_enum_cast : Warning<
+  "cast to smaller integer type %1 from %0">,
+  InGroup;
 def ext_ms_pointer_to_int_cast : ExtWarn<
   "cast to smaller integer type %1 from %0 is a Microsoft extension">,
 InGroup;
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -838,9 +838,13 @@
 def IntToVoidPointerCast : DiagGroup<"int-to-void-pointer-cast">;
 def IntToPointerCast : DiagGroup<"int-to-pointer-cast",
  [IntToVoidPointerCast]>;
-def VoidPointerToIntCast : DiagGroup<"void-pointer-to-int-cast">;
+def VoidPointerToEnumCast : DiagGroup<"void-pointer-to-enum-cast">;
+def VoidPointerToIntCast : DiagGroup<"void-pointer-to-int-cast",
+ [VoidPointerToEnumCast]>;
+def PointerToEnumCast : Di

[PATCH] D74144: [OPENMP50]Add basic support for array-shaping operation.

[Alexey Bataev via Phabricator via cfe-commits]

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



Comment at: clang/lib/Parse/ParseExpr.cpp:2891
   ArgExprs);
 }
+  } else if (!getLangOpts().ObjC && getLangOpts().OpenMP >= 50 &&

jdoerfert wrote:
> Out of curiosity, why do we prevent Objective C here? I doubt anyone runs it 
> in OpenMP mode but it would be kinda cool.
Just used this to prevent the error messages from Objective C tests when 
enabled OpenMP by default to test that parsing work correctl in the corner 
cases. Will remove this check.



Comment at: clang/lib/Sema/SemaExpr.cpp:4750
+return OMPArrayShapingExpr::Create(Context, Context.DependentTy, Base,
+   LParenLoc, RParenLoc, Dims, Brackets);
+  if (!BaseTy->isAnyPointerType())

jdoerfert wrote:
> Does this trigger as well if BaseTy is a pointer type but dependent? I mean, 
> why do we need the first check (`!BaseTy->isAnyPointerType() &&`)?
Not sure this is a real issue. If the type is not a pointer type, but the 
dependent, just skip all checks. All the required checks will be performed upon 
the instantiation.



Comment at: clang/test/OpenMP/parallel_reduction_messages.c:8
+#pragma omp parallel reduction(+ : incomplete, ([10])p) // expected-error {{a 
reduction list item with incomplete type 'int []'}} omp45-error {{expected 
expression}} omp50-error {{expected variable name, array element or array 
section}}
   ;
 }

jdoerfert wrote:
> Why is there a 50 error here?
Array shaping is not supported for the reductions



Comment at: clang/test/OpenMP/task_ast_print.cpp:153
+#pragma omp task untied mergeable depend(out:argv[:a][1], 
(arr)[0:],([argc][10])argv) if(task: argc > 0) priority(f)
+  // CHECK-NEXT: #pragma omp task untied mergeable depend(out : 
argv[:a][1],(arr)[0:],([argc][10])argv) if(task: argc > 0) priority(f)
   a = 2;

jdoerfert wrote:
> We do not complain that argc is non-constant. Is that on purpose?
Yes, the standard allows the use of the non-constant values as the dimensions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74144



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

[PATCH] D75209: [OPENMP][NVPTX]Fix PR45003: add support for complex functions.

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

I am unsure we need this with the proper math support. Sema patch for that is 
going for review today. I'll try this out soon.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75209



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

[PATCH] D73898: [analyzer] StdLibraryFunctionsChecker: Add argument constraints

[Gabor Marton via Phabricator via cfe-commits]

martong marked 9 inline comments as done.
martong added a comment.

In D73898#1894923 , @balazske wrote:

> It may be useful to make a "macro value map" kind of object. Some macros can 
> be added to it as a string, and it is possible to lookup for an `Expr` if one 
> of the added macros is used there. This can be done by checking the concrete 
> (numeric) value of the `Expr` and compare to the value of the macro, or by 
> checking if the expression comes from a macro and take this macro name (use 
> string comparison). Such an object can be useful because the functionality is 
> needed at more checkers, for example the ones I am working on (StreamChecker 
> and ErrorReturnChecker too).


Please see my previous answer to @gamesh411




Comment at: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td:296
+def StdCLibraryFunctionArgsChecker : Checker<"StdCLibraryFunctionArgs">,
+  HelpText<"Check constraints of arguments of C standard library functions">,
+  Dependencies<[StdCLibraryFunctionsChecker]>,

Szelethus wrote:
> How about we add an example as well?
You mean like NonNull or other constraints?



Comment at: 
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:697-699
+  // The behavior is undefined if the value of the argument is not
+  // representable as unsigned char or is not equal to EOF. See e.g. C99
+  // 7.4.1.2 The isalpha function (p: 181-182).

gamesh411 wrote:
> martong wrote:
> > Szelethus wrote:
> > > This is true for the rest of the summaries as well, but shouldn't we 
> > > retrieve the `unsigned char` size from `ASTContext`?
> > Yes this is a good idea. I will do this.
> > 
> > What bothers me really much, however, is that we should handle EOF in a 
> > platform dependent way as well ... and I have absolutely no idea how to do 
> > that given that is defined by a macro in a platform specific header file. I 
> > am desperately in need for help and ideas about how could we get the value 
> > of EOF for the analysed platform.
> If the EOF is not used in the TU analyzed, then there would be no way to find 
> the specific `#define`.
> Another approach would be to check if the value is defined by an expression 
> that is the EOF define (maybe transitively?).
I believe that the given standard C lib implementation (e.g. glibc) must 
provide a header for the prototypes of these functions where EOF is also 
defined transitively in any of the dependent system headers. Otherwise user 
code could misuse the value of EOF and thus the program would behave in an 
undefined manner.

C99 clearly states that you should #include  to use isalhpa.



Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:161
+
+ValueRange negate() const {
+  ValueRange tmp(*this);

Szelethus wrote:
> Maybe `complement` would be a better name? That sounds a lot more like a set 
> operation. Also, this function highlights well that inheritance might not be 
> the best solution here.
Well, we check the argument constraint validity by trying to apply it's logical 
negation. In case of a range inclusion this is being out of that range. In case 
of non-null this is being null. And so on. The logic how we try to check an 
argument constraint is the same in all cases of the different constraints. And 
that is the point: in order to support a new kind of constraint we just have to 
figure out how to "apply" and "negate" one constraint. In my opinion this is a 
perfect case for polimorphism.



Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:191
+  ///   * a list of branches - a list of list of ranges -
+  /// i.e. a list of lists of lists of segments,
+  ///   * a list of argument constraints, that must be true on every branch.

Szelethus wrote:
> I think that is a rather poor example to help understand what `list of list 
> of ranges` means :) -- Could you try to find something better?
Yeah, that part definitely should be reworded.



Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:10
 // This checker improves modeling of a few simple library functions.
 // It does not generate warnings.
 //

Szelethus wrote:
> I suspect this comment is no longer relevant.
Uh, yes.



Comment at: clang/test/Analysis/std-c-library-functions-arg-constraints.c:1-7
+// RUN: %clang_analyze_cc1 %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=apiModeling.StdCLibraryFunctions \
+// RUN:   -analyzer-checker=apiModeling.StdCLibraryFunctionArgs \
+// RUN:   -analyzer-checker=debug.ExprInspection \
+// RUN:   -triple x86_64-unknown-linux-gnu \
+// RUN:   -verify

Szelethus wrote:
> Hmm, why do we have 2 different test files that essentially do the same? 
> Shouldn't we only have a single o

[PATCH] D75760: [clang-format] Do not indent C# array initialisers as continuations

[Jonathan B Coe via Phabricator via cfe-commits]

jbcoe added a comment.

Needs https://reviews.llvm.org/D75731 and https://reviews.llvm.org/D75747 to be 
merged first.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75760



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

[PATCH] D75209: [OPENMP][NVPTX]Fix PR45003: add support for complex functions.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D75209#1909976 , @jdoerfert wrote:

> I am unsure we need this with the proper math support. Sema patch for that is 
> going for review today. I'll try this out soon.


It has nothing to do with the math functions support. These functions are 
required for the definition of __muldc3/__divdc3/__mulsc2/__divsc3 functions 
emitted for the complex types. CUDA does absolutely the same.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75209



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

[PATCH] D75760: [clang-format] Do not indent C# array initialisers as continuations

[Jonathan B Coe via Phabricator via cfe-commits]

jbcoe created this revision.
jbcoe added a reviewer: krasimir.
jbcoe added a project: clang-format.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
jbcoe added a comment.

Needs https://reviews.llvm.org/D75731 and https://reviews.llvm.org/D75747 to be 
merged first.


Flag '= {' as a braced init list when parsing C# code.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D75760

Files:
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/FormatTestCSharp.cpp


Index: clang/unittests/Format/FormatTestCSharp.cpp
===
--- clang/unittests/Format/FormatTestCSharp.cpp
+++ clang/unittests/Format/FormatTestCSharp.cpp
@@ -562,6 +562,17 @@
Style);
 }
 
+TEST_F(FormatTestCSharp, CSharpArrayInitializers) {
+  FormatStyle Style = getGoogleStyle(FormatStyle::LK_CSharp);
+  
+  verifyFormat(R"(//
+private MySet[] setPoints = {
+  new Point(),
+  new Point(),
+};)",
+   Style);
+}
+
 TEST_F(FormatTestCSharp, CSharpNamedArguments) {
   FormatStyle Style = getGoogleStyle(FormatStyle::LK_CSharp);
 
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1436,6 +1436,11 @@
 
   nextToken();
   if (FormatTok->Tok.is(tok::l_brace)) {
+// Block kind should probably be set to BK_BracedInit for any language.
+// C# needs this change to ensure that array initialisers and object
+// initialisers are indented the same way.
+if (Style.isCSharp())
+  FormatTok->BlockKind = BK_BracedInit;
 nextToken();
 parseBracedList();
   } else if (Style.Language == FormatStyle::LK_Proto &&
@@ -1628,7 +1633,7 @@
 bool UnwrappedLineParser::parseBracedList(bool ContinueOnSemicolons,
   tok::TokenKind ClosingBraceKind) {
   bool HasError = false;
-
+  
   // FIXME: Once we have an expression parser in the UnwrappedLineParser,
   // replace this by using parseAssigmentExpression() inside.
   do {


Index: clang/unittests/Format/FormatTestCSharp.cpp
===
--- clang/unittests/Format/FormatTestCSharp.cpp
+++ clang/unittests/Format/FormatTestCSharp.cpp
@@ -562,6 +562,17 @@
Style);
 }
 
+TEST_F(FormatTestCSharp, CSharpArrayInitializers) {
+  FormatStyle Style = getGoogleStyle(FormatStyle::LK_CSharp);
+  
+  verifyFormat(R"(//
+private MySet[] setPoints = {
+  new Point(),
+  new Point(),
+};)",
+   Style);
+}
+
 TEST_F(FormatTestCSharp, CSharpNamedArguments) {
   FormatStyle Style = getGoogleStyle(FormatStyle::LK_CSharp);
 
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1436,6 +1436,11 @@
 
   nextToken();
   if (FormatTok->Tok.is(tok::l_brace)) {
+// Block kind should probably be set to BK_BracedInit for any language.
+// C# needs this change to ensure that array initialisers and object
+// initialisers are indented the same way.
+if (Style.isCSharp())
+  FormatTok->BlockKind = BK_BracedInit;
 nextToken();
 parseBracedList();
   } else if (Style.Language == FormatStyle::LK_Proto &&
@@ -1628,7 +1633,7 @@
 bool UnwrappedLineParser::parseBracedList(bool ContinueOnSemicolons,
   tok::TokenKind ClosingBraceKind) {
   bool HasError = false;
-
+  
   // FIXME: Once we have an expression parser in the UnwrappedLineParser,
   // replace this by using parseAssigmentExpression() inside.
   do {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D73898: [analyzer] StdLibraryFunctionsChecker: Add argument constraints

[Gabor Marton via Phabricator via cfe-commits]

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

In D73898#1901142 , @balazske wrote:

> Is it sure that the signedness in the ranges is handled correctly? The EOF is 
> a negative value but the `RangeInt` is unsigned type. The 
> `tryExpandAsInteger` returns `int` too that is put into an unsigned 
> `RangeInt` later. Probably it is better to use `APSInt` for the ranges? (The 
> problem exists already before this change.)


That is not a problem, because finally in `apply` we use an `APSInt` that is 
constructed by considering the correct `T` type, e.g.:

  const llvm::APSInt &Min = BVF.getValue(R[I].first, T);

We could consider `RangeInt` as a buffer that is big enough to hold the 
representation of the range values. The concrete interpretation of the bits (as 
`T`) is done by `APSInt`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73898



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

[PATCH] D75298: [Clang][SVE] Parse builtin type string for scalable vectors

[Sander de Smalen via Phabricator via cfe-commits]

sdesmalen added a subscriber: simon_tatham.
sdesmalen added a comment.
Herald added a subscriber: danielkiss.

In D75298#1904561 , @efriedma wrote:

> > Do you happen to know which method in Sema does this? I had a look before, 
> > but couldn't find where we could do something like this.
>
> See Sema::LazilyCreateBuiltin, and its caller Sema::LookupBuiltin.


Thanks for those pointers! I spoke with @simon_tatham today, who is also very 
interested in this in order to simplify the MVE header file.

I would like to suggest first trying to land the current implementation (using 
the header file with all the declarations), so that we can use the regression 
tests added as part of this effort to test a different implementation that does 
not rely as much on the header file. What do you think?


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

https://reviews.llvm.org/D75298



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