[PATCH] D74669: [clang-tidy] New check: bugprone-suspicious-include

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.



Comment at: clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h:38
+/// extensions.
+inline StringRef defaultHeaderFileExtensions() { return ",h,hh,hpp,hxx"; }
+

njames93 wrote:
> A lot of configuration options for clang tidy use semi-colon `;` seperated 
> lists. Would it not be wise to carry on the convention and use semi colon 
> here (and below) as well. It could break a few peoples configuration but that 
> would be realised  fairly quickly and updated
I think i'm missing the point of D75621.
Why can't we keep using `,` as delimitter?
Why do we have to change everything, introduce inconsistency
(is it still documented that `,` is still valid?),
and spam into terminal if `,` is found?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74669



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

[PATCH] D75582: [clangd] Track document versions, include them with diags, enhance logs

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: clang-tools-extra/clangd/ClangdServer.h:73
 virtual void onDiagnosticsReady(PathRef File,
+const llvm::json::Value &Version,
 std::vector Diagnostics) {}

sammccall wrote:
> kadircet wrote:
> > can we rather have `Optional`s here(both for callbacks and 
> > `addDocument`)?
> > 
> > as clangdserver layer doesn't touch json objects at all currently.
> I really do want to make these opaque at the lower layer.
> json is a bit fiddly though, reworked to use strings instead.
looks better thanks !


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75582



___
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 marked 8 inline comments as done.
balazske added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:37-40
+  // NoError: No error flag is set or stream is not open.
+  // EofError: EOF condition (feof returns true)
+  // OtherError: other (non-EOF) error (ferror returns true)
+  // AnyError: EofError or OtherError

Szelethus wrote:
> balazske wrote:
> > Szelethus wrote:
> > > These too. Also, I'm not yet sure whether we need `OtherError` and 
> > > `AnyError`, as stated in a later inline.
> > I plan to use `AnyError` for failing functions that can have **EOF** and 
> > other error as result. At a later `ferror` or `feof` call a new state split 
> > follows to differentiate the error (instead of having to add 3 new states 
> > after the function, for success, EOF error and other error). If other 
> > operation is called we know anyway that some error happened.
> I think it would still be better to introduce them as we find uses for them 
> :) Also, to stay in the scope of this patch, shouldn't we only introduce 
> `FseekError`? If we did, we could make warning messages a lot clearer.
This change is generally about introduction of the error handling, not 
specifically about `fseek`. Probably not `fseek` was the best choice, it could 
be any other function. Probably I can add another, or multiple ones, but then 
again the too-big-patch problem comes up. (If now the generic error states 
exist the diffs for adding more stream operations could be more simple by only 
adding new functions and not changing the `StreamState` at all.) (How is this 
related to warning messages?)



Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:351-354
+  // Set state to any error.
+  // Assume that EOF and other error is possible after the call.
+  StateFailed =
+  StateFailed->set(StreamSym, 
StreamState::getOpenedWithError());

Szelethus wrote:
> balazske wrote:
> > Szelethus wrote:
> > > But why? The standard suggests that the error state isn't changed upon 
> > > failure. I think we should leave it as-is.
> > The fseek can fail and set the error flag, see the example code here:
> > https://en.cppreference.com/w/c/io/fseek
> > Probably it can not set the **EOF** flag, according to that page it is 
> > possible to seek after the end of the file. So the function can succeed or 
> > fail with `OtherError`.
> Yup, right, you won :) I tried some examples out on my system, and it could 
> preserve or change the error state of the stream. To me it seems like that 
> not checking the state of the stream after a failed `fseek` is surely 
> unadvised.
> 
> This still should be `AnyError` (or `FseekError`), as according to the 
> `OtherError`'s description `OtherError` may not refer to `EOF`, yet after a 
> failed `fseek` call the stream can be in `EOF`:
> 
> ```lang=bash
> $ cat test.cpp
> ```
> ```lang=cpp
> #include 
> 
> int main() {
>   FILE *F = fopen("test.cpp", "r");
> 
>   while (EOF != fgetc(F)) {}
> 
>   if (feof(F))
> printf("The file is closed!\n");
> 
>   // Return value is non-zero on failure.
>   if (fseek(F, -10, SEEK_END)) {
> if (feof(F))
>   printf("The file is still closed!\n");
> else
>   printf("The file is no longer closed!\n");
>   }
> }
> ```
> ```lang=bash
> $ clang test.cpp && ./a.out
> The file is closed!
> The file is still closed!
> ```
After some experimenting I think it is best to have every possibility of errors 
after `fseek`. This  means, either **EOF**, or other-error, or non-**EOF** and 
not other-error (but failed fseek call according to return value). So we need 1 
success and 2 error return value states, one error return with `AnyError` and 
one with `NoError` (strange but happens according to the shown output). 
Probably there is relation between the previous state and the produced result 
of `fseek` but I do not want to figure out, it may be different in other 
systems and the documentations say nothing.

This comes from this program:
```
#include 

void print_result(FILE *F, int rc, const char *errtxt) {
  printf("\n%s", errtxt);
  if (rc) {
printf("failed...\n");
if (feof(F)) {
  printf("FEOF\n");
}
if (ferror(F)) {
  printf("FERROR\n");
  perror("error");
}
  } else {
printf("success\n");
  }
}

int main() {
  FILE *F = fopen("fseek.c", "r");

  while (EOF != fgetc(F)) {}
  print_result(F, 1, "read done\n");

  // Return value is non-zero on failure.
  int rc = fseek(F, -10, SEEK_END);
  print_result(F, rc, "seek invalid\n");

  rc = fseek(F, 2, SEEK_END);
  print_result(F, rc, "seek valid\n");

  rc = fseek(F, -10, SEEK_END);
  print_result(F, rc, "seek invalid\n");
  
  fputs("str", F);
  print_result(F, 1, "failed operation\n");

  rc = fseek(F, -10, SEEK_END);
  print_result(F, rc, "seek invalid\n");

  rc = fseek(F, -1, SEEK_END);
  print_result(F, rc, "seek valid\n");

  rc = fseek(F, 

[PATCH] D75603: [clangd] Add instrumentation mode in clangd for metrics collection.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

In D75603#1906418 , @sammccall wrote:

> Forgot to mention: I also think the trace approach certainly has things going 
> for it, or even parsing out the messages from the existing logs.
>  But in this particular case the callback happens to be extremely convenient 
> and also not invasive (since the data structures are already exposed, code 
> complete has an opts struct etc). And since this is for analysis we have a 
> lot of flexibility to rework if it stops being easy to maintain.


as discussed offline, I was rather afraid of the initial version of the patch, 
but the final version seems ok as it only adds a single field to 
codecompleteopts.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75603



___
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 marked 2 inline comments as done.
balazske added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:37-40
+  // NoError: No error flag is set or stream is not open.
+  // EofError: EOF condition (feof returns true)
+  // OtherError: other (non-EOF) error (ferror returns true)
+  // AnyError: EofError or OtherError

balazske wrote:
> Szelethus wrote:
> > balazske wrote:
> > > Szelethus wrote:
> > > > These too. Also, I'm not yet sure whether we need `OtherError` and 
> > > > `AnyError`, as stated in a later inline.
> > > I plan to use `AnyError` for failing functions that can have **EOF** and 
> > > other error as result. At a later `ferror` or `feof` call a new state 
> > > split follows to differentiate the error (instead of having to add 3 new 
> > > states after the function, for success, EOF error and other error). If 
> > > other operation is called we know anyway that some error happened.
> > I think it would still be better to introduce them as we find uses for them 
> > :) Also, to stay in the scope of this patch, shouldn't we only introduce 
> > `FseekError`? If we did, we could make warning messages a lot clearer.
> This change is generally about introduction of the error handling, not 
> specifically about `fseek`. Probably not `fseek` was the best choice, it 
> could be any other function. Probably I can add another, or multiple ones, 
> but then again the too-big-patch problem comes up. (If now the generic error 
> states exist the diffs for adding more stream operations could be more simple 
> by only adding new functions and not changing the `StreamState` at all.) (How 
> is this related to warning messages?)
For now, the `EofError` and `OtherError` can be removed, in this change these 
are not used (according to how `fseek` will be done).


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

Re: [clang] ec3060c - [AST] Refactor propagation of dependency bits. NFC

[Mikael Holmén via cfe-commits]

Hi Hokein, Ilya,

Clang warns on this code when compiled without asserts:

../../clang/lib/AST/TemplateName.cpp:189:3: error: unannotated fall-
through between switch labels [-Werror,-Wimplicit-fallthrough]
  default:
  ^
../../clang/lib/AST/TemplateName.cpp:189:3: note: insert 'break;' to
avoid fall-through
  default:
  ^
  break; 
1 error generated.


The code is

  case TemplateName::NameKind::OverloadedTemplate:
assert(false && "overloaded templates shouldn't survive to here.");
  default:

I'm not sure if there should be a break after the assert (not sure if
someone would warn about that) or should there be a LLVM_FALLTHROUGH;
or should the assert perhaps be an llvm_unreachable?

/Mikael

On Wed, 2020-03-04 at 02:25 -0800, Haojian Wu via cfe-commits wrote:
> Author: Ilya Biryukov
> Date: 2020-03-04T11:25:17+01:00
> New Revision: ec3060c72de6ab6992269318d92764199856e5fe
> 
> URL: 
> https://github.com/llvm/llvm-project/commit/ec3060c72de6ab6992269318d92764199856e5fe
> DIFF: 
> https://github.com/llvm/llvm-project/commit/ec3060c72de6ab6992269318d92764199856e5fe.diff
> 
> LOG: [AST] Refactor propagation of dependency bits. NFC
> 
> Summary:
> This changes introduces an enum to represent dependencies as a
> bitmask
> and extract common patterns from code that computes dependency bits
> into
> helper functions.
> 
> Reviewers: rsmith, martong, shafik, ilya-biryukov, hokein
> 
> Subscribers: hokein, sammccall, Mordante, riccibruno,
> merge_guards_bot, rnkovacs, cfe-commits
> 
> Tags: #clang
> 
> Differential Revision: https://reviews.llvm.org/D71920
> 
> Added: 
> clang/include/clang/AST/DependencyFlags.h
> 
> Modified: 
> clang/include/clang/AST/Expr.h
> clang/include/clang/AST/ExprConcepts.h
> clang/include/clang/AST/NestedNameSpecifier.h
> clang/include/clang/AST/Stmt.h
> clang/include/clang/AST/TemplateBase.h
> clang/include/clang/AST/TemplateName.h
> clang/include/clang/AST/Type.h
> clang/lib/AST/ASTImporter.cpp
> clang/lib/AST/Expr.cpp
> clang/lib/AST/ExprCXX.cpp
> clang/lib/AST/ExprConcepts.cpp
> clang/lib/AST/ExprObjC.cpp
> clang/lib/AST/NestedNameSpecifier.cpp
> clang/lib/AST/TemplateBase.cpp
> clang/lib/AST/TemplateName.cpp
> clang/lib/Sema/SemaOverload.cpp
> clang/lib/Serialization/ASTReaderStmt.cpp
> 
> Removed: 
> 
> 
> 
> #
> ###
> diff  --git a/clang/include/clang/AST/DependencyFlags.h
> b/clang/include/clang/AST/DependencyFlags.h
> new file mode 100644
> index ..c27016aa9aec
> --- /dev/null
> +++ b/clang/include/clang/AST/DependencyFlags.h
> @@ -0,0 +1,138 @@
> +//===--- DependencyFlags.h ---
> -===//
> +//
> +// Part of the LLVM Project, under the Apache License v2.0 with LLVM
> Exceptions.
> +// See https://llvm.org/LICENSE.txt for license information.
> +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
> +//
> +//===---
> ---===//
> +#ifndef LLVM_CLANG_AST_DEPENDENCYFLAGS_H
> +#define LLVM_CLANG_AST_DEPENDENCYFLAGS_H
> +
> +#include "clang/Basic/BitmaskEnum.h"
> +#include "llvm/ADT/BitmaskEnum.h"
> +#include 
> +
> +namespace clang {
> +struct ExprDependenceScope {
> +  enum ExprDependence : uint8_t {
> +UnexpandedPack = 1,
> +Instantiation = 2,
> +Type = 4,
> +Value = 8,
> +
> +None = 0,
> +All = 15,
> +
> +TypeInstantiation = Type | Instantiation,
> +ValueInstantiation = Value | Instantiation,
> +TypeValueInstantiation = Type | Value | Instantiation,
> +
> +LLVM_MARK_AS_BITMASK_ENUM(/*LargestValue=*/Value)
> +  };
> +};
> +using ExprDependence = ExprDependenceScope::ExprDependence;
> +static constexpr unsigned ExprDependenceBits = 4;
> +
> +struct TypeDependenceScope {
> +  enum TypeDependence : uint8_t {
> +/// Whether this type contains an unexpanded parameter pack
> +/// (for C++11 variadic templates)
> +UnexpandedPack = 1,
> +/// Whether this type somehow involves a template parameter,
> even
> +/// if the resolution of the type does not depend on a template
> parameter.
> +Instantiation = 2,
> +/// Whether this type is a dependent type (C++ [temp.dep.type]).
> +Dependent = 4,
> +/// Whether this type is a variably-modified type (C99 6.7.5).
> +VariablyModified = 8,
> +
> +None = 0,
> +All = 15,
> +
> +DependentInstantiation = Dependent | Instantiation,
> +
> +LLVM_MARK_AS_BITMASK_ENUM(/*LargestValue=*/VariablyModified)
> +  };
> +};
> +using TypeDependence = TypeDependenceScope::TypeDependence;
> +static constexpr unsigned TypeDependenceBits = 4;
> +
> +#define
> LLVM_COMMON_DEPENDENCE(NAME) 
>   \
> +  struct NAME##Scope
> { \
> +enum NAME : uint8_t
> {  \

[PATCH] D75604: [clangd] Round WorkDoneProgressBegin.percentage down

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev abandoned this revision.
kbobyrev marked an inline comment as done.
kbobyrev added inline comments.



Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:1409
   (Stats.Enqueued - Stats.LastIdle);
+  // Round down to 2 decimal places for readability.
+  Report.percentage = std::floor(*Report.percentage * 100.0) / 100.0;

sammccall wrote:
> Yikes, rounding a float value in the protocol is pretty weird and would 
> deserve more of a comment.
> 
> Also this seems like a fairly... improbable bug, in that it's trivial to fix 
> in the editor and would usually be visible if the implementer tried the 
> feature even once.
> 
> Which editors specifically are you seeing bad display in? Can we just send 
> them a PR?
Okay, makes sense. I saw this in coc.nvim and I can just fix it there, but I 
thought it might also happen in other editors. I also thought we do not care 
too much about precision here, but I'm OK with fixing it on the editor side.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75604



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

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

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

Fix the test case.




Comment at: 
clang-tools-extra/clang-tidy/objc/MethodUnavailableNotOverrideCheck.cpp:29
+// intended to be matched here.
+AST_MATCHER(ObjCMethodDecl, isUnavailableMethodNotOverriding) {
+  return !Node.isOverriding() && Node.hasAttr();

Should probably be in an anonymous namespace this as you don't intend it to be 
visible in another translation unit.



Comment at: 
clang-tools-extra/clang-tidy/objc/MethodUnavailableNotOverrideCheck.cpp:34
+void MethodUnavailableNotOverrideCheck::registerMatchers(MatchFinder *Finder) {
+  if (!getLangOpts().ObjC)
+return;

See https://reviews.llvm.org/D75340



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/objc-method-unavailable-not-override.m:34
+// Verify check when using a macro that expands to the unavailable attribute.
+- (void)methodC NS_UNAVAILABLE;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: method 'methodC' is marked 
unavailable but does not override a superclass method 
[objc-method-unavailable-not-override]

Generally we are cautious about modifying MACRO usages in clang_tidy as we 
don't know if its definition can change based on configuration, perhaps its 
safer to just warn instead of providing a fix it



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/objc-method-unavailable-not-override.m:51
+// Verify that fixes exist to delete entire method declarations:
+// CHECK-FIXES: {{^\s*$}}

This is not a satisfactory check, it will ensure at least one method has been 
deleted but it wont ensure all methods have been deleted. Would probably be 
safer putting a unique comment on the line and having a check fix that checks 
for an empty line and the comment for each case.


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

[clang-tools-extra] 5abfe64 - [clangd] Fix test (it worked by coincidence before)

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2020-03-05T10:11:55+01:00
New Revision: 5abfe646f5e194bb2330b80c7f0e23fba00e30fe

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

LOG: [clangd] Fix test (it worked by coincidence before)

Added: 


Modified: 
clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp 
b/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
index 6ca6e5479677..c9569d2d96cc 100644
--- a/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
+++ b/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
@@ -466,7 +466,7 @@ TEST_F(TUSchedulerTests, ManyUpdates) {
 auto Inputs = getInputs(File, Contents.str());
 {
   WithContextValue WithNonce(NonceKey, ++Nonce);
-  Inputs.Version = Nonce;
+  Inputs.Version = std::to_string(Nonce);
   updateWithDiags(
   S, File, Inputs, WantDiagnostics::Auto,
   [File, Nonce, &Mut, &TotalUpdates](std::vector) {



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

[PATCH] D74669: [clang-tidy] New check: bugprone-suspicious-include

[Nathan James via Phabricator via cfe-commits]

njames93 added inline comments.



Comment at: clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h:38
+/// extensions.
+inline StringRef defaultHeaderFileExtensions() { return ",h,hh,hpp,hxx"; }
+

lebedev.ri wrote:
> njames93 wrote:
> > A lot of configuration options for clang tidy use semi-colon `;` seperated 
> > lists. Would it not be wise to carry on the convention and use semi colon 
> > here (and below) as well. It could break a few peoples configuration but 
> > that would be realised  fairly quickly and updated
> I think i'm missing the point of D75621.
> Why can't we keep using `,` as delimitter?
> Why do we have to change everything, introduce inconsistency
> (is it still documented that `,` is still valid?),
> and spam into terminal if `,` is found?
This reason
```
➜  ~ clang-tidy test.cpp --config="{CheckOptions: [{ key: HeaderFileExtensions, 
value: h,,hpp,hxx }]}" -- -std=c++17
YAML:1:59: error: unknown key 'hpp'
{CheckOptions: [{ key: HeaderFileExtensions, value: h,,hpp,hxx }]}
  ^
Error: invalid configuration specified.
Invalid argument
➜  ~ clang-tidy test.cpp --config="{CheckOptions: [{ key: HeaderFileExtensions, 
value: h;;hpp;hxx }]}" -- -std=c++17
➜  ~ 

```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74669



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

Just to give an example:

  int foo(int* restrict *pA, int* restrict *pB) {
int tmp=**pB;
**pA=42;
return tmp - **pB; // **pA and **pB can refer to the same objects
  }

This gives following llvm-ir code with the 'full noalias' clang version (after 
optimizations):

  ; Function Attrs: nofree nounwind uwtable
  define dso_local i32 @foo(i32** nocapture %pA, i32** nocapture readonly %pB) 
local_unnamed_addr #0 !noalias !2 {
  entry:
%0 = load i32*, i32** %pB, noalias_sidechannel i32** undef, align 8, !tbaa 
!5, !noalias !2
%1 = tail call i32* @llvm.side.noalias.p0i32.p0i8.p0p0i32.p0p0i32.i64(i32* 
%0, i8* null, i32** %pB, i32** undef, i64 0, metadata !2), !tbaa !5, !noalias !2
%2 = load i32, i32* %0, noalias_sidechannel i32* %1, align 4, !tbaa !9, 
!noalias !2
%3 = load i32*, i32** %pA, noalias_sidechannel i32** undef, align 8, !tbaa 
!5, !noalias !2
%4 = tail call i32* @llvm.side.noalias.p0i32.p0i8.p0p0i32.p0p0i32.i64(i32* 
%3, i8* null, i32** %pA, i32** undef, i64 0, metadata !2), !tbaa !5, !noalias !2
store i32 42, i32* %3, noalias_sidechannel i32* %4, align 4, !tbaa !9, 
!noalias !2
%5 = load i32, i32* %0, noalias_sidechannel i32* %1, align 4, !tbaa !9, 
!noalias !2
%sub = sub nsw i32 %2, %5
ret i32 %sub
  }

When adding 'noalias' to the arguments (my understanding is that this  is what 
the attributer would deduce, as pA and pB are only read):

  ; Function Attrs: nofree nounwind uwtable
  define dso_local i32 @foo(i32** noalias nocapture %pA, i32** noalias 
nocapture readonly %pB) local_unnamed_addr #0 !noalias !2 {
  entry:
%0 = load i32*, i32** %pB, noalias_sidechannel i32** undef, align 8, !tbaa 
!5, !noalias !2
%1 = tail call i32* @llvm.side.noalias.p0i32.p0i8.p0p0i32.p0p0i32.i64(i32* 
%0, i8* null, i32** %pB, i32** undef, i64 0, metadata !2), !tbaa !5, !noalias !2
%2 = load i32, i32* %0, noalias_sidechannel i32* %1, align 4, !tbaa !9, 
!noalias !2
%3 = load i32*, i32** %pA, noalias_sidechannel i32** undef, align 8, !tbaa 
!5, !noalias !2
%4 = tail call i32* @llvm.side.noalias.p0i32.p0i8.p0p0i32.p0p0i32.i64(i32* 
%3, i8* null, i32** %pA, i32** undef, i64 0, metadata !2), !tbaa !5, !noalias !2
store i32 42, i32* %3, noalias_sidechannel i32* %4, align 4, !tbaa !9, 
!noalias !2
%5 = load i32, i32* %0, noalias_sidechannel i32* %1, align 4, !tbaa !9, 
!noalias !2
%sub = sub nsw i32 %2, %5
ret i32 %sub
  }

and reoptimizing, we now get:

  ; Function Attrs: nofree nounwind uwtable
  define dso_local i32 @foo(i32** noalias nocapture %pA, i32** noalias 
nocapture readonly %pB) local_unnamed_addr #0 !noalias !2 {
  entry:
%0 = load i32*, i32** %pA, noalias_sidechannel i32** undef, align 8, !tbaa 
!5, !noalias !2
%1 = tail call i32* @llvm.side.noalias.p0i32.p0i8.p0p0i32.p0p0i32.i64(i32* 
%0, i8* null, i32** %pA, i32** undef, i64 0, metadata !2), !tbaa !5, !noalias !2
store i32 42, i32* %0, noalias_sidechannel i32* %1, align 4, !tbaa !9, 
!noalias !2
ret i32 0
  }

'%sub' is optimized away because the now the alias analyzer deduces that '%pA' 
and '%pB' cannot refer to the same objects. This implies (because of restrict) 
that '*%pA' and '*%pB' cannot refer to the same objects and because of that, 
the 'store i32' could not have interfered with the 'load i32'.


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] D75665: [analyzer] On-demand parsing capability for CTU

[Endre Fülöp via Phabricator via cfe-commits]

gamesh411 created this revision.
gamesh411 added reviewers: martong, balazske.
Herald added subscribers: cfe-commits, steakhal, Charusso, dkrupp, donat.nagy, 
Szelethus, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, 
xazax.hun, whisperity, mgorny.
Herald added a reviewer: Szelethus.
Herald added a project: clang.

Add an option to enable on-demand parsing of needed ASTs during CTU analysis.
Two options are introduced. CTUOnDemandParsing enables the feature, and
CTUOnDemandParsingDatabase specifies the path to a compilation database, which
has all the necessary information to generate the ASTs. In case an empty
string is given, on-the-fly parsing is disabled.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D75665

Files:
  clang/include/clang/CrossTU/CrossTranslationUnit.h
  clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
  clang/lib/CrossTU/CMakeLists.txt
  clang/lib/CrossTU/CrossTranslationUnit.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/StaticAnalyzer/Core/CallEvent.cpp
  clang/test/Analysis/Inputs/ctu-other.c.externalDefMap.ast-dump.txt
  clang/test/Analysis/Inputs/ctu-other.c.externalDefMap.on-the-fly.txt
  clang/test/Analysis/Inputs/ctu-other.c.externalDefMap.txt
  clang/test/Analysis/Inputs/ctu-other.cpp.externalDefMap.ast-dump.txt
  clang/test/Analysis/Inputs/ctu-other.cpp.externalDefMap.on-the-fly.txt
  clang/test/Analysis/Inputs/ctu-other.cpp.externalDefMap.txt
  clang/test/Analysis/analyzer-config.c
  clang/test/Analysis/ctu-different-triples.cpp
  clang/test/Analysis/ctu-main.c
  clang/test/Analysis/ctu-main.cpp
  clang/test/Analysis/ctu-on-demand-parsing.c
  clang/test/Analysis/ctu-on-demand-parsing.cpp
  clang/test/Analysis/ctu-unknown-parts-in-triples.cpp
  clang/unittests/CrossTU/CrossTranslationUnitTest.cpp

Index: clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
===
--- clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
+++ clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
@@ -7,10 +7,11 @@
 //===--===//
 
 #include "clang/CrossTU/CrossTranslationUnit.h"
-#include "clang/Frontend/CompilerInstance.h"
 #include "clang/AST/ASTConsumer.h"
+#include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/FrontendAction.h"
 #include "clang/Tooling/Tooling.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/ToolOutputFile.h"
@@ -162,7 +163,7 @@
   IndexFile.os().flush();
   EXPECT_TRUE(llvm::sys::fs::exists(IndexFileName));
   llvm::Expected> IndexOrErr =
-  parseCrossTUIndex(IndexFileName, "");
+  parseCrossTUIndex(IndexFileName);
   EXPECT_TRUE((bool)IndexOrErr);
   llvm::StringMap ParsedIndex = IndexOrErr.get();
   for (const auto &E : Index) {
@@ -173,25 +174,5 @@
 EXPECT_TRUE(Index.count(E.getKey()));
 }
 
-TEST(CrossTranslationUnit, CTUDirIsHandledCorrectly) {
-  llvm::StringMap Index;
-  Index["a"] = "/b/c/d";
-  std::string IndexText = createCrossTUIndexString(Index);
-
-  int IndexFD;
-  llvm::SmallString<256> IndexFileName;
-  ASSERT_FALSE(llvm::sys::fs::createTemporaryFile("index", "txt", IndexFD,
-  IndexFileName));
-  llvm::ToolOutputFile IndexFile(IndexFileName, IndexFD);
-  IndexFile.os() << IndexText;
-  IndexFile.os().flush();
-  EXPECT_TRUE(llvm::sys::fs::exists(IndexFileName));
-  llvm::Expected> IndexOrErr =
-  parseCrossTUIndex(IndexFileName, "/ctudir");
-  EXPECT_TRUE((bool)IndexOrErr);
-  llvm::StringMap ParsedIndex = IndexOrErr.get();
-  EXPECT_EQ(ParsedIndex["a"], "/ctudir/b/c/d");
-}
-
 } // end namespace cross_tu
 } // end namespace clang
Index: clang/test/Analysis/ctu-unknown-parts-in-triples.cpp
===
--- clang/test/Analysis/ctu-unknown-parts-in-triples.cpp
+++ clang/test/Analysis/ctu-unknown-parts-in-triples.cpp
@@ -5,7 +5,7 @@
 // RUN: mkdir -p %t/ctudir
 // RUN: %clang_cc1 -std=c++14 -triple x86_64-pc-linux-gnu \
 // RUN:   -emit-pch -o %t/ctudir/ctu-other.cpp.ast %S/Inputs/ctu-other.cpp
-// RUN: cp %S/Inputs/ctu-other.cpp.externalDefMap.txt %t/ctudir/externalDefMap.txt
+// RUN: cp %S/Inputs/ctu-other.cpp.externalDefMap.ast-dump.txt %t/ctudir/externalDefMap.txt
 // RUN: %clang_analyze_cc1 -std=c++14 -triple x86_64-unknown-linux-gnu \
 // RUN:   -analyzer-checker=core,debug.ExprInspection \
 // RUN:   -analyzer-config experimental-enable-naive-ctu-analysis=true \
Index: clang/test/Analysis/ctu-on-demand-parsing.cpp
===
--- /dev/null
+++ clang/test/Analysis/ctu-on-demand-parsing.cpp
@@ -0,0 +1,100 @@
+// RUN: rm -rf %t && mkdir %t
+// RUN: mkdir -p %t/ctudir
+// RUN: cp %S/Inputs/ctu-chain.cpp %t/ctudir/ctu-chain.cpp
+// RUN: echo '[{"directory":"%S/Inputs","command":"clang++ -c ctu-chain.cpp","f

[PATCH] D74669: [clang-tidy] New check: bugprone-suspicious-include

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.



Comment at: clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h:38
+/// extensions.
+inline StringRef defaultHeaderFileExtensions() { return ",h,hh,hpp,hxx"; }
+

njames93 wrote:
> lebedev.ri wrote:
> > njames93 wrote:
> > > A lot of configuration options for clang tidy use semi-colon `;` 
> > > seperated lists. Would it not be wise to carry on the convention and use 
> > > semi colon here (and below) as well. It could break a few peoples 
> > > configuration but that would be realised  fairly quickly and updated
> > I think i'm missing the point of D75621.
> > Why can't we keep using `,` as delimitter?
> > Why do we have to change everything, introduce inconsistency
> > (is it still documented that `,` is still valid?),
> > and spam into terminal if `,` is found?
> This reason
> ```
> ➜  ~ clang-tidy test.cpp --config="{CheckOptions: [{ key: 
> HeaderFileExtensions, value: h,,hpp,hxx }]}" -- -std=c++17
> YAML:1:59: error: unknown key 'hpp'
> {CheckOptions: [{ key: HeaderFileExtensions, value: h,,hpp,hxx }]}
>   ^
> Error: invalid configuration specified.
> Invalid argument
> ➜  ~ clang-tidy test.cpp --config="{CheckOptions: [{ key: 
> HeaderFileExtensions, value: h;;hpp;hxx }]}" -- -std=c++17
> ➜  ~ 
> 
> ```
```
$ clang-tidy test.cpp --config="{CheckOptions: [{ key: HeaderFileExtensions, 
value: h,,hpp,hxx }]}" -- -std=c++17
YAML:1:59: error: unknown key 'hpp'
{CheckOptions: [{ key: HeaderFileExtensions, value: h,,hpp,hxx }]}
  ^
Error: invalid configuration specified.
Invalid argument
$ clang-tidy test.cpp --config="{CheckOptions: [{ key: HeaderFileExtensions, 
value: \"h,,hpp,hxx\" }]}" -- -std=c++17
Error while processing /repositories/llvm-project/test.cpp.
error: no input files [clang-diagnostic-error]
error: no such file or directory: '/repositories/llvm-project/test.cpp' 
[clang-diagnostic-error]
error: unable to handle compilation, expected exactly one compiler job in '' 
[clang-diagnostic-error]
Found compiler error(s).

```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74669



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

[clang] cd1dc7f - [AST] Replace assert with llvm_unreachable to silence compiler warning

[Mikael Holmen via cfe-commits]

Author: Mikael Holmen
Date: 2020-03-05T11:07:54+01:00
New Revision: cd1dc7f15d637b42067546e658574237cd0f0d46

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

LOG: [AST] Replace assert with llvm_unreachable to silence compiler warning

New code added in ec3060c72de6 looked like

+  case TemplateName::NameKind::OverloadedTemplate:
+assert(false && "overloaded templates shouldn't survive to here.");
+  default:

If compiling without asserts we then got a warning about unannotated
fallthrough from the case into the default.

Change the assert into an llvm_unreachable to silence the warning.

Added: 


Modified: 
clang/lib/AST/TemplateName.cpp

Removed: 




diff  --git a/clang/lib/AST/TemplateName.cpp b/clang/lib/AST/TemplateName.cpp
index afabc575b164..3b8ae06c6339 100644
--- a/clang/lib/AST/TemplateName.cpp
+++ b/clang/lib/AST/TemplateName.cpp
@@ -185,7 +185,7 @@ TemplateNameDependence TemplateName::getDependence() const {
 D |= TemplateNameDependence::UnexpandedPack;
 break;
   case TemplateName::NameKind::OverloadedTemplate:
-assert(false && "overloaded templates shouldn't survive to here.");
+llvm_unreachable("overloaded templates shouldn't survive to here.");
   default:
 break;
   }



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

[PATCH] D74616: [ARM] Setting missing isLaneQ attribute on Neon Intrisics definitions

[Diogo N. Sampaio via Phabricator via cfe-commits]

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

LGTM with a nit: we can save some space using sintax like this:

  let isLaneQ = 1 in
  def UDOT_LANEQ : SOpInst<"vdot_laneq", "..(<<)(<;

or concatenating those that are just one after the other:

  let isLaneQ = 1 in {
def VFMLAL_LANEQ_LOW  : SOpInst<"vfmlal_laneq_low",  "(F>)(F>)F(FQ)I", 
"hQh", OP_FMLAL_LN>;
 def VFMLSL_LANEQ_LOW  : SOpInst<"vfmlsl_laneq_low",  "(F>)(F>)F(FQ)I", 
"hQh", OP_FMLSL_LN>;
 def VFMLAL_LANEQ_HIGH : SOpInst<"vfmlal_laneq_high", "(F>)(F>)F(FQ)I", 
"hQh", OP_FMLAL_LN_Hi>;
 def VFMLSL_LANEQ_HIGH : SOpInst<"vfmlsl_laneq_high", "(F>)(F>)F(FQ)I", 
"hQh", OP_FMLSL_LN_Hi>;
  }


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74616



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

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

[Endre Fülöp via Phabricator via cfe-commits]

gamesh411 added a comment.

This patch adds an alternative way of loading ASTs to provide the CTU 
definitions needed during analysis.
The additional approach is to use Tooling with a user-provided 
compile_commands.json,
and look up the needed file from the output of the ext-def-mapping tool (which 
is now used for both approaches).

As the CTUDir string prefix is only needed in case of the old approach, I have 
refactored the external definition mapping storage to NOT include that.

Experience with diagnostics generated by the on-demand approach shows that 
location information inside plists contain file paths as they were in the 
compile_commands.json.
I would say, a uniform handling of paths is desirable, so an ArgAdjuster is 
added to the Clang Tool which implements the path normalization step.

The following test still fails. As far as I can tell, this is due to the C 
language being laxer. The concrete failure:

   TEST 'Clang :: Analysis/ctu-on-demand-parsing.c' FAILED 

  Script:
  --
  : 'RUN: at line 1';   rm -rf 
/home/gamesh411/projects/clang-rwd/tools/clang/test/Analysis/Output/ctu-on-demand-parsing.c.tmp
 && mkdir 
/home/gamesh411/projects/clang-rwd/tools/clang/test/Analysis/Output/ctu-on-demand-parsing.c.tmp
  : 'RUN: at line 2';   mkdir -p 
/home/gamesh411/projects/clang-rwd/tools/clang/test/Analysis/Output/ctu-on-demand-parsing.c.tmp/ctudir2
  : 'RUN: at line 3';   echo 
'[{"directory":"/home/gamesh411/projects/llvm-project/clang/test/Analysis/Inputs","command":"clang
 -x c -std=c89 -c ctu-other.c","file":"ctu-other.c"}]' | sed -e 's/\\//g' > 
/home/gamesh411/projects/clang-rwd/tools/clang/test/Analysis/Output/ctu-on-demand-parsing.c.tmp/ctudir2/compile_commands.json
  : 'RUN: at line 4';   
/home/gamesh411/projects/clang-rwd/bin/clang-extdef-mapping 
/home/gamesh411/projects/llvm-project/clang/test/Analysis/Inputs/ctu-other.c > 
/home/gamesh411/projects/clang-rwd/tools/clang/test/Analysis/Output/ctu-on-demand-parsing.c.tmp/ctudir2/externalDefMap.txt
  : 'RUN: at line 5';   /home/gamesh411/projects/clang-rwd/bin/clang -cc1 
-internal-isystem /home/gamesh411/projects/clang-rwd/lib/clang/11.0.0/include 
-nostdsysteminc -triple x86_64-pc-linux-gnu -fsyntax-only -x c -std=c89 
-analyze-analyzer-checker=core,debug.ExprInspection-analyzer-config 
experimental-enable-naive-ctu-analysis=true-analyzer-config 
ctu-dir=/home/gamesh411/projects/clang-rwd/tools/clang/test/Analysis/Output/ctu-on-demand-parsing.c.tmp/ctudir2
-analyzer-config ctu-on-demand-parsing=true-analyzer-config 
ctu-on-demand-parsing-database="/home/gamesh411/projects/clang-rwd/tools/clang/test/Analysis/Output/ctu-on-demand-parsing.c.tmp/ctudir2/compile_commands.json"
-verify 
/home/gamesh411/projects/llvm-project/clang/test/Analysis/ctu-on-demand-parsing.c
  --
  Exit Code: 1
  
  Command Output (stderr):
  --
  warning: treating 'c' input as 'c++' when in C++ mode, this behavior is 
deprecated [-Wdeprecated]
  warning: unknown warning option '-Wno-maybe-uninitialized'; did you mean 
'-Wno-uninitialized'? [-Wunknown-warning-option]
  
/home/gamesh411/projects/llvm-project/clang/test/Analysis/Inputs/ctu-other.c:47:26:
 error: 'DataType' cannot be defined in a parameter type
  int structInProto(struct DataType {int a;int b; } * d) {
   ^
  1 warning and 1 error generated.
  Error while processing 
/home/gamesh411/projects/llvm-project/clang/test/Analysis/Inputs/ctu-other.c.

Would it be ok to remove those tests? Currently, I see no solution for solving 
this test case, any insight is welcome.




Comment at: clang/unittests/CrossTU/CrossTranslationUnitTest.cpp:176
 
-TEST(CrossTranslationUnit, CTUDirIsHandledCorrectly) {
-  llvm::StringMap Index;

CTUDir is no longer prepended to the mapping by default, as on-demand parsing 
does not use the CTUDir prefix.


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

[clang] 737394c - Revert "clang: Treat ieee mode as the default for denormal-fp-math"

[Jeremy Morse via cfe-commits]

Author: Jeremy Morse
Date: 2020-03-05T10:55:24Z
New Revision: 737394c490444e968a6f640b99a6614567ca7f28

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

LOG: Revert "clang: Treat ieee mode as the default for denormal-fp-math"

This reverts commit c64ca93053af235bac0ca4dcdcd21c8882478310.

This patch tripped a few build bots:

  http://lab.llvm.org:8011/builders/clang-x86_64-debian-fast/builds/24703/
  http://lab.llvm.org:8011/builders/clang-cmake-x86_64-avx2-linux/builds/13465/
  http://lab.llvm.org:8011/builders/clang-with-lto-ubuntu/builds/15994/

Reverting to clear the bots.

Added: 


Modified: 
clang/include/clang/Basic/CodeGenOptions.h
clang/include/clang/Driver/ToolChain.h
clang/lib/Driver/ToolChains/Clang.cpp
clang/test/CodeGenCUDA/flush-denormals.cu
clang/test/CodeGenCUDA/propagate-metadata.cu
clang/test/CodeGenOpenCL/amdgpu-features.cl
clang/test/Driver/cuda-flush-denormals-to-zero.cu
clang/test/Driver/denormal-fp-math.c
clang/test/Driver/fp-model.c

Removed: 




diff  --git a/clang/include/clang/Basic/CodeGenOptions.h 
b/clang/include/clang/Basic/CodeGenOptions.h
index d435bebcdcf9..0a28edefa1e6 100644
--- a/clang/include/clang/Basic/CodeGenOptions.h
+++ b/clang/include/clang/Basic/CodeGenOptions.h
@@ -164,10 +164,10 @@ class CodeGenOptions : public CodeGenOptionsBase {
   std::string FloatABI;
 
   /// The floating-point denormal mode to use.
-  llvm::DenormalMode FPDenormalMode = llvm::DenormalMode::getIEEE();
+  llvm::DenormalMode FPDenormalMode;
 
-  /// The floating-point denormal mode to use, for float.
-  llvm::DenormalMode FP32DenormalMode = llvm::DenormalMode::getIEEE();
+  /// The floating-point subnormal mode to use, for float.
+  llvm::DenormalMode FP32DenormalMode;
 
   /// The float precision limit to use, if non-empty.
   std::string LimitFloatPrecision;

diff  --git a/clang/include/clang/Driver/ToolChain.h 
b/clang/include/clang/Driver/ToolChain.h
index 88ce205228fd..400ff9d86664 100644
--- a/clang/include/clang/Driver/ToolChain.h
+++ b/clang/include/clang/Driver/ToolChain.h
@@ -623,7 +623,8 @@ class ToolChain {
   const llvm::opt::ArgList &DriverArgs,
   Action::OffloadKind DeviceOffloadKind,
   const llvm::fltSemantics *FPType = nullptr) const {
-return llvm::DenormalMode::getIEEE();
+// FIXME: This should be IEEE when default handling is fixed.
+return llvm::DenormalMode::getInvalid();
   }
 };
 

diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index c8da2d86490f..d387a1dc2079 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -2548,13 +2548,8 @@ static void RenderFloatingPointOptions(const ToolChain 
&TC, const Driver &D,
   ReciprocalMath = false;
   SignedZeros = true;
   // -fno_fast_math restores default denormal and fpcontract handling
-  FPContract = "";
   DenormalFPMath = DefaultDenormalFPMath;
-
-  // FIXME: The target may have picked a non-IEEE default mode here based 
on
-  // -cl-denorms-are-zero. Should the target consider -fp-model 
interaction?
-  DenormalFP32Math = DefaultDenormalFP32Math;
-
+  FPContract = "";
   StringRef Val = A->getValue();
   if (OFastEnabled && !Val.equals("fast")) {
   // Only -ffp-model=fast is compatible with OFast, ignore.
@@ -2731,9 +2726,7 @@ static void RenderFloatingPointOptions(const ToolChain 
&TC, const Driver &D,
   FPExceptionBehavior = "strict";
   // -fno_unsafe_math_optimizations restores default denormal handling
   DenormalFPMath = DefaultDenormalFPMath;
-
-  // The target may have opted to flush just f32 by default, so force IEEE.
-  DenormalFP32Math = llvm::DenormalMode::getIEEE();
+  DenormalFP32Math = DefaultDenormalFP32Math;
   break;
 
 case options::OPT_Ofast:
@@ -2774,12 +2767,11 @@ static void RenderFloatingPointOptions(const ToolChain 
&TC, const Driver &D,
 if (StrictFPModel) {
   // If -ffp-model=strict has been specified on command line but
   // subsequent options conflict then emit warning diagnostic.
+  // TODO: How should this interact with DenormalFP32Math?
   if (HonorINFs && HonorNaNs &&
 !AssociativeMath && !ReciprocalMath &&
 SignedZeros && TrappingMath && RoundingFPMath &&
-(FPContract.equals("off") || FPContract.empty()) &&
-DenormalFPMath == llvm::DenormalMode::getIEEE() &&
-DenormalFP32Math == llvm::DenormalMode::getIEEE())
+(FPContract.equals("off") || FPContract.empty()))
 // OK: Current Arg doesn't conflict with -ffp-model=strict
 ;
   else {
@@ -2833,8 +2825,7 @@ static void RenderFloatingPointOptions(const ToolChain 
&TC, const Driver &D,
 Cm

Re: [clang] c64ca93 - clang: Treat ieee mode as the default for denormal-fp-math

[Jeremy Morse via cfe-commits]

Hi Matt,

FYI several build bots tripped on this commit:

  http://lab.llvm.org:8011/builders/clang-x86_64-debian-fast/builds/24703/
  
http://lab.llvm.org:8011/builders/clang-cmake-x86_64-avx2-linux/builds/13465/
  http://lab.llvm.org:8011/builders/clang-with-lto-ubuntu/builds/15994/

I've reverted the commit in 737394c490444e968a6f640b99a6614567ca7f28
to clear the bots.

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

[PATCH] D75603: [clangd] Add instrumentation mode in clangd for metrics collection.

[UTKARSH SAXENA via Phabricator via cfe-commits]

usaxena95 updated this revision to Diff 248416.
usaxena95 marked 5 inline comments as done.
usaxena95 added a comment.

Addressed comments.

- Populated score in CodeCompletion before invoking the callback.
- Tested that CodeCompletion is scored
- Updated comment for callback.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75603

Files:
  clang-tools-extra/clangd/CodeComplete.cpp
  clang-tools-extra/clangd/CodeComplete.h
  clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp

Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -29,7 +29,9 @@
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include 
+#include 
 #include 
+#include 
 
 namespace clang {
 namespace clangd {
@@ -69,6 +71,7 @@
 MATCHER(InsertInclude, "") {
   return !arg.Includes.empty() && bool(arg.Includes[0].Insertion);
 }
+MATCHER(Scored, "") { return arg.Score.Total > 0; }
 MATCHER_P(SnippetSuffix, Text, "") { return arg.SnippetSuffix == Text; }
 MATCHER_P(Origin, OriginSet, "") { return arg.Origin == OriginSet; }
 MATCHER_P(Signature, S, "") { return arg.Signature == S; }
@@ -1041,6 +1044,21 @@
   EXPECT_THAT(Completions, Contains(Named("TT")));
 }
 
+TEST(CompletionTest, EnableInstrumentationMode) {
+  std::vector RecordedCompletions;
+  CodeCompleteOptions Opts;
+  Opts.RecordCCResult = [&RecordedCompletions](const CodeCompletion &CC,
+   const SymbolQualitySignals &,
+   const SymbolRelevanceSignals &) {
+RecordedCompletions.push_back(CC);
+  };
+
+  completions("int xy1, xy2; int a = xy^", /*IndexSymbols=*/{}, Opts);
+  EXPECT_THAT(RecordedCompletions,
+  UnorderedElementsAre(AllOf(Named("xy1"), Scored()),
+   AllOf(Named("xy2"), Scored(;
+}
+
 SignatureHelp signatures(llvm::StringRef Text, Position Point,
  std::vector IndexSymbols = {}) {
   std::unique_ptr Index;
Index: clang-tools-extra/clangd/CodeComplete.h
===
--- clang-tools-extra/clangd/CodeComplete.h
+++ clang-tools-extra/clangd/CodeComplete.h
@@ -19,6 +19,7 @@
 #include "Logger.h"
 #include "Path.h"
 #include "Protocol.h"
+#include "Quality.h"
 #include "index/Index.h"
 #include "index/Symbol.h"
 #include "index/SymbolOrigin.h"
@@ -29,12 +30,14 @@
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Error.h"
+#include 
 #include 
 
 namespace clang {
 class NamedDecl;
 namespace clangd {
 struct PreambleData;
+struct CodeCompletion;
 
 struct CodeCompleteOptions {
   /// Returns options that can be passed to clang's completion engine.
@@ -129,6 +132,16 @@
 /// Always use text-based completion.
 NeverParse,
   } RunParser = ParseIfReady;
+
+  /// Callback invoked on all CompletionCandidate after they are scored and
+  /// before they are ranked (by -Score). Thus the results are yielded in
+  /// arbitrary order. CodeCompletion also contains the computed Score.
+  ///
+  /// This callbacks allows capturing various internal structures used by clangd
+  /// during code completion. Eg: Symbol quality and relevance signals.
+  std::function
+  RecordCCResult;
 };
 
 // Semi-structured representation of a code-complete suggestion for our C++ API.
Index: clang-tools-extra/clangd/CodeComplete.cpp
===
--- clang-tools-extra/clangd/CodeComplete.cpp
+++ clang-tools-extra/clangd/CodeComplete.cpp
@@ -1660,6 +1660,13 @@
? Scores.Total / Relevance.NameMatch
: Scores.Quality;
 
+if (Opts.RecordCCResult) {
+  CodeCompletion Completion = toCodeCompletion(Bundle);
+  Completion.Score = Scores;
+  Completion.CompletionTokenRange = ReplacedRange;
+  Opts.RecordCCResult(Completion, Quality, Relevance);
+}
+
 dlog("CodeComplete: {0} ({1}) = {2}\n{3}{4}\n", First.Name,
  llvm::to_string(Origin), Scores.Total, llvm::to_string(Quality),
  llvm::to_string(Relevance));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D75603: [clangd] Add instrumentation mode in clangd for metrics collection.

[UTKARSH SAXENA via Phabricator via cfe-commits]

usaxena95 added inline comments.



Comment at: clang-tools-extra/clangd/CodeComplete.h:139
+  /// in the output.
+  std::function *RecordCCResult = 
nullptr;

sammccall wrote:
> I'd suggest including the final score in the signature rather than recompute 
> it, just so the contract is really clear and simple (results yielded in 
> arbitrary order, will be ranked by -score). Please spell this out.
Couldn't add it to the signature since inner classes cannot be forward declared.
Since `CodeCompletion` contains the `Score`, I have populated this field in the 
`CodeCompletion` (and also `CompletionTokenRange`) as done in 
`toCodeCompleteResult` to be consistent.

Also tested that the CodeCompletion is scored.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75603



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

[PATCH] D74617: [ARM] Keeping sign information on bitcasts for Neon vdot_lane intrinsics

[Diogo N. Sampaio via Phabricator via cfe-commits]

dnsampaio added a comment.

Is this missing a test?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74617



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

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

LGTM! Lets have a link to the original discussion: D75163 



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

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

Szelethus added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:64-70
+/// 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);
+}

balazske wrote:
> Szelethus wrote:
> > You could make this a method of `FnDescription`.
> Originally I wanted it too but there is no strong relation to it, somehow I 
> like better to have it as separate function.
Okay, I'll leave that to you.


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

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

Szelethus added a comment.

Wouldn't it be better just to upload this diff to D75163 
 by the way? It feels like we're discarding 
much of the discussion.


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

[clang-tools-extra] e397a0a - [clangd] Add instrumentation mode in clangd for metrics collection.

[Utkarsh Saxena via cfe-commits]

Author: Utkarsh Saxena
Date: 2020-03-05T12:39:15+01:00
New Revision: e397a0a5c3c0fa1912fbac23b550fa7d239196ba

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

LOG: [clangd] Add instrumentation mode in clangd for metrics collection.

Summary:
This patch adds an instrumentation mode for clangd (enabled by
corresponding option in cc_opts).
If this mode is enabled then user can specify callbacks to run on the
final code completion result.

Moreover the CodeCompletion::Score will contain the detailed Quality and
Relevance signals used to compute the score when this mode is enabled.
These are required because we do not any place in which the final
candidates (scored and sorted) are available along with the above
signals. The signals are temporary structures in `addCandidate`.

The callback is needed as it gives access to many data structures that
are internal to CodeCompleteFlow and are available once Sema has run. Eg:
ScopeDistnace and FileDistance.

If this mode is disabled (as in default) then Score would just contain 2
shared pointers (null). Thus cost(memory/time) increase for the default
mode would be fairly cheap and insignificant.

Reviewers: sammccall

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

Tags: #clang

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

Added: 


Modified: 
clang-tools-extra/clangd/CodeComplete.cpp
clang-tools-extra/clangd/CodeComplete.h
clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/CodeComplete.cpp 
b/clang-tools-extra/clangd/CodeComplete.cpp
index 3fbf98970cce..b2e97729ee6d 100644
--- a/clang-tools-extra/clangd/CodeComplete.cpp
+++ b/clang-tools-extra/clangd/CodeComplete.cpp
@@ -1660,6 +1660,10 @@ class CodeCompleteFlow {
? Scores.Total / Relevance.NameMatch
: Scores.Quality;
 
+if (Opts.RecordCCResult)
+  Opts.RecordCCResult(toCodeCompletion(Bundle), Quality, Relevance,
+  Scores.Total);
+
 dlog("CodeComplete: {0} ({1}) = {2}\n{3}{4}\n", First.Name,
  llvm::to_string(Origin), Scores.Total, llvm::to_string(Quality),
  llvm::to_string(Relevance));

diff  --git a/clang-tools-extra/clangd/CodeComplete.h 
b/clang-tools-extra/clangd/CodeComplete.h
index 3b3dca3ee8c5..df06c156049f 100644
--- a/clang-tools-extra/clangd/CodeComplete.h
+++ b/clang-tools-extra/clangd/CodeComplete.h
@@ -19,6 +19,7 @@
 #include "Logger.h"
 #include "Path.h"
 #include "Protocol.h"
+#include "Quality.h"
 #include "index/Index.h"
 #include "index/Symbol.h"
 #include "index/SymbolOrigin.h"
@@ -29,12 +30,14 @@
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Error.h"
+#include 
 #include 
 
 namespace clang {
 class NamedDecl;
 namespace clangd {
 struct PreambleData;
+struct CodeCompletion;
 
 struct CodeCompleteOptions {
   /// Returns options that can be passed to clang's completion engine.
@@ -129,6 +132,16 @@ struct CodeCompleteOptions {
 /// Always use text-based completion.
 NeverParse,
   } RunParser = ParseIfReady;
+
+  /// Callback invoked on all CompletionCandidate after they are scored and
+  /// before they are ranked (by -Score). Thus the results are yielded in
+  /// arbitrary order.
+  ///
+  /// This callbacks allows capturing various internal structures used by 
clangd
+  /// during code completion. Eg: Symbol quality and relevance signals.
+  std::function
+  RecordCCResult;
 };
 
 // Semi-structured representation of a code-complete suggestion for our C++ 
API.

diff  --git a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp 
b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
index e61dca9cf4fb..0d6e37f00118 100644
--- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -29,7 +29,9 @@
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include 
+#include 
 #include 
+#include 
 
 namespace clang {
 namespace clangd {
@@ -1041,6 +1043,21 @@ template  class TT> int foo() {
   EXPECT_THAT(Completions, Contains(Named("TT")));
 }
 
+TEST(CompletionTest, RecordCCResultCallback) {
+  std::vector RecordedCompletions;
+  CodeCompleteOptions Opts;
+  Opts.RecordCCResult = [&RecordedCompletions](const CodeCompletion &CC,
+   const SymbolQualitySignals &,
+   const SymbolRelevanceSignals &,
+   float Score) {
+RecordedCompletions.push_back(CC);
+  };
+
+  completions("int xy1, xy2; int a = xy^", /*IndexSymbols=*/{}, Opts);
+  EXPECT_THAT(RecordedCompletions,
+  U

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

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

Szelethus added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:37-40
+  // NoError: No error flag is set or stream is not open.
+  // EofError: EOF condition (feof returns true)
+  // OtherError: other (non-EOF) error (ferror returns true)
+  // AnyError: EofError or OtherError

balazske wrote:
> balazske wrote:
> > Szelethus wrote:
> > > balazske wrote:
> > > > Szelethus wrote:
> > > > > These too. Also, I'm not yet sure whether we need `OtherError` and 
> > > > > `AnyError`, as stated in a later inline.
> > > > I plan to use `AnyError` for failing functions that can have **EOF** 
> > > > and other error as result. At a later `ferror` or `feof` call a new 
> > > > state split follows to differentiate the error (instead of having to 
> > > > add 3 new states after the function, for success, EOF error and other 
> > > > error). If other operation is called we know anyway that some error 
> > > > happened.
> > > I think it would still be better to introduce them as we find uses for 
> > > them :) Also, to stay in the scope of this patch, shouldn't we only 
> > > introduce `FseekError`? If we did, we could make warning messages a lot 
> > > clearer.
> > This change is generally about introduction of the error handling, not 
> > specifically about `fseek`. Probably not `fseek` was the best choice, it 
> > could be any other function. Probably I can add another, or multiple ones, 
> > but then again the too-big-patch problem comes up. (If now the generic 
> > error states exist the diffs for adding more stream operations could be 
> > more simple by only adding new functions and not changing the `StreamState` 
> > at all.) (How is this related to warning messages?)
> For now, the `EofError` and `OtherError` can be removed, in this change these 
> are not used (according to how `fseek` will be done).
> This change is generally about introduction of the error handling, not 
> specifically about fseek. Probably not fseek was the best choice, it could be 
> any other function.

You could not have picked a better function! Since the rules around the error 
state of the stream after a failed fseek call are quite complex, few functions 
deserve their own error state more.

> (How is this related to warning messages?)

I had the following image in my head, it could be used at the bug report 
emission site to give a precise description:

```lang=cpp
  if (SS->isInErrorState()) {
switch(SS.getErrorKind) {
case FseekError:
  reportBug("After a failed fseek call, the state of the stream may "
"have changed, and it might be feof() or ferror()!");
  break;
case EofError:
  reportBug("The stream is in an feof() state!");
  break;
case Errorf:
  reportBug("The stream is in an ferror() state!");
  break;
case OtherError:
  // We don't know what the precise error is, but we surely
  // know its in one.
  reportBug("The stream is in an error state!");
  break;
}
```
> (If now the generic error states exist the diffs for adding more stream 
> operations could be more simple by only adding new functions and not changing 
> the StreamState at all.)

For the last case in the code snippet (`OtherError`), I'm not too sure what the 
conditions are -- when do we know **what** the stream state is (some sort of an 
error), but not know precisely **how**? In the case of `fseek`, we don't 
precisely know **what** the state is but we know **how** it came about. I just 
don't yet see why we need a generic error state.

> Probably I can add another, or multiple ones, but then again the 
> too-big-patch problem comes up.

I think the point of the patch is to demonstrate the implementation of an error 
state, not to implement them all, and it does it quite well!

> For now, the EofError and OtherError can be removed, in this change these are 
> not used (according to how fseek will be done).

I agree!


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] D74618: [ARM] Creating 'call_mangled' for Neon intrinsics definitions

[Diogo N. Sampaio via Phabricator via cfe-commits]

dnsampaio added inline comments.



Comment at: clang/utils/TableGen/NeonEmitter.cpp:1890-1891
 }
+if (MangledName)
+  Good &= I.getMangledName(true) == MangledName;
+

Can we move this above the loop just before? Perhaps, if false, can we just 
continue the outer loop?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74618



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

[PATCH] D75603: [clangd] Add instrumentation mode in clangd for metrics collection.

[UTKARSH SAXENA via Phabricator via cfe-commits]

usaxena95 updated this revision to Diff 248422.
usaxena95 added a comment.

Passed score as a float as an explicit argument of the callback.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75603

Files:
  clang-tools-extra/clangd/CodeComplete.cpp
  clang-tools-extra/clangd/CodeComplete.h
  clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp


Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -29,7 +29,9 @@
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include 
+#include 
 #include 
+#include 
 
 namespace clang {
 namespace clangd {
@@ -1041,6 +1043,21 @@
   EXPECT_THAT(Completions, Contains(Named("TT")));
 }
 
+TEST(CompletionTest, RecordCCResultCallback) {
+  std::vector RecordedCompletions;
+  CodeCompleteOptions Opts;
+  Opts.RecordCCResult = [&RecordedCompletions](const CodeCompletion &CC,
+   const SymbolQualitySignals &,
+   const SymbolRelevanceSignals &,
+   float Score) {
+RecordedCompletions.push_back(CC);
+  };
+
+  completions("int xy1, xy2; int a = xy^", /*IndexSymbols=*/{}, Opts);
+  EXPECT_THAT(RecordedCompletions,
+  UnorderedElementsAre(Named("xy1"), Named("xy2")));
+}
+
 SignatureHelp signatures(llvm::StringRef Text, Position Point,
  std::vector IndexSymbols = {}) {
   std::unique_ptr Index;
Index: clang-tools-extra/clangd/CodeComplete.h
===
--- clang-tools-extra/clangd/CodeComplete.h
+++ clang-tools-extra/clangd/CodeComplete.h
@@ -19,6 +19,7 @@
 #include "Logger.h"
 #include "Path.h"
 #include "Protocol.h"
+#include "Quality.h"
 #include "index/Index.h"
 #include "index/Symbol.h"
 #include "index/SymbolOrigin.h"
@@ -29,12 +30,14 @@
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Error.h"
+#include 
 #include 
 
 namespace clang {
 class NamedDecl;
 namespace clangd {
 struct PreambleData;
+struct CodeCompletion;
 
 struct CodeCompleteOptions {
   /// Returns options that can be passed to clang's completion engine.
@@ -129,6 +132,16 @@
 /// Always use text-based completion.
 NeverParse,
   } RunParser = ParseIfReady;
+
+  /// Callback invoked on all CompletionCandidate after they are scored and
+  /// before they are ranked (by -Score). Thus the results are yielded in
+  /// arbitrary order.
+  ///
+  /// This callbacks allows capturing various internal structures used by 
clangd
+  /// during code completion. Eg: Symbol quality and relevance signals.
+  std::function
+  RecordCCResult;
 };
 
 // Semi-structured representation of a code-complete suggestion for our C++ 
API.
Index: clang-tools-extra/clangd/CodeComplete.cpp
===
--- clang-tools-extra/clangd/CodeComplete.cpp
+++ clang-tools-extra/clangd/CodeComplete.cpp
@@ -1660,6 +1660,10 @@
? Scores.Total / Relevance.NameMatch
: Scores.Quality;
 
+if (Opts.RecordCCResult)
+  Opts.RecordCCResult(toCodeCompletion(Bundle), Quality, Relevance,
+  Scores.Total);
+
 dlog("CodeComplete: {0} ({1}) = {2}\n{3}{4}\n", First.Name,
  llvm::to_string(Origin), Scores.Total, llvm::to_string(Quality),
  llvm::to_string(Relevance));


Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -29,7 +29,9 @@
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include 
+#include 
 #include 
+#include 
 
 namespace clang {
 namespace clangd {
@@ -1041,6 +1043,21 @@
   EXPECT_THAT(Completions, Contains(Named("TT")));
 }
 
+TEST(CompletionTest, RecordCCResultCallback) {
+  std::vector RecordedCompletions;
+  CodeCompleteOptions Opts;
+  Opts.RecordCCResult = [&RecordedCompletions](const CodeCompletion &CC,
+   const SymbolQualitySignals &,
+   const SymbolRelevanceSignals &,
+   float Score) {
+RecordedCompletions.push_back(CC);
+  };
+
+  completions("int xy1, xy2; int a = xy^", /*IndexSymbols=*/{}, Opts);
+  EXPECT_THAT(RecordedCompletions,
+  UnorderedElementsAre(Named("xy1"), Named("xy2")));
+}
+
 SignatureHelp signatures(llvm::StringRef Text, Position Point,
  std::vector IndexSymbols = {}) {
   std::unique_ptr Index;
Index: clan

[PATCH] D75603: [clangd] Add instrumentation mode in clangd for metrics collection.

[UTKARSH SAXENA via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGe397a0a5c3c0: [clangd] Add instrumentation mode in clangd 
for metrics collection. (authored by usaxena95).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75603

Files:
  clang-tools-extra/clangd/CodeComplete.cpp
  clang-tools-extra/clangd/CodeComplete.h
  clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp


Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -29,7 +29,9 @@
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include 
+#include 
 #include 
+#include 
 
 namespace clang {
 namespace clangd {
@@ -1041,6 +1043,21 @@
   EXPECT_THAT(Completions, Contains(Named("TT")));
 }
 
+TEST(CompletionTest, RecordCCResultCallback) {
+  std::vector RecordedCompletions;
+  CodeCompleteOptions Opts;
+  Opts.RecordCCResult = [&RecordedCompletions](const CodeCompletion &CC,
+   const SymbolQualitySignals &,
+   const SymbolRelevanceSignals &,
+   float Score) {
+RecordedCompletions.push_back(CC);
+  };
+
+  completions("int xy1, xy2; int a = xy^", /*IndexSymbols=*/{}, Opts);
+  EXPECT_THAT(RecordedCompletions,
+  UnorderedElementsAre(Named("xy1"), Named("xy2")));
+}
+
 SignatureHelp signatures(llvm::StringRef Text, Position Point,
  std::vector IndexSymbols = {}) {
   std::unique_ptr Index;
Index: clang-tools-extra/clangd/CodeComplete.h
===
--- clang-tools-extra/clangd/CodeComplete.h
+++ clang-tools-extra/clangd/CodeComplete.h
@@ -19,6 +19,7 @@
 #include "Logger.h"
 #include "Path.h"
 #include "Protocol.h"
+#include "Quality.h"
 #include "index/Index.h"
 #include "index/Symbol.h"
 #include "index/SymbolOrigin.h"
@@ -29,12 +30,14 @@
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Error.h"
+#include 
 #include 
 
 namespace clang {
 class NamedDecl;
 namespace clangd {
 struct PreambleData;
+struct CodeCompletion;
 
 struct CodeCompleteOptions {
   /// Returns options that can be passed to clang's completion engine.
@@ -129,6 +132,16 @@
 /// Always use text-based completion.
 NeverParse,
   } RunParser = ParseIfReady;
+
+  /// Callback invoked on all CompletionCandidate after they are scored and
+  /// before they are ranked (by -Score). Thus the results are yielded in
+  /// arbitrary order.
+  ///
+  /// This callbacks allows capturing various internal structures used by 
clangd
+  /// during code completion. Eg: Symbol quality and relevance signals.
+  std::function
+  RecordCCResult;
 };
 
 // Semi-structured representation of a code-complete suggestion for our C++ 
API.
Index: clang-tools-extra/clangd/CodeComplete.cpp
===
--- clang-tools-extra/clangd/CodeComplete.cpp
+++ clang-tools-extra/clangd/CodeComplete.cpp
@@ -1660,6 +1660,10 @@
? Scores.Total / Relevance.NameMatch
: Scores.Quality;
 
+if (Opts.RecordCCResult)
+  Opts.RecordCCResult(toCodeCompletion(Bundle), Quality, Relevance,
+  Scores.Total);
+
 dlog("CodeComplete: {0} ({1}) = {2}\n{3}{4}\n", First.Name,
  llvm::to_string(Origin), Scores.Total, llvm::to_string(Quality),
  llvm::to_string(Relevance));


Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -29,7 +29,9 @@
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include 
+#include 
 #include 
+#include 
 
 namespace clang {
 namespace clangd {
@@ -1041,6 +1043,21 @@
   EXPECT_THAT(Completions, Contains(Named("TT")));
 }
 
+TEST(CompletionTest, RecordCCResultCallback) {
+  std::vector RecordedCompletions;
+  CodeCompleteOptions Opts;
+  Opts.RecordCCResult = [&RecordedCompletions](const CodeCompletion &CC,
+   const SymbolQualitySignals &,
+   const SymbolRelevanceSignals &,
+   float Score) {
+RecordedCompletions.push_back(CC);
+  };
+
+  completions("int xy1, xy2; int a = xy^", /*IndexSymbols=*/{}, Opts);
+  EXPECT_THAT(RecordedCompletions,
+  UnorderedElementsAre(Named("xy1"), Named("xy2")));
+}
+
 SignatureHelp signatures(llvm::StringRef Text, Position Point,
  std::vector

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

[Sander de Smalen via Phabricator via cfe-commits]

sdesmalen updated this revision to Diff 248432.
sdesmalen marked 11 inline comments as done.
sdesmalen added a comment.

- Renamed CK and BaseTS
- Refactored switch statementsd in SVEType::getTypeFlags()


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

https://reviews.llvm.org/D75470

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

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

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

[Sander de Smalen via Phabricator via cfe-commits]

sdesmalen added inline comments.



Comment at: clang/include/clang/Basic/arm_sve.td:121
+// Load one vector (scalar base)
+def SVLD1   : MInst<"svld1[_{2}]", "dPc", "csilUcUsUiUlhfd", [IsLoad]>;

SjoerdMeijer wrote:
> This encoding, e.g, this is  "csilUcUsUiUlhfd", is such a monstrosity. It's a 
> very efficient encoding, but of course completely unreadable.  I know there 
> is prior art, and know that this is how it's been done, but just curious if 
> you have given it thoughts how to do this in a normal way, a bit more c++y.  
> I don't want to de-rail this work, but if we are adding a new emitter, 
> perhaps now is the time to give it a thought, so was just curious.  
Haha, its a bit of a monstrosity indeed. The only thing I can think of here 
would be having something like:
```class TypeSpecs val> {
  list v = val;
}
def All_Int_Float_Ty : TypeSpecs<["c", "s", "i", "l", "Uc", "Us", "Ul", "h", 
"f", "d">;

def SVLD1 : Minst<"svld1[_{2}]", "dPc", All_Int_Float_Ty, [IsLoad]>;```

But I suspect this gets a bit awkward because of the many permutations, I count 
more than 40. Not sure if that would really improve the readability.



Comment at: clang/utils/TableGen/SveEmitter.cpp:64
+Predicate(false), PredicatePattern(false), PrefetchOp(false),
+Bitwidth(128), ElementBitwidth(~0U), NumVectors(1) {
+if (!TS.empty())

SjoerdMeijer wrote:
> why a default of 128? Will this gives problems for SVE implementions with> 
> 128 bits?
SVE vectors are n x 128bits, so the 128 is scalable here.



Comment at: clang/utils/TableGen/SveEmitter.cpp:119
+class Intrinsic {
+  /// The Record this intrinsic was created from.
+  Record *R;

SjoerdMeijer wrote:
> nit: for readability, perhaps don't abbreviate some of these member names? 
> 
>  R -> Record
>  BaseTS -> BaseTypeSpec
>  CK -> ClassKind
`Record` and `ClassKind` are also the names of the enum though.
Perhaps I can rename CK to `Class`?



Comment at: clang/utils/TableGen/SveEmitter.cpp:267
+switch (ElementBitwidth) {
+case 16: Base = (unsigned)SVETypeFlags::Float16; break;
+case 32: Base = (unsigned)SVETypeFlags::Float32; break;

SjoerdMeijer wrote:
> just a return here 
Good catch!


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

https://reviews.llvm.org/D75470



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

[PATCH] D75675: [clang-format] do not insert spaces around inline asm symbolic names

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
krasimir edited the summary of this revision.
krasimir added a reviewer: hans.
hans added a comment.

Very nice, thanks!




Comment at: clang/lib/Format/TokenAnnotator.cpp:502
 Left->Type = TT_ObjCMethodExpr;
+  } else if (Style.isCpp() && InsideInlineASM) {
+Left->Type = TT_InlineASMSymbolicNameLSquare;

Is the Style.isCpp() check necessary here?

If it's necessary, maybe that's because the setting of InsideInlineASM should 
be guarded by Style.isCpp() instead?


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

The `[` in such cases was mis-annotated as an `TT_ArrayInitializerLSquare`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D75675

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -15127,6 +15127,30 @@
   guessLanguage("foo.h", "int(^foo[(kNumEntries + 10)])(char, float);"));
 }
 
+TEST_F(FormatTest, FormatsInlineAsmSymbolicNames) {
+  // ASM symbolic names are identifiers that must be surrounded by [] without
+  // space in between:
+  // https://gcc.gnu.org/onlinedocs/gcc/Extended-Asm.html#InputOperands
+
+  // Example from https://bugs.llvm.org/show_bug.cgi?id=45108.
+  verifyFormat(R"(//
+asm volatile("mrs %x[result], FPCR" : [result] "=r"(result));
+)");
+
+  // A list of several ASM symbolic names.
+  verifyFormat(R"(asm("mov %[e], %[d]" : [d] "=rm"(d), [e] "rm"(*e));)");
+
+  // ASM symbolic names in inline ASM with inputs and outputs.
+  verifyFormat(R"(//
+asm("cmoveq %1, %2, %[result]"
+: [result] "=r"(result)
+: "r"(test), "r"(new), "[result]"(old));
+)");
+
+  // ASM symbolic names in inline ASM with no outputs.
+  verifyFormat(R"(asm("mov %[e], %[d]" : : [d] "=rm"(d), [e] "rm"(*e));)");
+}
+
 TEST_F(FormatTest, GuessedLanguageWithInlineAsmClobbers) {
   EXPECT_EQ(FormatStyle::LK_Cpp,
 guessLanguage("foo.h", "void f() {\n"
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -499,6 +499,8 @@
 } else if (Left->is(TT_Unknown)) {
   if (StartsObjCMethodExpr) {
 Left->Type = TT_ObjCMethodExpr;
+  } else if (Style.isCpp() && InsideInlineASM) {
+Left->Type = TT_InlineASMSymbolicNameLSquare;
   } else if (IsCpp11AttributeSpecifier) {
 Left->Type = TT_AttributeSquare;
   } else if (Style.Language == FormatStyle::LK_JavaScript && Parent &&
Index: clang/lib/Format/FormatToken.h
===
--- clang/lib/Format/FormatToken.h
+++ clang/lib/Format/FormatToken.h
@@ -54,6 +54,7 @@
   TYPE(InheritanceComma)   
\
   TYPE(InlineASMBrace) 
\
   TYPE(InlineASMColon) 
\
+  TYPE(InlineASMSymbolicNameLSquare)   
\
   TYPE(JavaAnnotation) 
\
   TYPE(JsComputedPropertyName) 
\
   TYPE(JsExponentiation)   
\


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -15127,6 +15127,30 @@
   guessLanguage("foo.h", "int(^foo[(kNumEntries + 10)])(char, float);"));
 }
 
+TEST_F(FormatTest, FormatsInlineAsmSymbolicNames) {
+  // ASM symbolic names are identifiers that must be surrounded by [] without
+  // space in between:
+  // https://gcc.gnu.org/onlinedocs/gcc/Extended-Asm.html#InputOperands
+
+  // Example from https://bugs.llvm.org/show_bug.cgi?id=45108.
+  verifyFormat(R"(//
+asm volatile("mrs %x[result], FPCR" : [result] "=r"(result));
+)");
+
+  // A list of several ASM symbolic names.
+  verifyFormat(R"(asm("mov %[e], %[d]" : [d] "=rm"(d), [e] "rm"(*e));)");
+
+  // ASM symbolic names in inline ASM with inputs and outputs.
+  verifyFormat(R"(//
+asm("cmoveq %1, %2, %[result]"
+: [result] "=r"(result)
+: "r"(test), "r"(new), "[result]"(old));
+)");
+
+  // ASM symbolic names in inline ASM with no outputs.
+  verifyFormat(R"(asm("mov %[e], %[d]" : : [d] "=rm"(d), [e] "rm"(*e));)");
+}
+
 TEST_F(FormatTest, GuessedLanguageWithInlineAsmClobbers) {
   EXPECT_EQ(FormatStyle::LK_Cpp,
 guessLanguage("foo.h", "void f() {\n"
Index: clang/lib/Format/TokenAnnotator.cpp
=

[PATCH] D75538: [clang-tidy] Updated language supported restrictions on some checks

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/modernize-use-nullptr-basic.cpp:1
-// RUN: %check_clang_tidy -std=c++98 %s modernize-use-nullptr %t -- -- 
-Wno-non-literal-null-conversion
-//

njames93 wrote:
> alexfh wrote:
> > gribozavr2 wrote:
> > > njames93 wrote:
> > > > alexfh wrote:
> > > > > IIRC, some of the modernize- checks were meant to be useful to make 
> > > > > the pre-C++11 code compile in C++11. This check is an example of this 
> > > > > (maybe the only one?). Limiting the check to C++11 and deleting this 
> > > > > test is a bit too radical.
> > > > I'm lost, this check is all about replacing assignment of pointer to 0 
> > > > with `nullptr` a keyword which doesn't exist pre c++11, so this test 
> > > > case will just result in invalid code. Or am I missing the point?
> > > The idea, if I understand correctly, is that you start with C++98 code, 
> > > apply modernize-* checks, and get C++11 code.
> > Yep, at least for this particular check there are cases, which won't 
> > compile in C++11, but will compile after its fixes are applied. Not sure if 
> > this was ever used like this though.
> My understanding of the modernize module was its designed to convert the old 
> c++98/03 code to use newer (safer) c++ constructs. Its purpose isn't to fix 
> compiler errors in c++11 mode that compile OK in c++98 mode
This and a number of other modernize- checks were migrated from the 
clang-modernize (previously cpp11-migrate) tool. It's hard to tell now, whether 
running on code in C++98 mode and fixing errors that would appear in C++11 
**was** a supported use case at the time (though all commits are in git and one 
could look up, if someone wants to). But that's irrelevant. We need to decide 
**now**, whether we want to support this use case or not.
I believe, fixing C++11 compatibility **errors** in valid C++98 code is a 
potentially useful feature. Given that it doesn't require additional 
maintenance costs, I think we should leave it as is. The only downside is 
complaints from users who blindly run _all_ the checks regardless of whether 
they need them or not. We could try to relieve this pain, but a better 
treatment here would be to use the tool correctly, i.e. carefully select the 
checks to enable. That seems to partly be a UX problem of the frontend they use 
(CLion in the recent bug report - 
https://www.jetbrains.com/help/clion/clang-tidy-checks-support.html). Some UIs 
try to be more user-friendly w.r.t. configuration of the checks (e.g. I found 
this one today: https://www.cppdepend.com/Modernize).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75538



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

[PATCH] D75675: [clang-format] do not insert spaces around inline asm symbolic names

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

Very nice, thanks!




Comment at: clang/lib/Format/TokenAnnotator.cpp:502
 Left->Type = TT_ObjCMethodExpr;
+  } else if (Style.isCpp() && InsideInlineASM) {
+Left->Type = TT_InlineASMSymbolicNameLSquare;

Is the Style.isCpp() check necessary here?

If it's necessary, maybe that's because the setting of InsideInlineASM should 
be guarded by Style.isCpp() instead?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75675



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

[clang] 66addf8 - Revert "Fix regression in bdad0a1: force rebuilding of StmtExpr nodes in", "PR45083: Mark statement expressions as being dependent if they appear in"

[Benjamin Kramer via cfe-commits]

Author: Benjamin Kramer
Date: 2020-03-05T14:14:31+01:00
New Revision: 66addf8e803618758457e4d578c5084e322ca448

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

LOG: Revert "Fix regression in bdad0a1: force rebuilding of StmtExpr nodes in", 
"PR45083: Mark statement expressions as being dependent if they appear in"

This reverts commit f545ede91c9d9f271e7504282cab7bf509607ead.
This reverts commit bdad0a1b79273733df9acc1be4e992fa5d70ec56.

This crashes clang. I'll follow up with reproduction instructions.

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 ffc1f54fe82d..7271dbb830a2 100644
--- a/clang/include/clang/AST/Expr.h
+++ b/clang/include/clang/AST/Expr.h
@@ -3959,14 +3959,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-dependence for
+  // statements? (ugh!)
   StmtExpr(CompoundStmt *substmt, QualType T,
-   SourceLocation lp, SourceLocation rp, bool InDependentContext) :
-// Note: we treat a statement-expression in a dependent context as always
-// being value- and instantiation-dependent. This matches the behavior of
-// lambda-expressions and GCC.
+   SourceLocation lp, SourceLocation rp) :
 Expr(StmtExprClass, T, VK_RValue, OK_Ordinary,
- T->isDependentType(), InDependentContext, InDependentContext, false),
-SubStmt(substmt), LParenLoc(lp), RParenLoc(rp) {}
+ 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/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 8e4d0828e2d0..2304a9718567 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -4964,10 +4964,8 @@ class Sema final {
 LabelDecl *TheDecl);
 
   void ActOnStartStmtExpr();
-  ExprResult ActOnStmtExpr(Scope *S, SourceLocation LPLoc, Stmt *SubStmt,
-   SourceLocation RPLoc);
-  ExprResult BuildStmtExpr(SourceLocation LPLoc, Stmt *SubStmt,
-   SourceLocation RPLoc, bool IsDependent);
+  ExprResult ActOnStmtExpr(SourceLocation LPLoc, Stmt *SubStmt,
+   SourceLocation RPLoc); // "({..})"
   // Handle the final expression in a statement expression.
   ExprResult ActOnStmtExprResult(ExprResult E);
   void ActOnStmtExprError();

diff  --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index ddbd3699bdc2..9f174e9c2440 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -6631,9 +6631,8 @@ ExpectedStmt ASTNodeImporter::VisitStmtExpr(StmtExpr *E) {
   if (Err)
 return std::move(Err);
 
-  return new (Importer.getToContext())
-  StmtExpr(ToSubStmt, ToType, ToLParenLoc, ToRParenLoc,
-   E->isInstantiationDependent());
+  return new (Importer.getToContext()) StmtExpr(
+  ToSubStmt, ToType, ToLParenLoc, ToRParenLoc);
 }
 
 ExpectedStmt ASTNodeImporter::VisitUnaryOperator(UnaryOperator *E) {

diff  --git a/clang/lib/Parse/ParseExpr.cpp b/clang/lib/Parse/ParseExpr.cpp
index b038e6935d87..584de6b87d90 100644
--- a/clang/lib/Parse/ParseExpr.cpp
+++ b/clang/lib/Parse/ParseExpr.cpp
@@ -2655,8 +2655,7 @@ Parser::ParseParenExpression(ParenParseOption &ExprType, 
bool stopIfCastExpr,
 
   // If the substmt parsed correctly, build the AST node.
   if (!Stmt.isInvalid()) {
-Result = Actions.ActOnStmtExpr(getCurScope(), OpenLoc, Stmt.get(),
-   Tok.getLocation());
+Result = Actions.ActOnStmtExpr(OpenLoc, Stmt.get(), Tok.getLocation());
   } else {
 Actions.ActOnStmtExprError();
   }

diff  --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 291c38ab20b4..f50a77a40510 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -13911,14 +13911,9 @@ void Sema::ActOnStmtExprError() {
   PopExpressionEvaluationContext();
 }
 
-ExprResult Sema::ActOnStmtExpr(Scope *S, SourceLocation LPLoc, Stmt *SubStmt,
-   SourceLocation RPLoc) {
-  return BuildStmtExpr(LPLoc, SubStmt, RPLoc,
-   S->getTemplateParamP

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

[Benjamin Kramer via cfe-commits]

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-dependence 
>> > for
>> > -  // statements? (ugh!)
>> >StmtExpr(CompoundStmt *substmt, QualType T,
>> > -   SourceLocation lp, SourceLocation rp) :
>> > +   SourceLocation lp, SourceLocation rp, bool InDependentContext) 
>> > :
>> > +// Note: we treat a statement-expression in a dependent context as 
>> > always
>> > +// being value- and instantiation-dependent. This matches the 
>> > behavior of
>> > +// lambda-expressions and GCC.
>> >  Expr(StmtExprClass, T, VK_RValue, OK_Ordinary,
>> > - T->isDependentType(), false, false, false),
>> > -SubStmt(substmt), LParenLoc(lp), RParenLoc(rp) { }
>> > + T->isDependentType(), InDependentContext, InDependentContext, 
>> > false),
>> > +SubStmt(substmt), LParenLoc(lp), RParenLoc(rp) {}
>> >
>> >/// Build an empty statement expression.
>> >explicit StmtExpr(EmptyShell Empty) : Expr(StmtExprClass, Empty) { }
>> >
>> > diff  --git a/clang/include/clang/Sema/Sema.h 
>> > b/clang/include/clang/Sema/Sema.h
>> > index 2304a9718567..5739808753e3 100644
>> > --- a/clang/include/clang/Sema/Sema.h
>> > +++ b/clang/include/clang/Sema/Sema.h
>> > @@ -4964,7 +4964,7 @@ class Sema final {
>> >  LabelDecl *TheDecl);
>> >
>> >void ActOnStartStmtExpr();
>> > -  ExprResult ActOnStmtExpr(SourceLocation LPLoc, Stmt *SubStmt,
>> > +  ExprResult ActOnStmtExpr(Scope *S, SourceLocation LPLoc, Stmt *SubStmt,
>> > SourceLocation RPLoc); // "({..})"
>> >// Handle the final expression in a statement expression.
>> >ExprResult ActOnStmtExprResult(ExprResult E);
>> >
>> > diff  --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
>> > index 52288318337d..0cf00f6ca15b 100644
>> > --- a/clang/lib/AST/ASTImporter.cpp
>> > +++ b/clang/lib/AST/ASTImporter.cpp
>> > @@ -6631,8 +6631,9 @@ ExpectedStmt ASTNodeImporter::VisitStmtExpr(StmtExpr 
>> > *E) {
>> >if (Err)
>> >  return std::move(Err);
>> >
>> > -  return new (Importer.getToContext()) StmtExpr(
>> > -  ToSubStmt, ToType, ToLParenLoc, ToRParenLoc);
>> > +  return new (Importer.getToContext())
>> > +  StmtExpr(ToSubStmt, ToType, ToLParenLoc, ToRParenLoc,
>> > +   E->isInstantiationDependent());
>> >  }
>> >
>> >  ExpectedStmt ASTNodeImporter::VisitUnaryOperator(UnaryOperator *E) {
>> >
>> > diff  --git a/clang/lib/Parse/ParseExpr.cpp b/clang/lib/Parse/ParseExpr.cpp
>> > index 584de6b87d90..b038e6935d87 100644
>> > --- a/clang/lib/Parse/ParseExpr.cpp
>> > +++ b/clang/lib/Parse/ParseExpr.cpp
>> > @@ -2655,7 +2655,8 @@ Parser::ParseParenExpression(ParenParseOption 
>> > &ExprType, bool stopIfCastExpr,
>> >
>> >// If the substmt parsed correctly, build the 

[clang] 36c2ab8 - [clang-format] do not insert spaces around inline asm symbolic names

[Krasimir Georgiev via cfe-commits]

Author: Krasimir Georgiev
Date: 2020-03-05T14:17:21+01:00
New Revision: 36c2ab8d04cd9ff2e50cffa1ca6b3de00e4faa81

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

LOG: [clang-format] do not insert spaces around inline asm symbolic names

Summary:
Fixes https://bugs.llvm.org/show_bug.cgi?id=45108.

The `[` in such cases was mis-annotated as an `TT_ArrayInitializerLSquare`.

Reviewers: hans

Reviewed By: hans

Subscribers: cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang/lib/Format/FormatToken.h
clang/lib/Format/TokenAnnotator.cpp
clang/unittests/Format/FormatTest.cpp

Removed: 




diff  --git a/clang/lib/Format/FormatToken.h b/clang/lib/Format/FormatToken.h
index d0d08e470e6c..072e1ad565c2 100644
--- a/clang/lib/Format/FormatToken.h
+++ b/clang/lib/Format/FormatToken.h
@@ -54,6 +54,7 @@ namespace format {
   TYPE(InheritanceComma)   
\
   TYPE(InlineASMBrace) 
\
   TYPE(InlineASMColon) 
\
+  TYPE(InlineASMSymbolicNameLSquare)   
\
   TYPE(JavaAnnotation) 
\
   TYPE(JsComputedPropertyName) 
\
   TYPE(JsExponentiation)   
\

diff  --git a/clang/lib/Format/TokenAnnotator.cpp 
b/clang/lib/Format/TokenAnnotator.cpp
index e491de85babd..2673dacfff13 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -499,6 +499,8 @@ class AnnotatingParser {
 } else if (Left->is(TT_Unknown)) {
   if (StartsObjCMethodExpr) {
 Left->Type = TT_ObjCMethodExpr;
+  } else if (InsideInlineASM) {
+Left->Type = TT_InlineASMSymbolicNameLSquare;
   } else if (IsCpp11AttributeSpecifier) {
 Left->Type = TT_AttributeSquare;
   } else if (Style.Language == FormatStyle::LK_JavaScript && Parent &&

diff  --git a/clang/unittests/Format/FormatTest.cpp 
b/clang/unittests/Format/FormatTest.cpp
index f00c980cb715..cb052ecaf111 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -15127,6 +15127,30 @@ TEST_F(FormatTest, GuessLanguageWithCaret) {
   guessLanguage("foo.h", "int(^foo[(kNumEntries + 10)])(char, float);"));
 }
 
+TEST_F(FormatTest, FormatsInlineAsmSymbolicNames) {
+  // ASM symbolic names are identifiers that must be surrounded by [] without
+  // space in between:
+  // https://gcc.gnu.org/onlinedocs/gcc/Extended-Asm.html#InputOperands
+
+  // Example from https://bugs.llvm.org/show_bug.cgi?id=45108.
+  verifyFormat(R"(//
+asm volatile("mrs %x[result], FPCR" : [result] "=r"(result));
+)");
+
+  // A list of several ASM symbolic names.
+  verifyFormat(R"(asm("mov %[e], %[d]" : [d] "=rm"(d), [e] "rm"(*e));)");
+
+  // ASM symbolic names in inline ASM with inputs and outputs.
+  verifyFormat(R"(//
+asm("cmoveq %1, %2, %[result]"
+: [result] "=r"(result)
+: "r"(test), "r"(new), "[result]"(old));
+)");
+
+  // ASM symbolic names in inline ASM with no outputs.
+  verifyFormat(R"(asm("mov %[e], %[d]" : : [d] "=rm"(d), [e] "rm"(*e));)");
+}
+
 TEST_F(FormatTest, GuessedLanguageWithInlineAsmClobbers) {
   EXPECT_EQ(FormatStyle::LK_Cpp,
 guessLanguage("foo.h", "void f() {\n"



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

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

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

baloghadamsoftware added a comment.

In D75514#1905268 , @Szelethus wrote:

> But why is this related to `UndefinedVal`?


Because `UndefinedVal` seems to be the "null" value of `SVal` thus it is 
suitable for default value of the parameter.


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

https://reviews.llvm.org/D75514



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

[PATCH] D75675: [clang-format] do not insert spaces around inline asm symbolic names

[Krasimir Georgiev via Phabricator via cfe-commits]

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



Comment at: clang/lib/Format/TokenAnnotator.cpp:502
 Left->Type = TT_ObjCMethodExpr;
+  } else if (Style.isCpp() && InsideInlineASM) {
+Left->Type = TT_InlineASMSymbolicNameLSquare;

hans wrote:
> Is the Style.isCpp() check necessary here?
> 
> If it's necessary, maybe that's because the setting of InsideInlineASM should 
> be guarded by Style.isCpp() instead?
Good point! I think it's unnecessary.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75675



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

[PATCH] D75675: [clang-format] do not insert spaces around inline asm symbolic names

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir updated this revision to Diff 248445.
krasimir marked an inline comment as done.
krasimir added a comment.

Remove redundant check.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75675

Files:
  clang/lib/Format/TokenAnnotator.cpp


Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -499,7 +499,7 @@
 } else if (Left->is(TT_Unknown)) {
   if (StartsObjCMethodExpr) {
 Left->Type = TT_ObjCMethodExpr;
-  } else if (Style.isCpp() && InsideInlineASM) {
+  } else if (InsideInlineASM) {
 Left->Type = TT_InlineASMSymbolicNameLSquare;
   } else if (IsCpp11AttributeSpecifier) {
 Left->Type = TT_AttributeSquare;


Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -499,7 +499,7 @@
 } else if (Left->is(TT_Unknown)) {
   if (StartsObjCMethodExpr) {
 Left->Type = TT_ObjCMethodExpr;
-  } else if (Style.isCpp() && InsideInlineASM) {
+  } else if (InsideInlineASM) {
 Left->Type = TT_InlineASMSymbolicNameLSquare;
   } else if (IsCpp11AttributeSpecifier) {
 Left->Type = TT_AttributeSquare;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D75675: [clang-format] do not insert spaces around inline asm symbolic names

[Hans Wennborg via Phabricator via cfe-commits]

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

lgtm, thanks for the quick fix!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75675



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

[PATCH] D75677: [Analyzer] Only add iterator note tags to the operations of the affected iterators

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

baloghadamsoftware created this revision.
baloghadamsoftware added reviewers: NoQ, Szelethus.
baloghadamsoftware added a project: clang.
Herald added subscribers: martong, steakhal, Charusso, gamesh411, donat.nagy, 
mikhail.ramalho, a.sidorin, rnkovacs, szepet, xazax.hun, whisperity.
baloghadamsoftware added a parent revision: D74541: [Analyzer] Use note tags to 
track iterator increments and decrements.

If an error happens which is related to some iterators the Iterator Modeling 
checker adds note tags to all the iterator operations along the bug path. This 
may be disturbing if there are other iterators beside the ones which are 
affected by the bug. This patch restricts the note tags to only the affected 
iterators and adjust the debug checkers to be able to test this change.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D75677

Files:
  clang/lib/StaticAnalyzer/Checkers/DebugIteratorModeling.cpp
  clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp
  clang/test/Analysis/iterator-modelling.cpp

Index: clang/test/Analysis/iterator-modelling.cpp
===
--- clang/test/Analysis/iterator-modelling.cpp
+++ clang/test/Analysis/iterator-modelling.cpp
@@ -81,7 +81,7 @@
 
   clang_analyzer_denote(clang_analyzer_container_begin(v), "$v.begin()");
 
-  auto j = i++; // expected-note 2{{Iterator 'i' incremented by 1}}
+  auto j = i++; // expected-note{{Iterator 'i' incremented by 1}}
 
   clang_analyzer_express(clang_analyzer_iterator_position(i)); //expected-warning{{$v.begin() + 1}}
//expected-note@-1{{$v.begin() + 1}}
@@ -94,7 +94,7 @@
 
   clang_analyzer_denote(clang_analyzer_container_end(v), "$v.end()");
 
-  auto j = i--; // expected-note 2{{Iterator 'i' decremented by 1}}
+  auto j = i--; // expected-note{{Iterator 'i' decremented by 1}}
 
   clang_analyzer_express(clang_analyzer_iterator_position(i)); //expected-warning{{$v.end() - 1}}
//expected-note@-1{{$v.end() - 1}}
@@ -164,7 +164,7 @@
 
   clang_analyzer_denote(clang_analyzer_container_begin(v), "$v.begin()");
 
-  auto i2 = i1 + 2; // expected-note 2{{Iterator 'i1' incremented by 2}}
+  auto i2 = i1 + 2; // expected-note{{Iterator 'i1' incremented by 2}}
 
   clang_analyzer_eval(clang_analyzer_iterator_container(i2) == &v); // expected-warning{{TRUE}}
 // expected-note@-1{{TRUE}}
@@ -177,7 +177,7 @@
 
   clang_analyzer_denote(clang_analyzer_container_end(v), "$v.end()");
 
-  auto i2 = i1 + (-2); // expected-note 2{{Iterator 'i1' decremented by 2}}
+  auto i2 = i1 + (-2); // expected-note{{Iterator 'i1' decremented by 2}}
 
   clang_analyzer_eval(clang_analyzer_iterator_container(i2) == &v); // expected-warning{{TRUE}}
 // expected-note@-1{{TRUE}}
@@ -190,7 +190,7 @@
 
   clang_analyzer_denote(clang_analyzer_container_end(v), "$v.end()");
 
-  auto i2 = i1 - 2;  // expected-note 2{{Iterator 'i1' decremented by 2}}
+  auto i2 = i1 - 2; // expected-note{{Iterator 'i1' decremented by 2}}
 
   clang_analyzer_eval(clang_analyzer_iterator_container(i2) == &v); // expected-warning{{TRUE}}
 // expected-note@-1{{TRUE}}
@@ -203,7 +203,7 @@
 
   clang_analyzer_denote(clang_analyzer_container_begin(v), "$v.begin()");
 
-  auto i2 = i1 - (-2); // expected-note 2{{Iterator 'i1' incremented by 2}}
+  auto i2 = i1 - (-2); // expected-note{{Iterator 'i1' incremented by 2}}
 
   clang_analyzer_eval(clang_analyzer_iterator_container(i2) == &v); // expected-warning{{TRUE}}
 // expected-note@-1{{TRUE}}
@@ -217,7 +217,7 @@
   clang_analyzer_denote(clang_analyzer_container_begin(v), "$v.begin()");
 
   auto i2 = i1;
-  ++i1;  // expected-note 2{{Iterator 'i1' incremented by 1}}
+  ++i1;  // expected-note{{Iterator 'i1' incremented by 1}}
 
   clang_analyzer_express(clang_analyzer_iterator_position(i1)); //expected-warning{{$v.begin() + 1}}
 //expected-note@-1{{$v.begin() + 1}}
@@ -231,7 +231,7 @@
   clang_analyzer_denote(clang_analyzer_container_begin(v), "$v.begin()");
 
   auto i2 = i1;
-  ++i2;  // expected-note 2{{Iterator 'i2' incremented by 1}}
+  ++i2;  // expected-note{{Iterator 'i2' incremented by 1}}
 
   clang_analyzer_express(clang_analyzer_iterator_position(i1)); //expected-warning{{$v.begin()}}
 //expected-note@-1{{$v.begin()}}
@@ -245,7 +245,7 @@
   clang_analyzer_denote(clang_analyzer_container_end(v), "$v.end()");
 
   auto i2 = i1;
-  --i1;  // expected-note 2{{Iterator 'i1' decremented by 1}}

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

[Gabor Marton via Phabricator via cfe-commits]

martong created this revision.
martong added a reviewer: NoQ.
Herald added subscribers: cfe-commits, steakhal, Charusso, gamesh411, dkrupp, 
donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, rnkovacs, szepet, 
baloghadamsoftware, xazax.hun, whisperity.
Herald added a reviewer: Szelethus.
Herald added a project: clang.

Skip analysis of inherited constructors as top-level functions because 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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D75678

Files:
  clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
  clang/test/Analysis/cxx-inherited-ctor-init-expr.cpp


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,27 @@
   if (VisitedAsTopLevel.count(D))
 return true;
 
+  // Skip analysis of inherited constructors as top-level functions because 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-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,27 @@
   if (VisitedAsTopLevel.count(D))
 return true;
 
+  // Skip analysis of inherited constructors as top-level functions because 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
+  //

[PATCH] D75675: [clang-format] do not insert spaces around inline asm symbolic names

[Krasimir Georgiev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG36c2ab8d04cd: [clang-format] do not insert spaces around 
inline asm symbolic names (authored by krasimir).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75675

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -15127,6 +15127,30 @@
   guessLanguage("foo.h", "int(^foo[(kNumEntries + 10)])(char, float);"));
 }
 
+TEST_F(FormatTest, FormatsInlineAsmSymbolicNames) {
+  // ASM symbolic names are identifiers that must be surrounded by [] without
+  // space in between:
+  // https://gcc.gnu.org/onlinedocs/gcc/Extended-Asm.html#InputOperands
+
+  // Example from https://bugs.llvm.org/show_bug.cgi?id=45108.
+  verifyFormat(R"(//
+asm volatile("mrs %x[result], FPCR" : [result] "=r"(result));
+)");
+
+  // A list of several ASM symbolic names.
+  verifyFormat(R"(asm("mov %[e], %[d]" : [d] "=rm"(d), [e] "rm"(*e));)");
+
+  // ASM symbolic names in inline ASM with inputs and outputs.
+  verifyFormat(R"(//
+asm("cmoveq %1, %2, %[result]"
+: [result] "=r"(result)
+: "r"(test), "r"(new), "[result]"(old));
+)");
+
+  // ASM symbolic names in inline ASM with no outputs.
+  verifyFormat(R"(asm("mov %[e], %[d]" : : [d] "=rm"(d), [e] "rm"(*e));)");
+}
+
 TEST_F(FormatTest, GuessedLanguageWithInlineAsmClobbers) {
   EXPECT_EQ(FormatStyle::LK_Cpp,
 guessLanguage("foo.h", "void f() {\n"
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -499,6 +499,8 @@
 } else if (Left->is(TT_Unknown)) {
   if (StartsObjCMethodExpr) {
 Left->Type = TT_ObjCMethodExpr;
+  } else if (InsideInlineASM) {
+Left->Type = TT_InlineASMSymbolicNameLSquare;
   } else if (IsCpp11AttributeSpecifier) {
 Left->Type = TT_AttributeSquare;
   } else if (Style.Language == FormatStyle::LK_JavaScript && Parent &&
Index: clang/lib/Format/FormatToken.h
===
--- clang/lib/Format/FormatToken.h
+++ clang/lib/Format/FormatToken.h
@@ -54,6 +54,7 @@
   TYPE(InheritanceComma)   
\
   TYPE(InlineASMBrace) 
\
   TYPE(InlineASMColon) 
\
+  TYPE(InlineASMSymbolicNameLSquare)   
\
   TYPE(JavaAnnotation) 
\
   TYPE(JsComputedPropertyName) 
\
   TYPE(JsExponentiation)   
\


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -15127,6 +15127,30 @@
   guessLanguage("foo.h", "int(^foo[(kNumEntries + 10)])(char, float);"));
 }
 
+TEST_F(FormatTest, FormatsInlineAsmSymbolicNames) {
+  // ASM symbolic names are identifiers that must be surrounded by [] without
+  // space in between:
+  // https://gcc.gnu.org/onlinedocs/gcc/Extended-Asm.html#InputOperands
+
+  // Example from https://bugs.llvm.org/show_bug.cgi?id=45108.
+  verifyFormat(R"(//
+asm volatile("mrs %x[result], FPCR" : [result] "=r"(result));
+)");
+
+  // A list of several ASM symbolic names.
+  verifyFormat(R"(asm("mov %[e], %[d]" : [d] "=rm"(d), [e] "rm"(*e));)");
+
+  // ASM symbolic names in inline ASM with inputs and outputs.
+  verifyFormat(R"(//
+asm("cmoveq %1, %2, %[result]"
+: [result] "=r"(result)
+: "r"(test), "r"(new), "[result]"(old));
+)");
+
+  // ASM symbolic names in inline ASM with no outputs.
+  verifyFormat(R"(asm("mov %[e], %[d]" : : [d] "=rm"(d), [e] "rm"(*e));)");
+}
+
 TEST_F(FormatTest, GuessedLanguageWithInlineAsmClobbers) {
   EXPECT_EQ(FormatStyle::LK_Cpp,
 guessLanguage("foo.h", "void f() {\n"
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -499,6 +499,8 @@
 } else if (Left->is(TT_Unknown)) {
   if (StartsObjCMethodExpr) {
 Left->Type = TT_ObjCMethodExpr;
+  } else if (InsideInlineASM) {
+Left->Type = TT_InlineASMSymbolicNameLSquare;
   } else if (IsCpp11AttributeSpecifier) {
 Left->Type = TT_AttributeSquare;
   } else if (Style.Language == FormatStyle::LK_JavaScript && Parent

[PATCH] D75675: [clang-format] do not insert spaces around inline asm symbolic names

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir updated this revision to Diff 248449.
krasimir added a comment.

Merging commits into 1 patch


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75675

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -15127,6 +15127,30 @@
   guessLanguage("foo.h", "int(^foo[(kNumEntries + 10)])(char, float);"));
 }
 
+TEST_F(FormatTest, FormatsInlineAsmSymbolicNames) {
+  // ASM symbolic names are identifiers that must be surrounded by [] without
+  // space in between:
+  // https://gcc.gnu.org/onlinedocs/gcc/Extended-Asm.html#InputOperands
+
+  // Example from https://bugs.llvm.org/show_bug.cgi?id=45108.
+  verifyFormat(R"(//
+asm volatile("mrs %x[result], FPCR" : [result] "=r"(result));
+)");
+
+  // A list of several ASM symbolic names.
+  verifyFormat(R"(asm("mov %[e], %[d]" : [d] "=rm"(d), [e] "rm"(*e));)");
+
+  // ASM symbolic names in inline ASM with inputs and outputs.
+  verifyFormat(R"(//
+asm("cmoveq %1, %2, %[result]"
+: [result] "=r"(result)
+: "r"(test), "r"(new), "[result]"(old));
+)");
+
+  // ASM symbolic names in inline ASM with no outputs.
+  verifyFormat(R"(asm("mov %[e], %[d]" : : [d] "=rm"(d), [e] "rm"(*e));)");
+}
+
 TEST_F(FormatTest, GuessedLanguageWithInlineAsmClobbers) {
   EXPECT_EQ(FormatStyle::LK_Cpp,
 guessLanguage("foo.h", "void f() {\n"
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -499,6 +499,8 @@
 } else if (Left->is(TT_Unknown)) {
   if (StartsObjCMethodExpr) {
 Left->Type = TT_ObjCMethodExpr;
+  } else if (InsideInlineASM) {
+Left->Type = TT_InlineASMSymbolicNameLSquare;
   } else if (IsCpp11AttributeSpecifier) {
 Left->Type = TT_AttributeSquare;
   } else if (Style.Language == FormatStyle::LK_JavaScript && Parent &&
Index: clang/lib/Format/FormatToken.h
===
--- clang/lib/Format/FormatToken.h
+++ clang/lib/Format/FormatToken.h
@@ -54,6 +54,7 @@
   TYPE(InheritanceComma)   
\
   TYPE(InlineASMBrace) 
\
   TYPE(InlineASMColon) 
\
+  TYPE(InlineASMSymbolicNameLSquare)   
\
   TYPE(JavaAnnotation) 
\
   TYPE(JsComputedPropertyName) 
\
   TYPE(JsExponentiation)   
\


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -15127,6 +15127,30 @@
   guessLanguage("foo.h", "int(^foo[(kNumEntries + 10)])(char, float);"));
 }
 
+TEST_F(FormatTest, FormatsInlineAsmSymbolicNames) {
+  // ASM symbolic names are identifiers that must be surrounded by [] without
+  // space in between:
+  // https://gcc.gnu.org/onlinedocs/gcc/Extended-Asm.html#InputOperands
+
+  // Example from https://bugs.llvm.org/show_bug.cgi?id=45108.
+  verifyFormat(R"(//
+asm volatile("mrs %x[result], FPCR" : [result] "=r"(result));
+)");
+
+  // A list of several ASM symbolic names.
+  verifyFormat(R"(asm("mov %[e], %[d]" : [d] "=rm"(d), [e] "rm"(*e));)");
+
+  // ASM symbolic names in inline ASM with inputs and outputs.
+  verifyFormat(R"(//
+asm("cmoveq %1, %2, %[result]"
+: [result] "=r"(result)
+: "r"(test), "r"(new), "[result]"(old));
+)");
+
+  // ASM symbolic names in inline ASM with no outputs.
+  verifyFormat(R"(asm("mov %[e], %[d]" : : [d] "=rm"(d), [e] "rm"(*e));)");
+}
+
 TEST_F(FormatTest, GuessedLanguageWithInlineAsmClobbers) {
   EXPECT_EQ(FormatStyle::LK_Cpp,
 guessLanguage("foo.h", "void f() {\n"
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -499,6 +499,8 @@
 } else if (Left->is(TT_Unknown)) {
   if (StartsObjCMethodExpr) {
 Left->Type = TT_ObjCMethodExpr;
+  } else if (InsideInlineASM) {
+Left->Type = TT_InlineASMSymbolicNameLSquare;
   } else if (IsCpp11AttributeSpecifier) {
 Left->Type = TT_AttributeSquare;
   } else if (Style.Language == FormatStyle::LK_JavaScript && Parent &&
Index: clang/lib/Format/FormatToken.h
===

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

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

Thanks Richard for the explanation.

Artem, I think this justifies your suggestion to skip the analysis of inherited 
constructors as top level functions. I just created the patch 
https://reviews.llvm.org/D75678


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

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

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

For those who are interested in more details please refer to the related 
discussion after the commit of the patch that introduces handling of inherited 
ctors .


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

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Thanks!! I also recommend a more direct test with `-analyzer-display-progress | 
FileCheck`.




Comment at: clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp:507-508
 
+  // Skip analysis of inherited constructors as top-level functions because we
+  // cannot model the parameters.
+  //

That's probably the last reason why we should skip them :) I think we should 
focus on how 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. We might as 
well find the bug in the inherited constructors (also 
/~~inherited~~/inheriting/ in your comment).


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] D75184: [clang-tidy] Optional inheritance of file configs from parent directories

[Dmitry Polukhin via Phabricator via cfe-commits]

DmitryPolukhin updated this revision to Diff 248457.
DmitryPolukhin added a comment.

Comments resolved, please take another look.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75184

Files:
  clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
  clang-tools-extra/clang-tidy/ClangTidyOptions.h
  clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
  
clang-tools-extra/test/clang-tidy/infrastructure/Inputs/config-files/3/.clang-tidy
  
clang-tools-extra/test/clang-tidy/infrastructure/Inputs/config-files/4/.clang-tidy
  
clang-tools-extra/test/clang-tidy/infrastructure/Inputs/config-files/4/44/.clang-tidy
  clang-tools-extra/test/clang-tidy/infrastructure/config-files.cpp

Index: clang-tools-extra/test/clang-tidy/infrastructure/config-files.cpp
===
--- clang-tools-extra/test/clang-tidy/infrastructure/config-files.cpp
+++ clang-tools-extra/test/clang-tidy/infrastructure/config-files.cpp
@@ -7,6 +7,23 @@
 // RUN: clang-tidy -dump-config %S/Inputs/config-files/2/- -- | FileCheck %s -check-prefix=CHECK-CHILD2
 // CHECK-CHILD2: Checks: {{.*}}from-parent
 // CHECK-CHILD2: HeaderFilterRegex: parent
+// RUN: clang-tidy -dump-config %S/Inputs/config-files/3/- -- | FileCheck %s -check-prefix=CHECK-CHILD3
+// CHECK-CHILD3: Checks: {{.*}}from-parent,from-child3
+// CHECK-CHILD3: HeaderFilterRegex: child3
 // RUN: clang-tidy -dump-config -checks='from-command-line' -header-filter='from command line' %S/Inputs/config-files/- -- | FileCheck %s -check-prefix=CHECK-COMMAND-LINE
 // CHECK-COMMAND-LINE: Checks: {{.*}}from-parent,from-command-line
 // CHECK-COMMAND-LINE: HeaderFilterRegex: from command line
+
+// For this test we have to use names of the real checks because otherwise values are ignored.
+// RUN: clang-tidy -dump-config %S/Inputs/config-files/4/44/- -- | FileCheck %s -check-prefix=CHECK-CHILD4
+// CHECK-CHILD4: Checks: {{.*}}modernize-loop-convert,llvm-qualified-auto
+// CHECK-CHILD4: - key: llvm-qualified-auto.AddConstToQualified
+// CHECK-CHILD4-NEXT: value: '1
+// CHECK-CHILD4: - key: modernize-loop-convert.MaxCopySize
+// CHECK-CHILD4-NEXT: value: '20'
+// CHECK-CHILD4: - key: modernize-loop-convert.MinConfidence
+// CHECK-CHILD4-NEXT: value: reasonable
+
+// RUN: clang-tidy --explain-config %S/Inputs/config-files/4/44/- -- | FileCheck %s -check-prefix=CHECK-EXPLAIN
+// CHECK-EXPLAIN: 'llvm-qualified-auto' is enabled in the {{.*}}/Inputs/config-files/4/44/.clang-tidy.
+// CHECK-EXPLAIN: 'modernize-loop-convert' is enabled in the {{.*}}/Inputs/config-files/4/.clang-tidy.
Index: clang-tools-extra/test/clang-tidy/infrastructure/Inputs/config-files/4/44/.clang-tidy
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/infrastructure/Inputs/config-files/4/44/.clang-tidy
@@ -0,0 +1,7 @@
+InheritParentConfig: true
+Checks: 'llvm-qualified-auto'
+CheckOptions:
+  - key: modernize-loop-convert.MaxCopySize
+value:   '20'
+  - key: llvm-qualified-auto.AddConstToQualified
+value:   '1'
Index: clang-tools-extra/test/clang-tidy/infrastructure/Inputs/config-files/4/.clang-tidy
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/infrastructure/Inputs/config-files/4/.clang-tidy
@@ -0,0 +1,6 @@
+Checks: '-*,modernize-loop-convert'
+CheckOptions:
+  - key: modernize-loop-convert.MaxCopySize
+value:   '10'
+  - key: modernize-loop-convert.MinConfidence
+value:   reasonable
Index: clang-tools-extra/test/clang-tidy/infrastructure/Inputs/config-files/3/.clang-tidy
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/infrastructure/Inputs/config-files/3/.clang-tidy
@@ -0,0 +1,3 @@
+InheritParentConfig: true
+Checks: 'from-child3'
+HeaderFilterRegex: 'child3'
Index: clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
===
--- clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
+++ clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
@@ -36,17 +36,20 @@
 Configuration files:
   clang-tidy attempts to read configuration for each source file from a
   .clang-tidy file located in the closest parent directory of the source
-  file. If any configuration options have a corresponding command-line
-  option, command-line option takes precedence. The effective
-  configuration can be inspected using -dump-config:
+  file. If InheritParentConfig is true in a config file, the configuration file
+  in the parent directory (if any exists) will be taken and current config file
+  will be applied on top of the parent one. If any configuration options have
+  a corresponding command-line option, command-line option takes precedence.
+  The effective co

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

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer added inline comments.



Comment at: clang/include/clang/Basic/arm_sve.td:121
+// Load one vector (scalar base)
+def SVLD1   : MInst<"svld1[_{2}]", "dPc", "csilUcUsUiUlhfd", [IsLoad]>;

sdesmalen wrote:
> SjoerdMeijer wrote:
> > This encoding, e.g, this is  "csilUcUsUiUlhfd", is such a monstrosity. It's 
> > a very efficient encoding, but of course completely unreadable.  I know 
> > there is prior art, and know that this is how it's been done, but just 
> > curious if you have given it thoughts how to do this in a normal way, a bit 
> > more c++y.  I don't want to de-rail this work, but if we are adding a new 
> > emitter, perhaps now is the time to give it a thought, so was just curious. 
> >  
> Haha, its a bit of a monstrosity indeed. The only thing I can think of here 
> would be having something like:
> ```class TypeSpecs val> {
>   list v = val;
> }
> def All_Int_Float_Ty : TypeSpecs<["c", "s", "i", "l", "Uc", "Us", "Ul", "h", 
> "f", "d">;
> 
> def SVLD1 : Minst<"svld1[_{2}]", "dPc", All_Int_Float_Ty, [IsLoad]>;```
> 
> But I suspect this gets a bit awkward because of the many permutations, I 
> count more than 40. Not sure if that would really improve the readability.
I would personally welcome any improvement here, even the smallest. But if you 
think it's tricky, then fair enough!

I've managed to completely ignore the MVE intrinsics work so far, but 
understood there were some innovations here and there (e.g. in tablegen). 
Probably because it is dealing with similar problems: a lot of intrinsics, some 
of them overloaded with different types. I'm going to have a little look now to 
see if there's anything we can borrow from that, or if that is unrelated



Comment at: clang/utils/TableGen/SveEmitter.cpp:64
+Predicate(false), PredicatePattern(false), PrefetchOp(false),
+Bitwidth(128), ElementBitwidth(~0U), NumVectors(1) {
+if (!TS.empty())

sdesmalen wrote:
> SjoerdMeijer wrote:
> > why a default of 128? Will this gives problems for SVE implementions with> 
> > 128 bits?
> SVE vectors are n x 128bits, so the 128 is scalable here.
ah, okay, fair enough, didn't realise that.


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

https://reviews.llvm.org/D75470



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

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

[Haojian Wu via Phabricator via cfe-commits]

hokein commandeered this revision.
hokein added a reviewer: ilya-biryukov.
hokein added a comment.

This patch contains too many changes, most of them are just NFC, it likely 
takes a long time to do a full review.

I actually did an review for the original patch. I have highlighted places (see 
me comments) that I'm uncertain and need another look, other places are NFC.




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

!  behavior change change, now the `ConvertVectorExpr::TypeDependent = 
DstType->isDependentType() | SrcExpr->isDependentType()`.



Comment at: clang/include/clang/AST/Expr.h:4139
- bool TypeDependent, bool ValueDependent)
-: Expr(ChooseExprClass, t, VK, OK, TypeDependent, ValueDependent,
-   (cond->isInstantiationDependent() ||

! this needs careful review, the ChooseExpr bits are from different sources:

- constructor here
- 
[`clang/lib/Sema/SemaPseudoObject.cpp`](https://github.com/llvm/llvm-project/blob/master/clang/lib/Sema/SemaPseudoObject.cpp#L170)
- 
[`clang/lib/AST/ASTImporter.cpp`](https://github.com/llvm/llvm-project/blob/master/clang/lib/AST/ASTImporter.cpp#L6347)
- 
[`clang/lib/Sema/SemaExpr.cpp`](https://github.com/llvm/llvm-project/blob/master/clang/lib/Sema/SemaExpr.cpp#L14207)

 



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

! the implementation in the original patch doesn't seem to correct to me, I 
rewrote it, need another review..



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

! behavior change here, now `ArrayInitLoopExpr::Instantiation =  
T->isInstantiationDependentType() |  CommonInit->isInstantiationDependentType() 
|  ElementInit->isInstantiationDependentType()`.



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

! behavior change here, now `Expr::TypeDependent = DstType->isDependentType() | 
SrcExpr->isTypeDependent()`.



Comment at: clang/include/clang/AST/Expr.h:1537
 CharacterLiteralBits.Kind = kind;
+setDependence(ExprDependence::None);
   }

this is not needed indeed as the default value is 0, but I'd keep it here to 
make it more explicit.



Comment at: clang/include/clang/AST/ExprCXX.h:2742
-  : Expr(ArrayTypeTraitExprClass, ty, VK_RValue, OK_Ordinary,
- false, queried->getType()->isDependentType(),
- (queried->getType()->isInstantiationDependentType() ||

! behavior change here, now we the `ValueDependent`, `UnexpandedPack` takes 
`dimension` into account as well.



Comment at: clang/lib/AST/ExprCXX.cpp:444
-  SC, Context.OverloadTy, VK_LValue, OK_Ordinary, KnownDependent,
-  KnownDependent,
-  (KnownInstantiationDependent || NameInfo.isInstantiationDependent() 
||

! this change is not trivial, need an review.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73638



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

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

[Benjamin Kramer via cfe-commits]

creduce produced this. It's a crash on invalid, but was created from a
valid input.

$ 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;

$ clang r.ii -std=c++17 -w
clang-11: clang/lib/AST/Decl.cpp:2343: clang::APValue
*clang::VarDecl::evaluateValue(SmallVectorImpl
&) const: Assertion `!Init->isValueDep
endent()' 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-dependence for
> >> > -  // statements? (ugh!)
> >> >StmtExpr(CompoundStmt *substmt, QualType T,
> >> > -   SourceLocation lp, SourceLocation rp) :
> >> > +   SourceLocation lp, SourceLocation rp, bool 
> >> > InDependentContext) :
> >> > +// Note: we treat a statement-expression in a dependent context as 
> >> > always
> >> > +// being value- and instantiation-dependent. This matches the 
> >> > behavior of
> >> > +// lambda-expressions and GCC.
> >> >  Expr(StmtExprClass, T, VK_RValue, OK_Ordinary,
> >> > - T->isDependentType(), false, false, false),
> >> > -SubStmt(substmt), LParenLoc(lp), RParenLoc(rp) { }
> >> > + T->isDependentType(), InDependentContext, InDependentContext, 
> >> > false),
> >> > +SubStmt(substmt), LParenLoc(lp), RParenLoc(rp) {}
> >> >
> >> >/// Build an empty statement expression.
> >> >explicit StmtExpr(EmptyShell Empty) : Expr(StmtExprClass, Empty) { }
> >> >
> >> > diff  --git a/clang/include/clang/Sema/Sema.h 
> >> > b/clang/include/clang/Sema/Sema.h
> >> > index 2304a9718567..5739808753e3 100644
> >> > --- a/clang/include/clang/Sema/Sema.h
> >> > +++ b/clang/include/clang/Sema/Sema.h
> >> > @@ -4964,7 +4964,7 @@ class Sema final {
> >> >  LabelDecl *TheDecl);
> >> >
> >> >void ActOnStartStmtExpr();
> >> > -  ExprResult ActOnStmtExpr(SourceLocation LPLoc, Stmt *SubStmt,
> >> > +  ExprResult ActOnStmtExpr(Scope *S, SourceLocation LPLoc, Stmt 
> >> > *SubStmt,
> >> > SourceLocation RPLoc); // "({..})"
> >> >// Handle the final expression in a statement expression.
> >> >ExprResult ActOnStmtExprResult(ExprResult E);
> >> >
> >> > diff  --git a/clang/lib/AST/ASTImporter.cpp 
> >> > b/clang/lib/AST/ASTImporter.cpp
> >> > index 52288318337d..0cf00f6ca15b 100644
> >> > --- a/clang/lib/AST/ASTImporter.cpp
> >> > +++ b/clang/lib/AST/ASTImporter.cpp
> >> > @@ -6631,8 +6631,9 @@ ExpectedStmt 
> >> > ASTNodeImporter::VisitStmtExpr(StmtExpr *E) {
> >> >if (Err)
> >> >  return std::move(Err);
> >> >
> >> > -  return new (Importer.getToCo

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

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

Szelethus added a comment.

In D75514#1907392 , 
@baloghadamsoftware wrote:

> In D75514#1905268 , @Szelethus wrote:
>
> > But why is this related to `UndefinedVal`?
>
>
> Because `UndefinedVal` seems to be the "null" value of `SVal` thus it is 
> suitable for default value of the parameter.


Shouldn't we use `Optional` then? `None` means something completely different 
then `UndefinedVal`.




Comment at: clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp:53-57
+  ExplodedNode *reportBug(llvm::StringRef Msg, CheckerContext &C,
+  SVal ExprVal = UndefinedVal()) const;
   ExplodedNode *reportBug(llvm::StringRef Msg, BugReporter &BR,
-  ExplodedNode *N) const;
+  ExplodedNode *N,
+  SVal ExprVal = UndefinedVal()) const;

I would also prefer to comment what happens when `ExprVal` has a special value, 
because its non-obvious from the declaration.


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

https://reviews.llvm.org/D75514



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

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

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

Szelethus added a comment.

I followed the discussion, on the other patch, and this seems to be the 
appropriate fix -- I lack the confidence to accept, but LGTM!


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] D75623: [clangd][VSCode] Force VSCode to use the ranking provided by clangd.

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

thanks for fixing that! Played around it locally, it seems work, found an issue 
where the global code completion is not triggered, see my comment.




Comment at: clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts:116
+// Get the incomplete identifier before the cursor.
+let word = document.getWordRangeAtPosition(position);
+let prefix = document.getText(new vscode.Range(word.start, 
position));

the word is `undefined` if we trigger the global code completion, e.g. `^`, 
`std::^`.





Comment at: clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts:124
+})
+return new vscode.CompletionList(items, true);
+  }

nit: /*isInComplete=*/true


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75623



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

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

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

In D75514#1907392 , 
@baloghadamsoftware wrote:

> In D75514#1905268 , @Szelethus wrote:
>
> > But why is this related to `UndefinedVal`?
>
>
> Because `UndefinedVal` seems to be the "null" value of `SVal`


//It is not.// It is a very specific and exotic `SVal` that almost never 
appears in practice but plays a very important role in the analysis.
Let's remove the default constructor for `SVal` entirely because it seems to be 
a common source of confusion.


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

https://reviews.llvm.org/D75514



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

[PATCH] D75632: Comment parsing: Treat \ref as inline command

[Dmitri Gribenko via Phabricator via cfe-commits]

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

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75632



___
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 marked an inline comment as done.
martong added a comment.

In D75678#1907449 , @NoQ wrote:

> Thanks!! I also recommend a more direct test with `-analyzer-display-progress 
> | FileCheck`.


Ok, I added a new test file with FileCheck.




Comment at: clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp:507-508
 
+  // Skip analysis of inherited constructors as top-level functions because we
+  // cannot model the parameters.
+  //

NoQ wrote:
> That's probably the last reason why we should skip them :) I think we should 
> focus on how 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. We might as 
> well find the bug in the inherited constructors (also 
> /~~inherited~~/inheriting/ in your comment).
Ok, I changed the comments.


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] D75470: [SVE] Auto-generate builtins and header for svld1.

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer added a subscriber: simon_tatham.
SjoerdMeijer added a comment.

Adding @simon_tatham in case he feels wants to have a look too.


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

https://reviews.llvm.org/D75470



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

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

[Gabor Marton via Phabricator via cfe-commits]

martong updated this revision to Diff 248473.
martong marked an inline comment as done.
martong added a comment.

- Change comments, add FileCheck 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(int)
+// 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 x) {
+  int d;
+  (b(d));
+  (a(x));
+}
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(int)
+// 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 x) {
+  int d;
+  (b(d));
+  (a(x));
+}
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{{1

[clang] a8648fd - Replace getAs with castAs to fix null dereference static analyzer warning.

[Simon Pilgrim via cfe-commits]

Author: Simon Pilgrim
Date: 2020-03-05T15:28:54Z
New Revision: a8648fd19aecfe2aed3ce529f488930cc37db4c2

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

LOG: Replace getAs with castAs to fix null dereference static analyzer warning.

Use castAs as we know the cast should succeed and we're dereferencing in the 
mangleBareFunctionType call.

Added: 


Modified: 
clang/lib/AST/ItaniumMangle.cpp

Removed: 




diff  --git a/clang/lib/AST/ItaniumMangle.cpp b/clang/lib/AST/ItaniumMangle.cpp
index 5d485e000750..6d21869e2f11 100644
--- a/clang/lib/AST/ItaniumMangle.cpp
+++ b/clang/lib/AST/ItaniumMangle.cpp
@@ -1776,8 +1776,8 @@ void CXXNameMangler::mangleLambda(const CXXRecordDecl 
*Lambda) {
 void CXXNameMangler::mangleLambdaSig(const CXXRecordDecl *Lambda) {
   for (auto *D : Lambda->getLambdaExplicitTemplateParameters())
 mangleTemplateParamDecl(D);
-  const FunctionProtoType *Proto = Lambda->getLambdaTypeInfo()->getType()->
-   getAs();
+  auto *Proto =
+  Lambda->getLambdaTypeInfo()->getType()->castAs();
   mangleBareFunctionType(Proto, /*MangleReturnType=*/false,
  Lambda->getLambdaStaticInvoker());
 }



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

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

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp:719-720
+  const auto *PSBR = dyn_cast(&BR);
+  if (!PSBR)
+return "";
+

Mmm, this is definitely impossible. We should change the `NoteTag` API to pass 
in a `PathSensitiveBugReport &`. My bad.


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

https://reviews.llvm.org/D75514



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

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

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added a comment.

ping


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

[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/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] D75682: [Analyzer][StreamChecker] Introduction of stream error handling.

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

balazske added a comment.

For older discussion see this:
https://reviews.llvm.org/D75356

Functions `feof` and `ferror` are added to show how the stream error flags are 
used and to make tests possible.


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] D75682: [Analyzer][StreamChecker] Introduction of stream error handling.

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

balazske created this revision.
Herald added subscribers: cfe-commits, martong, Charusso, gamesh411, dkrupp, 
donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, 
xazax.hun.
Herald added a reviewer: Szelethus.
Herald added a project: clang.
balazske added a comment.

For older discussion see this:
https://reviews.llvm.org/D75356

Functions `feof` and `ferror` are added to show how the stream error flags are 
used and to make tests possible.


A new field is added to the stream state to store the actual error.
Function `fseek` is implemented to set the error state.
(More are to be added later.)
Modeling of stream error state handling functions is added.

(`clearerr` is not done yet but should follow.)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D75682

Files:
  clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
  clang/test/Analysis/Inputs/system-header-simulator.h
  clang/test/Analysis/stream-error.c
  clang/test/Analysis/stream.c

Index: clang/test/Analysis/stream.c
===
--- clang/test/Analysis/stream.c
+++ clang/test/Analysis/stream.c
@@ -1,26 +1,6 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=alpha.unix.Stream -analyzer-store region -verify %s
 
-typedef __typeof__(sizeof(int)) size_t;
-typedef __typeof__(sizeof(int)) fpos_t;
-typedef struct _IO_FILE FILE;
-#define SEEK_SET  0  /* Seek from beginning of file.  */
-#define SEEK_CUR  1  /* Seek from current position.  */
-#define SEEK_END  2  /* Seek from end of file.  */
-extern FILE *fopen(const char *path, const char *mode);
-extern FILE *tmpfile(void);
-extern int fclose(FILE *fp);
-extern size_t fread(void *ptr, size_t size, size_t nmemb, FILE *stream);
-extern size_t fwrite(const void *ptr, size_t size, size_t nmemb, FILE *stream);
-extern int fseek (FILE *__stream, long int __off, int __whence);
-extern long int ftell (FILE *__stream);
-extern void rewind (FILE *__stream);
-extern int fgetpos(FILE *stream, fpos_t *pos);
-extern int fsetpos(FILE *stream, const fpos_t *pos);
-extern void clearerr(FILE *stream);
-extern int feof(FILE *stream);
-extern int ferror(FILE *stream);
-extern int fileno(FILE *stream);
-extern FILE *freopen(const char *pathname, const char *mode, FILE *stream);
+#include "Inputs/system-header-simulator.h"
 
 void check_fread() {
   FILE *fp = tmpfile();
Index: clang/test/Analysis/stream-error.c
===
--- /dev/null
+++ clang/test/Analysis/stream-error.c
@@ -0,0 +1,43 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.unix.Stream -analyzer-checker=debug.ExprInspection -analyzer-store region -verify %s
+
+#include "Inputs/system-header-simulator.h"
+
+void clang_analyzer_eval(int);
+
+void error_fopen() {
+  FILE *F = fopen("file", "r");
+  if (!F)
+return;
+  clang_analyzer_eval(feof(F)); // expected-warning {{FALSE}}
+  clang_analyzer_eval(ferror(F)); // expected-warning {{FALSE}}
+  fclose(F);
+}
+
+void error_freopen() {
+  FILE *F = fopen("file", "r");
+  if (!F)
+return;
+  F = freopen(0, "w", F);
+  if (!F)
+return;
+  clang_analyzer_eval(feof(F)); // expected-warning {{FALSE}}
+  clang_analyzer_eval(ferror(F)); // expected-warning {{FALSE}}
+  fclose(F);
+}
+
+void error_fseek() {
+  FILE *F = fopen("file", "r");
+  if (!F)
+return;
+  int rc = fseek(F, 0, SEEK_SET);
+  if (rc) {
+int Eof = feof(F), Error = ferror(F);
+clang_analyzer_eval(Eof || Error); // expected-warning {{FALSE}} \
+   // expected-warning {{TRUE}}
+clang_analyzer_eval(Eof && Error); // expected-warning {{FALSE}}
+  } else {
+clang_analyzer_eval(feof(F)); // expected-warning {{FALSE}}
+clang_analyzer_eval(ferror(F)); // expected-warning {{FALSE}}
+  }
+  fclose(F);
+}
\ No newline at end of file
Index: clang/test/Analysis/Inputs/system-header-simulator.h
===
--- clang/test/Analysis/Inputs/system-header-simulator.h
+++ clang/test/Analysis/Inputs/system-header-simulator.h
@@ -9,7 +9,16 @@
 #define restrict /*restrict*/
 #endif
 
+typedef __typeof(sizeof(int)) size_t;
+typedef long long __int64_t;
+typedef __int64_t __darwin_off_t;
+typedef __darwin_off_t fpos_t;
+
 typedef struct _FILE FILE;
+#define SEEK_SET  0  /* Seek from beginning of file.  */
+#define SEEK_CUR  1  /* Seek from current position.  */
+#define SEEK_END  2  /* Seek from end of file.  */
+
 extern FILE *stdin;
 extern FILE *stdout;
 extern FILE *stderr;
@@ -24,11 +33,34 @@
 int fprintf(FILE *restrict, const char *restrict, ...);
 int getchar(void);
 
+void setbuf(FILE * restrict, char * restrict);
+int setvbuf(FILE * restrict, char * restrict, int, size_t);
+
+FILE *funopen(const void *,
+ int (*)(void *, char *, int),
+ int (*)(void *, const char *, int),
+ fpos_t (*)(void *, fpos_t, int),
+ int (*)(void *));
+

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

[Simon Tatham via Phabricator via cfe-commits]

simon_tatham added inline comments.



Comment at: clang/include/clang/Basic/arm_sve.td:121
+// Load one vector (scalar base)
+def SVLD1   : MInst<"svld1[_{2}]", "dPc", "csilUcUsUiUlhfd", [IsLoad]>;

SjoerdMeijer wrote:
> sdesmalen wrote:
> > SjoerdMeijer wrote:
> > > This encoding, e.g, this is  "csilUcUsUiUlhfd", is such a monstrosity. 
> > > It's a very efficient encoding, but of course completely unreadable.  I 
> > > know there is prior art, and know that this is how it's been done, but 
> > > just curious if you have given it thoughts how to do this in a normal 
> > > way, a bit more c++y.  I don't want to de-rail this work, but if we are 
> > > adding a new emitter, perhaps now is the time to give it a thought, so 
> > > was just curious.  
> > Haha, its a bit of a monstrosity indeed. The only thing I can think of here 
> > would be having something like:
> > ```class TypeSpecs val> {
> >   list v = val;
> > }
> > def All_Int_Float_Ty : TypeSpecs<["c", "s", "i", "l", "Uc", "Us", "Ul", 
> > "h", "f", "d">;
> > 
> > def SVLD1 : Minst<"svld1[_{2}]", "dPc", All_Int_Float_Ty, [IsLoad]>;```
> > 
> > But I suspect this gets a bit awkward because of the many permutations, I 
> > count more than 40. Not sure if that would really improve the readability.
> I would personally welcome any improvement here, even the smallest. But if 
> you think it's tricky, then fair enough!
> 
> I've managed to completely ignore the MVE intrinsics work so far, but 
> understood there were some innovations here and there (e.g. in tablegen). 
> Probably because it is dealing with similar problems: a lot of intrinsics, 
> some of them overloaded with different types. I'm going to have a little look 
> now to see if there's anything we can borrow from that, or if that is 
> unrelated
In the MVE intrinsics implementation I completely avoided that entire system of 
string-based type specifications. There's another completely different way you 
can set up the types of builtins, and I used that instead.

You can declare the function for `Builtins.def` purposes with no type 
specification at all, and then you fill in its type signature using a 
declaration in the //header file//, with the unusual combination of 
`__inline__` and no function body:
```static __inline__ int32x4_t __builtin_arm_foo_bar(int16x8_t, float23x7t); // 
or whatever```

In fact I went one step further: the user-facing names for the MVE intrinsics 
are declared in `arm_mve.h` with a special attribute indicating that they're 
aliases for clang builtins. And the MVE polymorphic intrinsics are done by 
adding `__attribute__((overloadable))` to the declaration, which allows 
C++-style overloading based on parameter types even when compiling in C. So 
when the user invokes an MVE intrinsic by its polymorphic name, the compiler 
first does overload resolution to decide which declaration in the header file 
to select; then it looks at the builtin-alias attribute and discovers which 
internal builtin id it corresponds to; and then it can do codegen for that 
builtin directly, without a wrapper function in the header.

Pros of doing it this way:
 - if the builtin requires some of its arguments to be compile-time constants, 
then you don't run into the problem that a wrapper function in the header fails 
to pass through the constantness. (In NEON this is worked around by making some 
wrapper functions be wrapper macros instead – but NEON doesn't have to deal 
with polymorphism.)
 - declaring a builtin's type signature in the header file means that it can 
include definitions that the header file has created beforehand. For example, 
one of the arguments to the MVE `vld2q` family involves a small `struct` 
containing 2 or 4 vectors, and it would be tricky to get that struct type into 
the `Builtins.def` type specification before the header file can tell clang it 
exists.
 - doing polymorphism like this, rather than making the polymorphic function be 
a macro expanding to something involving C11 `_Generic`, means the error 
messages are orders of magnitude more friendly when the user messes up a call. 
(Also it's remarkably fiddly to use `_Generic` in complicated cases, because of 
the requirement that even its untaken branches not provoke any type-checking or 
semantic errors.)
 - I don't know of any way that the preprocessor + `_Generic` approach can 
avoid expanding its macro arguments at least twice. It can avoid //evaluating// 
twice, so that's safe in the side-effect sense, but you still have the problem 
that you get exponential inflation of the size of preprocessed output if calls 
to these macros are lexically nested too deeply.

Cons:
 - you have to do all of your codegen inside the clang binary, starting from 
the function operands you're given, and ending up with LLVM IR. You don't get 
to do the tedious parts (like unpacking structs, or dereferencing pointer 
arguments for passed-by-reference parameters) in the wrapper function 

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

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

balazske added a comment.

Some explanation of the error states:

- `EofError` indicates an EOF condition in the stream.
- `OtherError` indicates a generic (I/O or other but not EOF) error.
- `AnyError` is a "placeholder" if the exact error kind is not important. This 
is a "Schrödinger's cat" type of state: If we need to observe the exact error 
it can change to Eof or Other error. (See code in `evalFeof` and `evalFerror`, 
probably this is the only place for this event.) This error kind is used to 
save state splits, other solution is to make for example after fseek a new 
state for `EofError` and another for `OtherError`. The file error functions are 
relatively seldomly used (?), because checking success of an operation can be 
done with the return value and there is no need to check `ferror` or `feof`. 
And for analyzer warnings it is often enough to know if any error happened, not 
if exactly **EOF** or other error.


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] D75682: [Analyzer][StreamChecker] Introduction of stream error handling.

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

balazske added a comment.

We can not make a warning if a stream operation is called in failed state: The 
error was probably handled based on the return value of the previous failed 
operation. This thing is checked by the "ErrorReturnChecker" 
(https://reviews.llvm.org/D72705). Only some special functions like `getc` are 
to be handled specially where the error state has importance.


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] D75685: Add MS Mangling for OpenCL Pipe types, add mangling test.

[Erich Keane via Phabricator via cfe-commits]

erichkeane created this revision.
erichkeane added reviewers: bader, Anastasia, pxli168.
Herald added a subscriber: yaxunl.

SPIRV2.0 Spec only specifies Linux mangling, however our downstream has
use for a Windows mangling for these types.

Unfortunately, the SPIRV
spec specifies a single mangling for all pipe types, despite clang
allowing overloading on these types.  Because of this, this patch
chooses to mangle the read/writability and element type for the windows
mangling.

The windows manglings in the test all demangle according to demangler:
"void __cdecl test1(struct __clang::ocl_pipe)
"void __cdecl test2(struct __clang::ocl_pipe)
"void __cdecl test2(struct __clang::ocl_pipe)
"void __cdecl test3(struct __clang::ocl_pipe)
"void __cdecl test4(struct __clang::ocl_pipe,1>)
"void __cdecl test5(struct __clang::ocl_pipe,1>)
"void __cdecl test_reserved_read_pipe(struct __clang::_ASCLglobal * __ptr64,struct __clang::ocl_pipe)


https://reviews.llvm.org/D75685

Files:
  clang/lib/AST/MicrosoftMangle.cpp
  clang/test/CodeGenOpenCLCXX/pipe_types_mangling.cl


Index: clang/test/CodeGenOpenCLCXX/pipe_types_mangling.cl
===
--- /dev/null
+++ clang/test/CodeGenOpenCLCXX/pipe_types_mangling.cl
@@ -0,0 +1,57 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -O0 
-cl-std=clc++ -o - %s | FileCheck %s --check-prefixes=LINUX
+// RUN: %clang_cc1 -triple x86_64-unknown-windows-pc -emit-llvm -O0 
-cl-std=clc++ -o - %s -DWIN| FileCheck %s --check-prefixes=WINDOWS
+
+typedef unsigned char __attribute__((ext_vector_type(3))) uchar3;
+typedef int __attribute__((ext_vector_type(4))) int4;
+
+void test1(read_only pipe int p) {
+// LINUX: define void @_Z5test18ocl_pipe
+// WINDOWS: define dso_local void @"?test1@@YAXU?$ocl_pipe@H$00@__clang@@@Z"
+}
+
+void test2(write_only pipe float p) {
+// LINUX: define void @_Z5test28ocl_pipe
+// WINDOWS: define dso_local void @"?test2@@YAXU?$ocl_pipe@M$0A@@__clang@@@Z"
+}
+
+#ifdef WIN
+// SPIR Spec specifies mangling on pipes that doesn't include the element type
+//  or write/read. Our Windows mangling does, so make sure this still works.
+void test2(read_only pipe int p) {
+// WINDOWS: define dso_local void @"?test2@@YAXU?$ocl_pipe@H$00@__clang@@@Z"
+}
+#endif
+
+
+void test3(read_only pipe const int p) {
+// LINUX: define void @_Z5test38ocl_pipe
+// WINDOWS: define dso_local void 
@"?test3@@YAXU?$ocl_pipe@$$CBH$00@__clang@@@Z"
+}
+
+void test4(read_only pipe uchar3 p) {
+// LINUX: define void @_Z5test48ocl_pipe
+// WINDOWS: define dso_local void 
@"?test4@@YAXU?$ocl_pipe@T?$__vector@E$02@__clang@@$00@__clang@@@Z"
+}
+
+void test5(read_only pipe int4 p) {
+// LINUX: define void @_Z5test58ocl_pipe
+// WINDOWS: define dso_local void 
@"?test5@@YAXU?$ocl_pipe@T?$__vector@H$03@__clang@@$00@__clang@@@Z"
+}
+
+typedef read_only pipe int MyPipe;
+kernel void test6(MyPipe p) {
+// LINUX: define spir_kernel void @test6
+// WINDOWS: define dso_local spir_kernel void @test6
+}
+
+struct Person {
+  const char *Name;
+  bool isFemale;
+  int ID;
+};
+
+void test_reserved_read_pipe(global struct Person *SDst,
+ read_only pipe struct Person SPipe) {
+// LINUX: define void @_Z23test_reserved_read_pipePU8CLglobal6Person8ocl_pipe
+// WINDOWS: define dso_local void 
@"?test_reserved_read_pipe@@YAXPEAU?$_ASCLglobal@$$CAUPerson@@@__clang@@U?$ocl_pipe@UPerson@@$00@2@@Z"
+}
Index: clang/lib/AST/MicrosoftMangle.cpp
===
--- clang/lib/AST/MicrosoftMangle.cpp
+++ clang/lib/AST/MicrosoftMangle.cpp
@@ -2943,11 +2943,17 @@
 
 void MicrosoftCXXNameMangler::mangleType(const PipeType *T, Qualifiers,
  SourceRange Range) {
-  DiagnosticsEngine &Diags = Context.getDiags();
-  unsigned DiagID = Diags.getCustomDiagID(DiagnosticsEngine::Error,
-"cannot mangle this OpenCL pipe type yet");
-  Diags.Report(Range.getBegin(), DiagID)
-<< Range;
+  QualType ElementType = T->getElementType();
+
+  llvm::SmallString<64> TemplateMangling;
+  llvm::raw_svector_ostream Stream(TemplateMangling);
+  MicrosoftCXXNameMangler Extra(Context, Stream);
+  Stream << "?$";
+  Extra.mangleSourceName("ocl_pipe");
+  Extra.mangleType(ElementType, Range, QMM_Escape);
+  Extra.mangleIntegerLiteral(llvm::APSInt::get(T->isReadOnly()), true);
+
+  mangleArtificialTagType(TTK_Struct, TemplateMangling, {"__clang"});
 }
 
 void MicrosoftMangleContextImpl::mangleCXXName(const NamedDecl *D,


Index: clang/test/CodeGenOpenCLCXX/pipe_types_mangling.cl
===
--- /dev/null
+++ clang/test/CodeGenOpenCLCXX/pipe_types_mangling.cl
@@ -0,0 +1,57 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -O0 -cl-std=clc++ -o - %s | FileCheck %s --check-prefixes=LINUX
+// RUN: %clang_cc1 -triple x86_64-unknown-windows-pc -emit-llvm -O0 -cl-std=clc++ -o - %s -DWIN| FileCheck %s --check-

[PATCH] D64464: [CodeGen] Emit destructor calls to destruct compound literals

[Gabor Marton via Phabricator via cfe-commits]

martong added inline comments.



Comment at: clang/lib/AST/ASTImporter.cpp:7918
+
+  return nullptr;
+}

I think 
```
return make_errorhttps://reviews.llvm.org/D64464/new/

https://reviews.llvm.org/D64464



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

[PATCH] D74617: [ARM] Keeping sign information on bitcasts for Neon vdot_lane intrinsics

[Lucas Prates via Phabricator via cfe-commits]

pratlucas added a comment.

This patch by itself is a NFC. It aims to enable the proper overload resolution 
of intrinsic calls when using the `call_mangled` operation introduced on D74618 
.
To make this more clear, I'll discard this review and merge this changes into 
D74619 , where `call_mangled` gets used.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74617



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

[PATCH] D74166: [AIX][Frontend] Static init implementation for AIX considering no priority

[Xiangling Liao via Phabricator via cfe-commits]

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



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:283
   llvm::FunctionCallee atexit =
-  CGM.CreateRuntimeFunction(atexitTy, "atexit", llvm::AttributeList(),
-/*Local=*/true);
+  CGM.CreateRuntimeFunction(atexitTy, "atexit", llvm::AttributeList());
   if (llvm::Function *atexitFn = dyn_cast(atexit.getCallee()))

sfertile wrote:
> The default value for `Local` is false, was this change intentional? If so 
> why is it needed?
Thanks for pointing this out. I believe this is a bug. I was supposed to only 
let `Local` in `unregisterGlobalDtorWithUnAtExit` as default value `false`. 
Because it is only used in relation to Windows. 





Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:531
+
+  bool isCXXGlobalInitAndDtorFuncInternal() const override { return false; }
+

yusra.syeda wrote:
> Perhaps adding a check to see if the OS is AIX before setting linkage to 
> external will be useful here.
Since we are already under the context that `XLCXXABI` is an AIX C++ ABI, I 
kinda feel it's a duplication to add OS check.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74166



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

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

[Sam McCall via Phabricator via cfe-commits]

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

Conservatively escaping everything is bad in coc.nvim which shows the markdown
to the user, and we have reports of it causing problems for other parsers.

Fixes https://github.com/clangd/clangd/issues/301


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D75687

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

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

[libunwind] 470f995 - Promote nameless lambda used by dl_iterate_phdr to named function.

[Sterling Augustine via cfe-commits]

Author: Sterling Augustine
Date: 2020-03-05T08:55:22-08:00
New Revision: 470f995a517f5dbb53b1f5cd87ca3c9be0b32d79

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

LOG: Promote nameless lambda used by dl_iterate_phdr to named function.

Summary:
This cleans up control flow inside findUnwindSections, and will make
it easier to replace this code in a following patch. Also, expose the
data structure to allow use by a future replacment function.

Reviewers: mstorsjo, miyuki

Subscribers: krytarowski, libcxx-commits

Tags: #libc

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

Added: 


Modified: 
libunwind/src/AddressSpace.hpp

Removed: 




diff  --git a/libunwind/src/AddressSpace.hpp b/libunwind/src/AddressSpace.hpp
index 7433476f9117..2f68d6525a0f 100644
--- a/libunwind/src/AddressSpace.hpp
+++ b/libunwind/src/AddressSpace.hpp
@@ -392,6 +392,127 @@ LocalAddressSpace::getEncodedP(pint_t &addr, pint_t end, 
uint8_t encoding,
   return result;
 }
 
+#ifdef __APPLE__
+#elif defined(_LIBUNWIND_SUPPORT_DWARF_UNWIND) && 
defined(_LIBUNWIND_IS_BAREMETAL)
+#elif defined(_LIBUNWIND_ARM_EHABI) && defined(_LIBUNWIND_IS_BAREMETAL)
+#elif defined(_LIBUNWIND_SUPPORT_DWARF_UNWIND) && defined(_WIN32)
+#elif defined(_LIBUNWIND_SUPPORT_SEH_UNWIND) && defined(_WIN32)
+#elif defined(_LIBUNWIND_ARM_EHABI) && defined(__BIONIC__)
+// Code inside findUnwindSections handles all these cases.
+//
+// Although the above ifdef chain is ugly, there doesn't seem to be a cleaner
+// way to handle it. The generalized boolean expression is:
+//
+//  A OR (B AND C) OR (D AND C) OR (B AND E) OR (F AND E) OR (D AND G)
+//
+// Running it through various boolean expression simplifiers gives expressions
+// that don't help at all.
+#elif defined(_LIBUNWIND_ARM_EHABI) || defined(_LIBUNWIND_SUPPORT_DWARF_UNWIND)
+
+struct _LIBUNWIND_HIDDEN dl_iterate_cb_data {
+  LocalAddressSpace *addressSpace;
+  UnwindInfoSections *sects;
+  uintptr_t targetAddr;
+};
+
+int findUnwindSectionsByPhdr(struct dl_phdr_info *pinfo, size_t, void *data) {
+  auto cbdata = static_cast(data);
+  bool found_obj = false;
+  bool found_hdr = false;
+
+  assert(cbdata);
+  assert(cbdata->sects);
+
+  if (cbdata->targetAddr < pinfo->dlpi_addr) {
+return false;
+  }
+
+#if !defined(Elf_Half)
+  typedef ElfW(Half) Elf_Half;
+#endif
+#if !defined(Elf_Phdr)
+  typedef ElfW(Phdr) Elf_Phdr;
+#endif
+#if !defined(Elf_Addr)
+  typedef ElfW(Addr) Elf_Addr;
+#endif
+
+  Elf_Addr image_base = pinfo->dlpi_addr;
+
+#if defined(__ANDROID__) && __ANDROID_API__ < 18
+  if (image_base == 0) {
+// Normally, an image base of 0 indicates a non-PIE executable. On
+// versions of Android prior to API 18, the dynamic linker reported a
+// dlpi_addr of 0 for PIE executables. Compute the true image base
+// using the PT_PHDR segment.
+// See https://github.com/android/ndk/issues/505.
+for (Elf_Half i = 0; i < pinfo->dlpi_phnum; i++) {
+  const Elf_Phdr *phdr = &pinfo->dlpi_phdr[i];
+  if (phdr->p_type == PT_PHDR) {
+image_base = reinterpret_cast(pinfo->dlpi_phdr) -
+  phdr->p_vaddr;
+break;
+  }
+}
+  }
+#endif
+
+ #if defined(_LIBUNWIND_SUPPORT_DWARF_UNWIND)
+  #if !defined(_LIBUNWIND_SUPPORT_DWARF_INDEX)
+   #error "_LIBUNWIND_SUPPORT_DWARF_UNWIND requires 
_LIBUNWIND_SUPPORT_DWARF_INDEX on this platform."
+  #endif
+  size_t object_length;
+
+  for (Elf_Half i = 0; i < pinfo->dlpi_phnum; i++) {
+const Elf_Phdr *phdr = &pinfo->dlpi_phdr[i];
+if (phdr->p_type == PT_LOAD) {
+  uintptr_t begin = image_base + phdr->p_vaddr;
+  uintptr_t end = begin + phdr->p_memsz;
+  if (cbdata->targetAddr >= begin && cbdata->targetAddr < end) {
+cbdata->sects->dso_base = begin;
+object_length = phdr->p_memsz;
+found_obj = true;
+  }
+} else if (phdr->p_type == PT_GNU_EH_FRAME) {
+  EHHeaderParser::EHHeaderInfo hdrInfo;
+  uintptr_t eh_frame_hdr_start = image_base + phdr->p_vaddr;
+  cbdata->sects->dwarf_index_section = eh_frame_hdr_start;
+  cbdata->sects->dwarf_index_section_length = phdr->p_memsz;
+  found_hdr = EHHeaderParser::decodeEHHdr(
+  *cbdata->addressSpace, eh_frame_hdr_start, phdr->p_memsz,
+  hdrInfo);
+  if (found_hdr)
+cbdata->sects->dwarf_section = hdrInfo.eh_frame_ptr;
+}
+  }
+
+  if (found_obj && found_hdr) {
+cbdata->sects->dwarf_section_length = object_length;
+return true;
+  } else {
+return false;
+  }
+ #else // defined(_LIBUNWIND_ARM_EHABI)
+  for (Elf_Half i = 0; i < pinfo->dlpi_phnum; i++) {
+const Elf_Phdr *phdr = &pinfo->dlpi_phdr[i];
+if (phdr->p_type == PT_LOAD) {
+  uintptr_t begin = image_base + phdr->p_vaddr;
+  uintptr_t end = begin + phdr->p_memsz;
+

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

[Sam McCall via Phabricator via cfe-commits]

sammccall updated this revision to Diff 248493.
sammccall added a comment.

couple more tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75687

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

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

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

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

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

Updated according to the comments.


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

https://reviews.llvm.org/D75514

Files:
  clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp
  clang/lib/StaticAnalyzer/Checkers/DebugContainerModeling.cpp
  clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
  clang/test/Analysis/container-modeling.cpp

Index: clang/test/Analysis/container-modeling.cpp
===
--- clang/test/Analysis/container-modeling.cpp
+++ clang/test/Analysis/container-modeling.cpp
@@ -97,7 +97,8 @@
   clang_analyzer_denote(clang_analyzer_container_begin(V), "$V.begin()");
   clang_analyzer_denote(clang_analyzer_container_end(V), "$V.end()");
 
-  V.emplace_back(n); // expected-note 2{{Container 'V' extended to the right by 1 position}}
+  V.emplace_back(n); // expected-note{{Container 'V' extended to the right by 1 position}}
+ // expected-note@-1{{Container 'V' extended to the right by 1 position}}
 
 
   clang_analyzer_express(clang_analyzer_container_begin(V)); // expected-warning{{$V.begin()}}
@@ -118,7 +119,8 @@
   clang_analyzer_denote(clang_analyzer_container_begin(V), "$V.begin()");
   clang_analyzer_denote(clang_analyzer_container_end(V), "$V.end()");
 
-  V.pop_back(); // expected-note 2{{Container 'V' shrank from the right by 1 position}}
+  V.pop_back(); // expected-note{{Container 'V' shrank from the right by 1 position}}
+// expected-note@-1{{Container 'V' shrank from the right by 1 position}}
 
 
   clang_analyzer_express(clang_analyzer_container_begin(V)); // expected-warning{{$V.begin()}}
@@ -139,7 +141,8 @@
   clang_analyzer_denote(clang_analyzer_container_begin(L), "$L.begin()");
   clang_analyzer_denote(clang_analyzer_container_end(L), "$L.end()");
 
-  L.push_front(n); // expected-note 2{{Container 'L' extended to the left by 1 position}}
+  L.push_front(n); // expected-note{{Container 'L' extended to the left by 1 position}}
+   // expected-note@-1{{Container 'L' extended to the left by 1 position}}
 
   clang_analyzer_express(clang_analyzer_container_begin(L)); // expected-warning{{$L.begin() - 1}}
  // expected-note@-1{{$L.begin() - 1}}
@@ -159,7 +162,8 @@
   clang_analyzer_denote(clang_analyzer_container_begin(L), "$L.begin()");
   clang_analyzer_denote(clang_analyzer_container_end(L), "$L.end()");
 
-  L.emplace_front(n); // expected-note 2{{Container 'L' extended to the left by 1 position}}
+  L.emplace_front(n); // expected-note{{Container 'L' extended to the left by 1 position}}
+  // expected-note@-1{{Container 'L' extended to the left by 1 position}}
 
   clang_analyzer_express(clang_analyzer_container_begin(L)); // expected-warning{{$L.begin() - 1}}
  // expected-note@-1{{$L.begin() - 1}}
@@ -179,7 +183,8 @@
   clang_analyzer_denote(clang_analyzer_container_begin(L), "$L.begin()");
   clang_analyzer_denote(clang_analyzer_container_end(L), "$L.end()");
 
-  L.pop_front(); // expected-note 2{{Container 'L' shrank from the left by 1 position}}
+  L.pop_front(); // expected-note{{Container 'L' shrank from the left by 1 position}}
+ // expected-note@-1{{Container 'L' shrank from the left by 1 position}}
 
   clang_analyzer_express(clang_analyzer_container_begin(L)); // expected-warning{{$L.begin() + 1}}
  // expected-note@-1{{$L.begin() + 1}}
@@ -203,10 +208,10 @@
 
   clang_analyzer_denote(clang_analyzer_container_begin(V1), "$V1.begin()");
 
-  V2.push_back(n); // expected-note{{Container 'V2' extended to the right by 1 position}} FIXME: This note should not appear since `V2` is not affected in the "bug"
+  V2.push_back(n); // no-note
 
   clang_analyzer_express(clang_analyzer_container_begin(V1)); // expected-warning{{$V1.begin()}}
- // expected-note@-1{{$V1.begin()}}
+  // expected-note@-1{{$V1.begin()}}
 }
 
 void push_back2(std::vector &V1, std::vector &V2, int n) {
@@ -218,15 +223,14 @@
   clang_analyzer_denote(clang_analyzer_container_begin(V1), "$V1.begin()");
   clang_analyzer_denote(clang_analyzer_container_begin(V2), "$V2.begin()");
 
-  V1.push_back(n); // expected-note 2{{Container 'V1' extended to the right by 1 position}}
-   // FIXME: This should appear only once since there is only
-   // one "bug" where `V1` is affected
+  V1.push_back(n); // expected-note{{Container 'V1' extended to the right by 1 position}}
+   // Only once!
 
   clang_analyzer_express(clang_analyzer_container_begin(V1)); // expected-warning{{$V1.begin()}}
-   

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

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

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

Minor style change.


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

https://reviews.llvm.org/D73720

Files:
  clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp
  clang/lib/StaticAnalyzer/Checkers/IteratorRangeChecker.cpp
  clang/test/Analysis/container-modeling.cpp
  clang/test/Analysis/iterator-range.cpp

Index: clang/test/Analysis/iterator-range.cpp
===
--- clang/test/Analysis/iterator-range.cpp
+++ clang/test/Analysis/iterator-range.cpp
@@ -1,5 +1,5 @@
-// RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,cplusplus,alpha.cplusplus.IteratorRange -analyzer-config aggressive-binary-operation-simplification=true -analyzer-config c++-container-inlining=false %s -verify
-// RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,cplusplus,alpha.cplusplus.IteratorRange -analyzer-config aggressive-binary-operation-simplification=true -analyzer-config c++-container-inlining=true -DINLINE=1 %s -verify
+// RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,cplusplus,alpha.cplusplus.IteratorRange -analyzer-config aggressive-binary-operation-simplification=true -analyzer-config c++-container-inlining=false -analyzer-output=text %s -verify
+// RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,cplusplus,alpha.cplusplus.IteratorRange -analyzer-config aggressive-binary-operation-simplification=true -analyzer-config c++-container-inlining=true -DINLINE=1 -analyzer-output=text %s -verify
 
 #include "Inputs/system-header-simulator-cxx.h"
 
@@ -32,6 +32,7 @@
 void deref_end(const std::vector &V) {
   auto i = V.end();
   *i; // expected-warning{{Past-the-end iterator dereferenced}}
+  // expected-note@-1{{Past-the-end iterator dereferenced}}
 }
 
 // Prefix increment - operator++()
@@ -59,6 +60,7 @@
 void incr_end(const std::vector &V) {
   auto i = V.end();
   ++i; // expected-warning{{Iterator incremented behind the past-the-end iterator}}
+   // expected-note@-1{{Iterator incremented behind the past-the-end iterator}}
 }
 
 // Postfix increment - operator++(int)
@@ -86,6 +88,7 @@
 void end_incr(const std::vector &V) {
   auto i = V.end();
   i++; // expected-warning{{Iterator incremented behind the past-the-end iterator}}
+   // expected-note@-1{{Iterator incremented behind the past-the-end iterator}}
 }
 
 // Prefix decrement - operator--()
@@ -93,6 +96,7 @@
 void decr_begin(const std::vector &V) {
   auto i = V.begin();
   --i; // expected-warning{{Iterator decremented ahead of its valid range}}
+   // expected-note@-1{{Iterator decremented ahead of its valid range}}
 }
 
 void decr_behind_begin(const std::vector &V) {
@@ -120,6 +124,7 @@
 void begin_decr(const std::vector &V) {
   auto i = V.begin();
   i--; // expected-warning{{Iterator decremented ahead of its valid range}}
+   // expected-note@-1{{Iterator decremented ahead of its valid range}}
 }
 
 void behind_begin_decr(const std::vector &V) {
@@ -168,11 +173,13 @@
 void incr_by_2_ahead_of_end(const std::vector &V) {
   auto i = --V.end();
   i += 2; // expected-warning{{Iterator incremented behind the past-the-end iterator}}
+  // expected-note@-1{{Iterator incremented behind the past-the-end iterator}}
 }
 
 void incr_by_2_end(const std::vector &V) {
   auto i = V.end();
   i += 2; // expected-warning{{Iterator incremented behind the past-the-end iterator}}
+  // expected-note@-1{{Iterator incremented behind the past-the-end iterator}}
 }
 
 // Addition - operator+(int)
@@ -201,11 +208,13 @@
 void incr_by_2_copy_ahead_of_end(const std::vector &V) {
   auto i = --V.end();
   auto j = i + 2; // expected-warning{{Iterator incremented behind the past-the-end iterator}}
+  // expected-note@-1{{Iterator incremented behind the past-the-end iterator}}
 }
 
 void incr_by_2_copy_end(const std::vector &V) {
   auto i = V.end();
   auto j = i + 2; // expected-warning{{Iterator incremented behind the past-the-end iterator}}
+  // expected-note@-1{{Iterator incremented behind the past-the-end iterator}}
 }
 
 // Subtraction assignment - operator-=(int)
@@ -213,11 +222,13 @@
 void decr_by_2_begin(const std::vector &V) {
   auto i = V.begin();
   i -= 2; // expected-warning{{Iterator decremented ahead of its valid range}}
+  // expected-note@-1{{Iterator decremented ahead of its valid range}}
 }
 
 void decr_by_2_behind_begin(const std::vector &V) {
   auto i = ++V.begin();
   i -= 2; // expected-warning{{Iterator decremented ahead of its valid range}}
+  // expected-note@-1{{Iterator decremented ahead of its valid range}}
 }
 
 void decr_by_2_behind_begin_by_2(const std::vector &V) {
@@ -246,11 +257,13 @@
 void decr_by_2_copy_begin(const std::vector &V) {
   auto i = V.begin();
   auto j = i - 2; // expected-warning{{Iterator decremented ahead of its valid range}}
+  // 

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

[Sam McCall via Phabricator via cfe-commits]

sammccall updated this revision to Diff 248497.
sammccall marked 2 inline comments as done.
sammccall added a comment.

address review comments


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] D75514: [Analyzer] Only add container note tags to the operations of the affected container

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

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

Updated according to the comments.


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

https://reviews.llvm.org/D75514

Files:
  clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp
  clang/lib/StaticAnalyzer/Checkers/DebugContainerModeling.cpp
  clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
  clang/test/Analysis/container-modeling.cpp

Index: clang/test/Analysis/container-modeling.cpp
===
--- clang/test/Analysis/container-modeling.cpp
+++ clang/test/Analysis/container-modeling.cpp
@@ -97,7 +97,8 @@
   clang_analyzer_denote(clang_analyzer_container_begin(V), "$V.begin()");
   clang_analyzer_denote(clang_analyzer_container_end(V), "$V.end()");
 
-  V.emplace_back(n); // expected-note 2{{Container 'V' extended to the right by 1 position}}
+  V.emplace_back(n); // expected-note{{Container 'V' extended to the right by 1 position}}
+ // expected-note@-1{{Container 'V' extended to the right by 1 position}}
 
 
   clang_analyzer_express(clang_analyzer_container_begin(V)); // expected-warning{{$V.begin()}}
@@ -118,7 +119,8 @@
   clang_analyzer_denote(clang_analyzer_container_begin(V), "$V.begin()");
   clang_analyzer_denote(clang_analyzer_container_end(V), "$V.end()");
 
-  V.pop_back(); // expected-note 2{{Container 'V' shrank from the right by 1 position}}
+  V.pop_back(); // expected-note{{Container 'V' shrank from the right by 1 position}}
+// expected-note@-1{{Container 'V' shrank from the right by 1 position}}
 
 
   clang_analyzer_express(clang_analyzer_container_begin(V)); // expected-warning{{$V.begin()}}
@@ -139,7 +141,8 @@
   clang_analyzer_denote(clang_analyzer_container_begin(L), "$L.begin()");
   clang_analyzer_denote(clang_analyzer_container_end(L), "$L.end()");
 
-  L.push_front(n); // expected-note 2{{Container 'L' extended to the left by 1 position}}
+  L.push_front(n); // expected-note{{Container 'L' extended to the left by 1 position}}
+   // expected-note@-1{{Container 'L' extended to the left by 1 position}}
 
   clang_analyzer_express(clang_analyzer_container_begin(L)); // expected-warning{{$L.begin() - 1}}
  // expected-note@-1{{$L.begin() - 1}}
@@ -159,7 +162,8 @@
   clang_analyzer_denote(clang_analyzer_container_begin(L), "$L.begin()");
   clang_analyzer_denote(clang_analyzer_container_end(L), "$L.end()");
 
-  L.emplace_front(n); // expected-note 2{{Container 'L' extended to the left by 1 position}}
+  L.emplace_front(n); // expected-note{{Container 'L' extended to the left by 1 position}}
+  // expected-note@-1{{Container 'L' extended to the left by 1 position}}
 
   clang_analyzer_express(clang_analyzer_container_begin(L)); // expected-warning{{$L.begin() - 1}}
  // expected-note@-1{{$L.begin() - 1}}
@@ -179,7 +183,8 @@
   clang_analyzer_denote(clang_analyzer_container_begin(L), "$L.begin()");
   clang_analyzer_denote(clang_analyzer_container_end(L), "$L.end()");
 
-  L.pop_front(); // expected-note 2{{Container 'L' shrank from the left by 1 position}}
+  L.pop_front(); // expected-note{{Container 'L' shrank from the left by 1 position}}
+ // expected-note@-1{{Container 'L' shrank from the left by 1 position}}
 
   clang_analyzer_express(clang_analyzer_container_begin(L)); // expected-warning{{$L.begin() + 1}}
  // expected-note@-1{{$L.begin() + 1}}
@@ -203,10 +208,10 @@
 
   clang_analyzer_denote(clang_analyzer_container_begin(V1), "$V1.begin()");
 
-  V2.push_back(n); // expected-note{{Container 'V2' extended to the right by 1 position}} FIXME: This note should not appear since `V2` is not affected in the "bug"
+  V2.push_back(n); // no-note
 
   clang_analyzer_express(clang_analyzer_container_begin(V1)); // expected-warning{{$V1.begin()}}
- // expected-note@-1{{$V1.begin()}}
+  // expected-note@-1{{$V1.begin()}}
 }
 
 void push_back2(std::vector &V1, std::vector &V2, int n) {
@@ -218,15 +223,14 @@
   clang_analyzer_denote(clang_analyzer_container_begin(V1), "$V1.begin()");
   clang_analyzer_denote(clang_analyzer_container_begin(V2), "$V2.begin()");
 
-  V1.push_back(n); // expected-note 2{{Container 'V1' extended to the right by 1 position}}
-   // FIXME: This should appear only once since there is only
-   // one "bug" where `V1` is affected
+  V1.push_back(n); // expected-note{{Container 'V1' extended to the right by 1 position}}
+   // Only once!
 
   clang_analyzer_express(clang_analyzer_container_begin(V1)); // expected-warning{{$V1.begin()}}
-   

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

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

baloghadamsoftware updated this revision to Diff 248502.
baloghadamsoftware marked an inline comment as done.
baloghadamsoftware added a comment.

Typo fixed.


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

https://reviews.llvm.org/D75514

Files:
  clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp
  clang/lib/StaticAnalyzer/Checkers/DebugContainerModeling.cpp
  clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
  clang/test/Analysis/container-modeling.cpp

Index: clang/test/Analysis/container-modeling.cpp
===
--- clang/test/Analysis/container-modeling.cpp
+++ clang/test/Analysis/container-modeling.cpp
@@ -97,7 +97,8 @@
   clang_analyzer_denote(clang_analyzer_container_begin(V), "$V.begin()");
   clang_analyzer_denote(clang_analyzer_container_end(V), "$V.end()");
 
-  V.emplace_back(n); // expected-note 2{{Container 'V' extended to the right by 1 position}}
+  V.emplace_back(n); // expected-note{{Container 'V' extended to the right by 1 position}}
+ // expected-note@-1{{Container 'V' extended to the right by 1 position}}
 
 
   clang_analyzer_express(clang_analyzer_container_begin(V)); // expected-warning{{$V.begin()}}
@@ -118,7 +119,8 @@
   clang_analyzer_denote(clang_analyzer_container_begin(V), "$V.begin()");
   clang_analyzer_denote(clang_analyzer_container_end(V), "$V.end()");
 
-  V.pop_back(); // expected-note 2{{Container 'V' shrank from the right by 1 position}}
+  V.pop_back(); // expected-note{{Container 'V' shrank from the right by 1 position}}
+// expected-note@-1{{Container 'V' shrank from the right by 1 position}}
 
 
   clang_analyzer_express(clang_analyzer_container_begin(V)); // expected-warning{{$V.begin()}}
@@ -139,7 +141,8 @@
   clang_analyzer_denote(clang_analyzer_container_begin(L), "$L.begin()");
   clang_analyzer_denote(clang_analyzer_container_end(L), "$L.end()");
 
-  L.push_front(n); // expected-note 2{{Container 'L' extended to the left by 1 position}}
+  L.push_front(n); // expected-note{{Container 'L' extended to the left by 1 position}}
+   // expected-note@-1{{Container 'L' extended to the left by 1 position}}
 
   clang_analyzer_express(clang_analyzer_container_begin(L)); // expected-warning{{$L.begin() - 1}}
  // expected-note@-1{{$L.begin() - 1}}
@@ -159,7 +162,8 @@
   clang_analyzer_denote(clang_analyzer_container_begin(L), "$L.begin()");
   clang_analyzer_denote(clang_analyzer_container_end(L), "$L.end()");
 
-  L.emplace_front(n); // expected-note 2{{Container 'L' extended to the left by 1 position}}
+  L.emplace_front(n); // expected-note{{Container 'L' extended to the left by 1 position}}
+  // expected-note@-1{{Container 'L' extended to the left by 1 position}}
 
   clang_analyzer_express(clang_analyzer_container_begin(L)); // expected-warning{{$L.begin() - 1}}
  // expected-note@-1{{$L.begin() - 1}}
@@ -179,7 +183,8 @@
   clang_analyzer_denote(clang_analyzer_container_begin(L), "$L.begin()");
   clang_analyzer_denote(clang_analyzer_container_end(L), "$L.end()");
 
-  L.pop_front(); // expected-note 2{{Container 'L' shrank from the left by 1 position}}
+  L.pop_front(); // expected-note{{Container 'L' shrank from the left by 1 position}}
+ // expected-note@-1{{Container 'L' shrank from the left by 1 position}}
 
   clang_analyzer_express(clang_analyzer_container_begin(L)); // expected-warning{{$L.begin() + 1}}
  // expected-note@-1{{$L.begin() + 1}}
@@ -203,10 +208,10 @@
 
   clang_analyzer_denote(clang_analyzer_container_begin(V1), "$V1.begin()");
 
-  V2.push_back(n); // expected-note{{Container 'V2' extended to the right by 1 position}} FIXME: This note should not appear since `V2` is not affected in the "bug"
+  V2.push_back(n); // no-note
 
   clang_analyzer_express(clang_analyzer_container_begin(V1)); // expected-warning{{$V1.begin()}}
- // expected-note@-1{{$V1.begin()}}
+  // expected-note@-1{{$V1.begin()}}
 }
 
 void push_back2(std::vector &V1, std::vector &V2, int n) {
@@ -218,15 +223,14 @@
   clang_analyzer_denote(clang_analyzer_container_begin(V1), "$V1.begin()");
   clang_analyzer_denote(clang_analyzer_container_begin(V2), "$V2.begin()");
 
-  V1.push_back(n); // expected-note 2{{Container 'V1' extended to the right by 1 position}}
-   // FIXME: This should appear only once since there is only
-   // one "bug" where `V1` is affected
+  V1.push_back(n); // expected-note{{Container 'V1' extended to the right by 1 position}}
+   // Only once!
 
   clang_analyzer_express(clang_analyzer_container_begin(V1)); // expected-warning{{$V1.

[PATCH] D75690: [SVE][Inline-Asm] Add constraints for SVE ACLE types

[Kerry McLaughlin via Phabricator via cfe-commits]

kmclaughlin created this revision.
kmclaughlin added reviewers: sdesmalen, huntergr, rovka, cameron.mcinally, 
efriedma.
Herald added subscribers: cfe-commits, psnobl, rkruppe, tschuett.
Herald added a reviewer: rengolin.
Herald added a project: clang.
kmclaughlin added a parent revision: D75297: [TypeSize] Allow returning 
scalable size in implicit conversion to uint64_t.

Adds the constraints described below to ensure that we
can tie variables of SVE ACLE types to operands in inline-asm:

- y: SVE registers Z0-Z7
- Upl: One of the low eight SVE predicate registers (P0-P7)
- Upa: Full range of SVE predicate registers (P0-P15)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D75690

Files:
  clang/lib/Basic/Targets/AArch64.cpp
  clang/lib/Basic/Targets/AArch64.h
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/test/CodeGen/aarch64-sve-inline-asm-crash.c
  clang/test/CodeGen/aarch64-sve-inline-asm-datatypes.c
  clang/test/CodeGen/aarch64-sve-inline-asm-negative-test.c
  clang/test/CodeGen/aarch64-sve-inline-asm-vec-low.c

Index: clang/test/CodeGen/aarch64-sve-inline-asm-vec-low.c
===
--- /dev/null
+++ clang/test/CodeGen/aarch64-sve-inline-asm-vec-low.c
@@ -0,0 +1,39 @@
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -fallow-half-arguments-and-returns -S -O1 -Werror -W -Wall -o - %s | FileCheck %s
+
+#define svfcmla_test_f16(Zda, Zn, Zm, rot, imm3) \
+({ \
+__SVFloat16_t _Zn = Zn;\
+__SVFloat16_t _Zm = Zm;\
+__SVFloat16_t _res = Zda;\
+__asm__("fcmla %[__res].h, %[__Zn].h, %[__Zm].h[" #imm3 "], %[__rot]"\
+: [__res] "+w" (_res) \
+: [__Zn] "w" (_Zn), [__Zm] "y" (_Zm), [__rot] "i" (rot) \
+:  \
+);\
+_res; \
+})
+
+
+// CHECK: fcmla {{z[0-9]+\.h}}, {{z[0-9]+\.h}}, {{z[0-7]\.h}}{{\[[0-9]+\]}}, #270
+__SVFloat16_t test_svfcmla_lane_f16(__SVFloat16_t aZda, __SVFloat16_t aZn, __SVFloat16_t aZm) {
+return svfcmla_test_f16(aZda, aZn, aZm, 270, 0);
+}
+
+#define svfcmla_test_f32(Zda, Zn, Zm, rot, imm3) \
+({ \
+__SVFloat32_t _Zn = Zn;\
+__SVFloat32_t _Zm = Zm;\
+__SVFloat32_t _res = Zda;\
+__asm__("fcmla %[__res].s, %[__Zn].s, %[__Zm].s[" #imm3 "], %[__rot]"\
+: [__res] "+w" (_res) \
+: [__Zn] "w" (_Zn), [__Zm] "x" (_Zm), [__rot] "i" (rot) \
+:  \
+);\
+_res; \
+})
+
+
+// CHECK: fcmla {{z[0-9]+\.s}}, {{z[0-9]+\.s}}, {{z[0-9][0-5]?\.s}}{{\[[0-9]+\]}}, #270
+__SVFloat32_t test_svfcmla_lane_f(__SVFloat32_t aZda, __SVFloat32_t aZn, __SVFloat32_t aZm) {
+return svfcmla_test_f32(aZda, aZn, aZm, 270, 0);
+}
Index: clang/test/CodeGen/aarch64-sve-inline-asm-negative-test.c
===
--- /dev/null
+++ clang/test/CodeGen/aarch64-sve-inline-asm-negative-test.c
@@ -0,0 +1,19 @@
+// RUN: not %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -fallow-half-arguments-and-returns \
+// RUN:   -target-feature +neon -S -O1 -o - %s | FileCheck %s
+
+// Assembler error
+// Output constraint : Set a vector constraint on an integer
+__SVFloat32_t funcB2()
+{
+  __SVFloat32_t ret ;
+  asm volatile (
+"fmov %[ret], wzr \n"
+: [ret] "=w" (ret)
+:
+:);
+
+  return ret ;
+}
+
+// CHECK: funcB2
+// CHECK-ERROR: error: invalid operand for instruction
Index: clang/test/CodeGen/aarch64-sve-inline-asm-datatypes.c
===
--- /dev/null
+++ clang/test/CodeGen/aarch64-sve-inline-asm-datatypes.c
@@ -0,0 +1,209 @@
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -fallow-half-arguments-and-returns \
+// RUN:   -target-feature +neon -S -O1 -o - %s | FileCheck %s
+
+// Tests to check that all sve datatypes can be passed in as input operands
+// and passed out as output operands.
+
+#define SVINT_TEST(DT, KIND)\
+DT func_int_##DT##KIND(DT in)\
+{\
+  DT out;\
+  asm volatile (\
+"ptrue p0.b\n"\
+"mov %[out]." #KIND ", p0/m, %[in]." #KIND "\n"\
+: [out] "=w" (out)\
+: [in] "w" (in)\
+: "p0"\
+);\
+  return out;\
+}
+
+SVINT_TEST(__SVUint8_t,b);
+// CHECK: mov {{z[0-9]+}}.b, p0/m, {{z[0-9]+}}.b
+SVINT_TEST(__SVUint8_t,h);
+// CHECK: mov {{z[0-9]+}}.h, p0/m, {{z[0-9]+}}.h
+SVINT_TEST(__SVUint8_t,s);
+// CHECK: mov {{z[0-9]+}}.s, p0/m, {{z[0-9]+}}.s
+SVINT_TEST(__SVUint8_t,d);
+// CHECK: mov {{z[0-9]+}}.d, p0/m, {{z[0-9]+}}.d
+
+SVINT_TEST(__SVUint16_t,b);
+// CHECK: mov {{z[0-9]+}}.b, p0/m, {{z[0-9]+}}.b
+SVINT_TEST(__SVUint16_t,h);
+// CHECK: mov {{z[0-9]+}}.h, p0/m, {{z[0-9]+}}.h
+SVINT_TEST(__SVUint16_t,s);
+// CHECK: mov {{z[0-9]+}}.s, p0/m, {{z[0-9]+}}.s
+SV

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

[Lucas Prates via Phabricator via cfe-commits]

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

Addressing review comment


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,6 +27,7 @@
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/None.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringExtras.h"
@@ -518,7 +519,7 @@
 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 +547,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 +1385,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 +1413,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 +1430,12 @@
 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 +1840,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 +1870,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.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

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

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



Comment at: clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp:719-720
+  const auto *PSBR = dyn_cast(&BR);
+  if (!PSBR)
+return "";
+

NoQ wrote:
> Mmm, this is definitely impossible. We should change the `NoteTag` API to 
> pass in a `PathSensitiveBugReport &`. My bad.
Good idea. For now I changed it to simple `cast<>()` without check.


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

https://reviews.llvm.org/D75514



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

[PATCH] D75677: [Analyzer] Only add iterator note tags to the operations of the affected iterators

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

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

Updated according to the comments to D75514 .


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

https://reviews.llvm.org/D75677

Files:
  clang/lib/StaticAnalyzer/Checkers/DebugIteratorModeling.cpp
  clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp
  clang/test/Analysis/iterator-modelling.cpp

Index: clang/test/Analysis/iterator-modelling.cpp
===
--- clang/test/Analysis/iterator-modelling.cpp
+++ clang/test/Analysis/iterator-modelling.cpp
@@ -81,7 +81,7 @@
 
   clang_analyzer_denote(clang_analyzer_container_begin(v), "$v.begin()");
 
-  auto j = i++; // expected-note 2{{Iterator 'i' incremented by 1}}
+  auto j = i++; // expected-note{{Iterator 'i' incremented by 1}}
 
   clang_analyzer_express(clang_analyzer_iterator_position(i)); //expected-warning{{$v.begin() + 1}}
//expected-note@-1{{$v.begin() + 1}}
@@ -94,7 +94,7 @@
 
   clang_analyzer_denote(clang_analyzer_container_end(v), "$v.end()");
 
-  auto j = i--; // expected-note 2{{Iterator 'i' decremented by 1}}
+  auto j = i--; // expected-note{{Iterator 'i' decremented by 1}}
 
   clang_analyzer_express(clang_analyzer_iterator_position(i)); //expected-warning{{$v.end() - 1}}
//expected-note@-1{{$v.end() - 1}}
@@ -164,7 +164,7 @@
 
   clang_analyzer_denote(clang_analyzer_container_begin(v), "$v.begin()");
 
-  auto i2 = i1 + 2; // expected-note 2{{Iterator 'i1' incremented by 2}}
+  auto i2 = i1 + 2; // expected-note{{Iterator 'i1' incremented by 2}}
 
   clang_analyzer_eval(clang_analyzer_iterator_container(i2) == &v); // expected-warning{{TRUE}}
 // expected-note@-1{{TRUE}}
@@ -177,7 +177,7 @@
 
   clang_analyzer_denote(clang_analyzer_container_end(v), "$v.end()");
 
-  auto i2 = i1 + (-2); // expected-note 2{{Iterator 'i1' decremented by 2}}
+  auto i2 = i1 + (-2); // expected-note{{Iterator 'i1' decremented by 2}}
 
   clang_analyzer_eval(clang_analyzer_iterator_container(i2) == &v); // expected-warning{{TRUE}}
 // expected-note@-1{{TRUE}}
@@ -190,7 +190,7 @@
 
   clang_analyzer_denote(clang_analyzer_container_end(v), "$v.end()");
 
-  auto i2 = i1 - 2;  // expected-note 2{{Iterator 'i1' decremented by 2}}
+  auto i2 = i1 - 2; // expected-note{{Iterator 'i1' decremented by 2}}
 
   clang_analyzer_eval(clang_analyzer_iterator_container(i2) == &v); // expected-warning{{TRUE}}
 // expected-note@-1{{TRUE}}
@@ -203,7 +203,7 @@
 
   clang_analyzer_denote(clang_analyzer_container_begin(v), "$v.begin()");
 
-  auto i2 = i1 - (-2); // expected-note 2{{Iterator 'i1' incremented by 2}}
+  auto i2 = i1 - (-2); // expected-note{{Iterator 'i1' incremented by 2}}
 
   clang_analyzer_eval(clang_analyzer_iterator_container(i2) == &v); // expected-warning{{TRUE}}
 // expected-note@-1{{TRUE}}
@@ -217,7 +217,7 @@
   clang_analyzer_denote(clang_analyzer_container_begin(v), "$v.begin()");
 
   auto i2 = i1;
-  ++i1;  // expected-note 2{{Iterator 'i1' incremented by 1}}
+  ++i1;  // expected-note{{Iterator 'i1' incremented by 1}}
 
   clang_analyzer_express(clang_analyzer_iterator_position(i1)); //expected-warning{{$v.begin() + 1}}
 //expected-note@-1{{$v.begin() + 1}}
@@ -231,7 +231,7 @@
   clang_analyzer_denote(clang_analyzer_container_begin(v), "$v.begin()");
 
   auto i2 = i1;
-  ++i2;  // expected-note 2{{Iterator 'i2' incremented by 1}}
+  ++i2;  // expected-note{{Iterator 'i2' incremented by 1}}
 
   clang_analyzer_express(clang_analyzer_iterator_position(i1)); //expected-warning{{$v.begin()}}
 //expected-note@-1{{$v.begin()}}
@@ -245,7 +245,7 @@
   clang_analyzer_denote(clang_analyzer_container_end(v), "$v.end()");
 
   auto i2 = i1;
-  --i1;  // expected-note 2{{Iterator 'i1' decremented by 1}}
+  --i1;  // expected-note{{Iterator 'i1' decremented by 1}}
 
   clang_analyzer_express(clang_analyzer_iterator_position(i1)); //expected-warning{{$v.end() - 1}}
 //expected-note@-1{{$v.end() - 1}}
@@ -259,7 +259,7 @@
   clang_analyzer_denote(clang_analyzer_container_end(v), "$v.end()");
 
   auto i2 = i1;
-  --i2;  // expected-note 2{{Iterator 'i2' decremented by 1}}
+  --i2;  // expected-note{{Iterator 'i2' decremented by 1}}
 
   clang_analyzer_express(clang_analyzer_iterator_position(i1)); //expected-warning{{$v.end(

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

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

The warning below suggests that we parse the ctu-other.c file (and presumably 
every file) as a C++ file, even if it should be parsed as a C file. Perhaps the 
invocation of the parser is missing some setting? Also, could this be the 
reason why on-the-fly and pch driven ctu gives different statistics?

  warning: treating 'c' input as 'c++' when in C++ mode, this behavior is 
deprecated [-Wdeprecated]
  
/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/clang/test/Analysis/Inputs/ctu-other.c:47:26:
 error: 'DataType' cannot be defined in a parameter type
  int structInProto(struct DataType {int a;int b; } * d) {
   ^
  1 error generated.
  Error while processing 
/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/clang/test/Analysis/Inputs/ctu-other.c.


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] D75665: [analyzer] On-demand parsing capability for CTU

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

> As the CTUDir string prefix is only needed in case of the old approach, I 
> have refactored the external definition mapping storage to NOT include that.

Both `ctu-main.c` and `ctu-on-demand-parsing.c` use the `-analyzer-config 
ctu-dir=%t/ctudir2` setting. So it is not clear for me why we don't need the 
prefix. I believe we still need that directory setting otherwise where could we 
find the externalDefMap 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] D75215: [DebugInfo] Fix for adding "returns cxx udt" option to functions in CodeView.

[Amy Huang via Phabricator via cfe-commits]

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



Comment at: llvm/include/llvm/IR/DebugInfoFlags.def:61
 HANDLE_DI_FLAG((1 << 29), AllCallsDescribed)
+HANDLE_DI_FLAG((1 << 30), CxxReturnUdt)
 

rnk wrote:
> aprantl wrote:
> > dblaikie wrote:
> > > rnk wrote:
> > > > @dblaikie @aprantl, does this seem like a reasonable flag to add, or 
> > > > should we mark record forward decls as trivial/nontrivial instead?
> > > Currently we only have a trivial/nontrivial flag that goes one way, 
> > > right? (ie: true/false, not three state true/false/unknown)
> > > 
> > > That would present a problem for forward declarations - because for a 
> > > true forward decl you can't know if it's trivial/non-trivial for passing, 
> > > right? so that'd present a subtle difference between trivial/non-trivial 
> > > on a decl (where it might be trivial/unknown) and on a def (where it's 
> > > trivial/non-trivial), yes?
> > Should this perhaps be a DI_SPFLAG instead?
> It's true that there is an ambiguity between known trivial, known 
> non-trivial, and forward declared, unknown triviality. Amy's solution to this 
> problem was to mark forward declarations as nontrivial, which matches the 
> logic MSVC uses to set CxxReturnUdt, but might not be the right thing for 
> other consumers.
Alternatively, I guess we can FlagNonTrivial to FlagTrivial, and then we can 
mark all the unknown things with cxxreturnudt.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75215



___
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#1907100 , @jeroen.dobbelaere 
wrote:

> Just to give an example:
>
>   int foo(int* restrict *pA, int* restrict *pB) {
> int tmp=**pB;
> **pA=42;
> return tmp - **pB; // **pA and **pB can refer to the same objects
>   }
>
>
> [...]


`*pA` and `*pB` are both restrict qualified pointers, are they not?
If so, why can they point to the same location?

I mean, w/o the side channel stuff the two `int *` values may alias
even if you have `noalias` on the arguments (https://godbolt.org/z/J9v_gK).


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] D73979: [HIP] Allow non-incomplete array type for extern shared var

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added a comment.

In D73979#1857736 , @yaxunl wrote:

> BTW this is requested by HIP users, who have similar code for CUDA and HIP. 
> They found it surprised that nvcc allows it but hip-clang does not.


I think I'm one of the HIP users here, but the above change is not what I was 
hoping for.

I'd like:

  __shared__ int x;
  __shared__ int y;
  __device__ void foo()
  {
assert(&x != &y);
x = 2 * y;
  }

to compile and behave as it does on cuda, i.e. the 'x' variable gets allocated 
in __shared__ memory for each kernel which accesses it, and so does 'y'.

The 'extern __shared__' feature where nvcc builds a union out of all the things 
it sees and the user indexes into it at runtime is totally unappealing. That 
cuda uses the 'extern' keyword to opt into this magic union also seems 
undesirable.


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

https://reviews.llvm.org/D73979



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

[clang] 314b927 - Revert "[CGBlocks] Improve line info in backtraces containing *_helper_block"

[Adrian Prantl via cfe-commits]

Author: Adrian Prantl
Date: 2020-03-05T09:58:42-08:00
New Revision: 314b9278f0975b12e15d6e12f896eaf7c4519ef2

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

LOG: Revert "[CGBlocks] Improve line info in backtraces containing 
*_helper_block"

Block copy/destroy helpers are now linkonce_odr functions, meant to be uniqued, 
and thus attaching debug information from one translation unit (or even just 
from one instance of many inside one translation unit) would be misleading and 
wrong in the general case.

This effectively reverts commit 9c6b6826ce3720ca8bb9bd15f3abf71261e6b911.



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

Added: 


Modified: 
clang/lib/CodeGen/CGBlocks.cpp
clang/test/CodeGenObjC/debug-info-blocks.m

Removed: 




diff  --git a/clang/lib/CodeGen/CGBlocks.cpp b/clang/lib/CodeGen/CGBlocks.cpp
index 33fad77eb4da..9ec37385c4a7 100644
--- a/clang/lib/CodeGen/CGBlocks.cpp
+++ b/clang/lib/CodeGen/CGBlocks.cpp
@@ -2032,11 +2032,13 @@ CodeGenFunction::GenerateCopyHelperFunction(const 
CGBlockInfo &blockInfo) {
   FunctionDecl *FD = FunctionDecl::Create(
   C, C.getTranslationUnitDecl(), SourceLocation(), SourceLocation(), II,
   FunctionTy, nullptr, SC_Static, false, false);
-
   setBlockHelperAttributesVisibility(blockInfo.CapturesNonExternalType, Fn, FI,
  CGM);
+  // This is necessary to avoid inheriting the previous line number.
+  FD->setImplicit();
   StartFunction(FD, ReturnTy, Fn, FI, args);
-  ApplyDebugLocation NL{*this, blockInfo.getBlockExpr()->getBeginLoc()};
+  auto AL = ApplyDebugLocation::CreateArtificial(*this);
+
   llvm::Type *structPtrTy = blockInfo.StructureType->getPointerTo();
 
   Address src = GetAddrOfLocalVar(&SrcDecl);
@@ -2227,10 +2229,12 @@ CodeGenFunction::GenerateDestroyHelperFunction(const 
CGBlockInfo &blockInfo) {
 
   setBlockHelperAttributesVisibility(blockInfo.CapturesNonExternalType, Fn, FI,
  CGM);
+  // This is necessary to avoid inheriting the previous line number.
+  FD->setImplicit();
   StartFunction(FD, ReturnTy, Fn, FI, args);
   markAsIgnoreThreadCheckingAtRuntime(Fn);
 
-  ApplyDebugLocation NL{*this, blockInfo.getBlockExpr()->getBeginLoc()};
+  auto AL = ApplyDebugLocation::CreateArtificial(*this);
 
   llvm::Type *structPtrTy = blockInfo.StructureType->getPointerTo();
 

diff  --git a/clang/test/CodeGenObjC/debug-info-blocks.m 
b/clang/test/CodeGenObjC/debug-info-blocks.m
index 9204f8d56c07..257045b05c32 100644
--- a/clang/test/CodeGenObjC/debug-info-blocks.m
+++ b/clang/test/CodeGenObjC/debug-info-blocks.m
@@ -17,13 +17,17 @@
 // CHECK-NOT: ret
 // CHECK: call {{.*}}, !dbg ![[DBG_LINE:[0-9]+]]
 // CHECK-NOT: ret
-// CHECK: load {{.*}}, !dbg ![[COPY_LINE:[0-9]+]]
-// CHECK: ret void, !dbg ![[COPY_LINE]]
-// CHECK: define {{.*}} @__destroy_helper_block_{{.*}}(i8* %0)
+// CHECK: load {{.*}}, !dbg ![[DBG_LINE]]
+// CHECK: ret {{.*}}, !dbg ![[DBG_LINE]]
+// CHECK: define {{.*}} @__destroy_helper_block_{{.*}}(i8*
 // CHECK-NOT: ret
 // CHECK: load {{.*}}, !dbg ![[DESTROY_LINE:[0-9]+]]
-// CHECK: ret void, !dbg ![[DESTROY_LINE]]
+// CHECK: ret {{.*}}, !dbg ![[DESTROY_LINE]]
 
+// CHECK-DAG: [[DBG_LINE]] = !DILocation(line: 0, scope: ![[COPY_SP:[0-9]+]])
+// CHECK-DAG: [[COPY_SP]] = distinct !DISubprogram(name: "__copy_helper_block_
+// CHECK-DAG: [[DESTROY_LINE]] = !DILocation(line: 0, scope: 
![[DESTROY_SP:[0-9]+]])
+// CHECK-DAG: [[DESTROY_SP]] = distinct !DISubprogram(name: 
"__destroy_helper_block_
 typedef unsigned int NSUInteger;
 
 @protocol NSObject
@@ -57,11 +61,6 @@ @implementation A
 - (id)init
 {
 if ((self = [super init])) {
-  // CHECK-DAG: [[DBG_LINE]] = !DILocation(line: 0, scope: 
![[COPY_SP:[0-9]+]])
-  // CHECK-DAG: [[COPY_LINE]] = !DILocation(line: [[@LINE+7]], scope: 
![[COPY_SP:[0-9]+]])
-  // CHECK-DAG: [[COPY_SP]] = distinct !DISubprogram(name: 
"__copy_helper_block_8_32o"
-  // CHECK-DAG: [[DESTROY_LINE]] = !DILocation(line: [[@LINE+5]], scope: 
![[DESTROY_SP:[0-9]+]])
-  // CHECK-DAG: [[DESTROY_SP]] = distinct !DISubprogram(name: 
"__destroy_helper_block_8_32o"
   // CHECK-DAG: !DILocalVariable(arg: 1, scope: ![[COPY_SP]], {{.*}}, 
flags: DIFlagArtificial)
   // CHECK-DAG: !DILocalVariable(arg: 2, scope: ![[COPY_SP]], {{.*}}, 
flags: DIFlagArtificial)
   // CHECK-DAG: !DILocalVariable(arg: 1, scope: ![[DESTROY_SP]], {{.*}}, 
flags: DIFlagArtificial)



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

[PATCH] D75271: [analyzer][NFC] Change LangOptions to CheckerManager in the shouldRegister* functions.

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

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

This patch still does not help. Most of the functions of `CheckerManager` are 
non-const thus they cannot be called on a const object. E.g. 
`getAnalyzerOptions()` is non-const (why?). For error reporting 
`getASTContext()` should be const as well (to access the diagnostics engine). 
Thus either `CheckerManager` should be changed by setting its getters to const 
or the `shouldRegister`//XXX//`()` should take a non-const reference to 
`CheckerManager`. I would prefer the former one.


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

https://reviews.llvm.org/D75271



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

[PATCH] D75494: [PowerPC] Delete PPCMachObjectWriter and triple for darwin

[Sean Fertile via Phabricator via cfe-commits]

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

LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75494



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

[PATCH] D75697: [analyzer] Allow null false positive suppression for conditions

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

Szelethus created this revision.
Szelethus added reviewers: NoQ, baloghadamsoftware, balazske, martong, 
xazax.hun, dcoughlin.
Szelethus added a project: clang.
Herald added subscribers: cfe-commits, steakhal, Charusso, gamesh411, dkrupp, 
donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, whisperity.

If the a value has received its value through suspicious means, we suppress it. 
Tracked conditions are very much relevant to the occurrence of a bug, if their 
value is fishy, the entire bug report probably is as well.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D75697

Files:
  clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
  clang/test/Analysis/inlining/inline-defensive-checks.m


Index: clang/test/Analysis/inlining/inline-defensive-checks.m
===
--- clang/test/Analysis/inlining/inline-defensive-checks.m
+++ clang/test/Analysis/inlining/inline-defensive-checks.m
@@ -108,11 +108,9 @@
   unsigned zero = 0;
   fPtr = retNil();
   // On a path where fPtr is nil, mem should be nil.
-  // The warning is not suppressed because the receiver being nil is not
-  // directly related to the value that triggers the warning.
   Foo *mem = [fPtr getFooPtr];
   if (!mem)
-return 5/zero; // expected-warning {{Division by zero}}
+return 5/zero;
   return 0;
 }
 
Index: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
===
--- clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -1813,7 +1813,7 @@
   if (BR.addTrackedCondition(N)) {
 bugreporter::trackExpressionValue(
 N, Condition, BR, bugreporter::TrackingKind::Condition,
-/*EnableNullFPSuppression=*/false);
+/*EnableNullFPSuppression=*/true);
 return constructDebugPieceForTrackedCondition(Condition, N, BRC);
   }
 }


Index: clang/test/Analysis/inlining/inline-defensive-checks.m
===
--- clang/test/Analysis/inlining/inline-defensive-checks.m
+++ clang/test/Analysis/inlining/inline-defensive-checks.m
@@ -108,11 +108,9 @@
   unsigned zero = 0;
   fPtr = retNil();
   // On a path where fPtr is nil, mem should be nil.
-  // The warning is not suppressed because the receiver being nil is not
-  // directly related to the value that triggers the warning.
   Foo *mem = [fPtr getFooPtr];
   if (!mem)
-return 5/zero; // expected-warning {{Division by zero}}
+return 5/zero;
   return 0;
 }
 
Index: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
===
--- clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -1813,7 +1813,7 @@
   if (BR.addTrackedCondition(N)) {
 bugreporter::trackExpressionValue(
 N, Condition, BR, bugreporter::TrackingKind::Condition,
-/*EnableNullFPSuppression=*/false);
+/*EnableNullFPSuppression=*/true);
 return constructDebugPieceForTrackedCondition(Condition, N, BRC);
   }
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

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

Szelethus created this revision.
Szelethus added reviewers: NoQ, baloghadamsoftware, xazax.hun, rnkovacs, 
dcoughlin, martong, balazske.
Szelethus added a project: clang.
Herald added subscribers: cfe-commits, danielkiss, steakhal, Charusso, 
gamesh411, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, szepet, 
kristof.beyls, whisperity, mgorny.

You may be familiar with reports that are demonstrated in the test file -- 
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). Invalidation are often incorrect, so this patch attempts 
to suppress reports where a tracked expression's last value change was a result 
of an invalidation.

This is solved by a adding a new checker that notes which regions at which 
`ProgramPoint` were invalidated, and adding a new `BugReporterVisitor` to the 
army of `trackExpressionValue` related visitors that looks for the last value 
change, and suppresses the report if it was a previously noted invalidation.

The big scare in such changes is always whether we would suppress so many 
things, so this patch says little without results. I'll share some as soon as I 
have them :)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D75698

Files:
  clang/include/clang/Analysis/ProgramPoint.h
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
  clang/lib/Analysis/ProgramPoint.cpp
  clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
  clang/lib/StaticAnalyzer/Checkers/SuppressInvalidationRelatedReports.cpp
  clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
  clang/lib/StaticAnalyzer/Core/ExplodedGraph.cpp
  clang/test/Analysis/analyzer-enabled-checkers.c
  clang/test/Analysis/suppress-invalidation-related-reports.cpp

Index: clang/test/Analysis/suppress-invalidation-related-reports.cpp
===
--- /dev/null
+++ clang/test/Analysis/suppress-invalidation-related-reports.cpp
@@ -0,0 +1,102 @@
+// RUN: %clang_analyze_cc1 -verify=expected %s \
+// RUN:   -analyzer-checker=core,apiModeling.SuppressInvalidationRelatedReports
+
+// RUN: %clang_analyze_cc1 -verify=non-suppressed,expected %s \
+// RUN:   -analyzer-checker=core
+
+namespace invalidated_global {
+int flag;
+void foo();
+
+void f() {
+  flag = 0;   // Initialize flag to false.
+  int *x = 0; // x is initialized to a nullptr.
+
+  foo(); // Invalidate flag's value.
+
+  if (flag) // Assume flag is true.
+*x = 5; // non-suppressed-warning{{Dereference of null pointer}}
+}
+} // namespace invalidated_global
+
+namespace invalidated_field {
+struct S {
+  int flag;
+  void invalidate();
+};
+
+void f() {
+  S s;
+  s.flag = 0; // Initialize s.flag to false.
+  int *x = 0; // x is initialized to a nullptr.
+
+  s.invalidate(); // Invalidate s.flag's value.
+
+  if (s.flag) // Assume s.flag is true.
+*x = 5;   // non-suppressed-warning{{Dereference of null pointer}}
+}
+} // namespace invalidated_field
+
+namespace invalidated_field_before_bind {
+struct S {
+  int flag;
+  void invalidate();
+  void setFlagToTrue() {
+invalidate();
+flag = 1;
+  }
+};
+
+void f() {
+  S s;
+  s.flag = 0; // Initialize s.flag to false.
+  int *x = 0; // x is initialized to a nullptr.
+
+  s.setFlagToTrue(); // Invalidate s.flag's, but also set it to true.
+
+  if (s.flag) // Assume s.flag is true.
+*x = 5;   // expected-warning{{Dereference of null pointer}}
+}
+} // namespace invalidated_field_before_bind
+
+namespace invalidated_field_twice {
+struct S {
+  int flag;
+  void invalidate();
+  void setFlagToTrue() {
+invalidate();
+flag = 1;
+invalidate();
+  }
+};
+
+void f() {
+  S s;
+  s.flag = 0; // Initialize s.flag to false.
+  int *x = 0; // x is initialized to a nullptr.
+
+  s.setFlagToTrue(); // Invalidate s.flag's, but also set it to true.
+
+  if (s.flag) // Assume s.flag is true.
+*x = 5;   // non-suppressed-warning{{Dereference of null pointer}}
+}
+} // namespace invalidated_field_twice
+
+namespace invalidated_field_through_bind {
+struct S {
+  int flag;
+};
+
+void f() {
+  S s;
+  s.flag = 0; // Initialize s.flag to false.
+  S s2;
+  s2.flag = 1; // Initialize s2.flag to true
+  int *x = 0;  // x is initialized to a nullptr.
+
+  s = s2; // "Invalidate" s.flag's value through bind.
+
+  if (s.flag) // Assume s.flag is true.
+*x = 5;   // expected-warning{{Dereference of null pointer}}
+}
+} // namespace invalidated_field_through_bind
Index: clang/test/Analysis/analyzer-enabled-checkers.c
===
--- clang/test/Analysis/analyzer-enabled-checkers.c
+++ clang/test/Analysis/analyzer-enabled-checkers.c
@@ -7,6 +7,7 @@
 // CHECK:  OVERVIEW: Clang Static Analyzer Enabled Checkers List
 // CHECK-EMPTY:
 // CHECK-NEXT: apiModeling.StdCLibraryFunctions
+// CHECK-NEXT: apiMode

[PATCH] D75690: [SVE][Inline-Asm] Add constraints for SVE ACLE types

[Eli Friedman via Phabricator via cfe-commits]

efriedma added inline comments.



Comment at: clang/lib/Basic/Targets/AArch64.h:95
+case 'U':   // Three-character constraint; add "@3" hint for later parsing.
+  R = std::string("@3") + std::string(Constraint, 3);
+  Constraint += 2;

Is "@3" some LLVM thing?  I don't think I've seen it before.



Comment at: clang/test/CodeGen/aarch64-sve-inline-asm-datatypes.c:2
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve 
-fallow-half-arguments-and-returns \
+// RUN:   -target-feature +neon -S -O1 -o - %s | FileCheck %s
+

`REQUIRES: aarch64-registered-target` is necessary for any test that isn't 
using -emit-llvm.

Generally, I'd prefer tests using -emit-llvm so we can independently verify 
that the IR is what we expect, vs. the backend processing the IR the way we 
expect.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75690



___
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/include/clang/Basic/DiagnosticParseKinds.td:32
   "'asm goto' cannot have output constraints">;
+def err_asm_duplicate_qual : Error<"duplicate asm qualifier %0">;
 }

`duplicate asm qualifier '%0'` (with the quotes around the `%0`).



Comment at: clang/include/clang/Sema/DeclSpec.h:324
 
+  // GNU Asm Qualifiers
+  enum AQ {

Asm Qualifiers -> asm qualifiers



Comment at: clang/include/clang/Sema/DeclSpec.h:378
+  // GNU asm specifier
+  unsigned GNUAsmQualifiers : 4;
+

I think we only need three bits to store this. Also, the comment should be 
clear that this is a bitwise OR of AQ, similar to the comment for 
`TypeQualifiers`.



Comment at: clang/include/clang/Sema/DeclSpec.h:582
+  /// getGNUAsmQualifiers - Return a set of AQs.
+  unsigned GetGNUAsmQual() const { return GNUAsmQualifiers; }
+

The comment and the code don't match. I prefer the spelling in the comment.



Comment at: clang/include/clang/Sema/DeclSpec.h:736
 
+  bool SetGNUAsmQual(AQ A, SourceLocation Loc);
+

Given that we're already inconsistent in this area regarding naming, I'd prefer 
to follow the LLVM naming convention and use `setGNUAsmQualifier()`



Comment at: clang/lib/Parse/ParseStmtAsm.cpp:688
+/// ParseGNUAsmQualifierListOpt - Parse a GNU extended asm qualifier list.
+///   asm-qualifiers:
+/// volatile

The comment doesn't match the code -- I think the comment should be:
```
/// asm-qualifier:
///   volatile
///   inline
///   goto
///
/// asm-qualifier-list:
///   asm-qualifier
///   asm-qualifier-list asm-qualifier
```



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

I would expect a diagnostic if the unknown token is anything other than a left 
paren.



Comment at: clang/test/Parser/asm-qualifiers.c:3
+
+void qualifiers(void) {
+  asm("");

There should also be tests where the qualifiers are incorrect. e.g.,
```
asm noodle("");
asm goto noodle("");
asm volatile noodle inline("");
```


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] D75615: Revert "[CGBlocks] Improve line info in backtraces containing *_helper_block"

[Adrian Prantl via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG314b9278f097: Revert "[CGBlocks] Improve line info in 
backtraces containing *_helper_block" (authored by aprantl).
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75615

Files:
  clang/lib/CodeGen/CGBlocks.cpp
  clang/test/CodeGenObjC/debug-info-blocks.m


Index: clang/test/CodeGenObjC/debug-info-blocks.m
===
--- clang/test/CodeGenObjC/debug-info-blocks.m
+++ clang/test/CodeGenObjC/debug-info-blocks.m
@@ -17,13 +17,17 @@
 // CHECK-NOT: ret
 // CHECK: call {{.*}}, !dbg ![[DBG_LINE:[0-9]+]]
 // CHECK-NOT: ret
-// CHECK: load {{.*}}, !dbg ![[COPY_LINE:[0-9]+]]
-// CHECK: ret void, !dbg ![[COPY_LINE]]
-// CHECK: define {{.*}} @__destroy_helper_block_{{.*}}(i8* %0)
+// CHECK: load {{.*}}, !dbg ![[DBG_LINE]]
+// CHECK: ret {{.*}}, !dbg ![[DBG_LINE]]
+// CHECK: define {{.*}} @__destroy_helper_block_{{.*}}(i8*
 // CHECK-NOT: ret
 // CHECK: load {{.*}}, !dbg ![[DESTROY_LINE:[0-9]+]]
-// CHECK: ret void, !dbg ![[DESTROY_LINE]]
+// CHECK: ret {{.*}}, !dbg ![[DESTROY_LINE]]
 
+// CHECK-DAG: [[DBG_LINE]] = !DILocation(line: 0, scope: ![[COPY_SP:[0-9]+]])
+// CHECK-DAG: [[COPY_SP]] = distinct !DISubprogram(name: "__copy_helper_block_
+// CHECK-DAG: [[DESTROY_LINE]] = !DILocation(line: 0, scope: 
![[DESTROY_SP:[0-9]+]])
+// CHECK-DAG: [[DESTROY_SP]] = distinct !DISubprogram(name: 
"__destroy_helper_block_
 typedef unsigned int NSUInteger;
 
 @protocol NSObject
@@ -57,11 +61,6 @@
 - (id)init
 {
 if ((self = [super init])) {
-  // CHECK-DAG: [[DBG_LINE]] = !DILocation(line: 0, scope: 
![[COPY_SP:[0-9]+]])
-  // CHECK-DAG: [[COPY_LINE]] = !DILocation(line: [[@LINE+7]], scope: 
![[COPY_SP:[0-9]+]])
-  // CHECK-DAG: [[COPY_SP]] = distinct !DISubprogram(name: 
"__copy_helper_block_8_32o"
-  // CHECK-DAG: [[DESTROY_LINE]] = !DILocation(line: [[@LINE+5]], scope: 
![[DESTROY_SP:[0-9]+]])
-  // CHECK-DAG: [[DESTROY_SP]] = distinct !DISubprogram(name: 
"__destroy_helper_block_8_32o"
   // CHECK-DAG: !DILocalVariable(arg: 1, scope: ![[COPY_SP]], {{.*}}, 
flags: DIFlagArtificial)
   // CHECK-DAG: !DILocalVariable(arg: 2, scope: ![[COPY_SP]], {{.*}}, 
flags: DIFlagArtificial)
   // CHECK-DAG: !DILocalVariable(arg: 1, scope: ![[DESTROY_SP]], {{.*}}, 
flags: DIFlagArtificial)
Index: clang/lib/CodeGen/CGBlocks.cpp
===
--- clang/lib/CodeGen/CGBlocks.cpp
+++ clang/lib/CodeGen/CGBlocks.cpp
@@ -2032,11 +2032,13 @@
   FunctionDecl *FD = FunctionDecl::Create(
   C, C.getTranslationUnitDecl(), SourceLocation(), SourceLocation(), II,
   FunctionTy, nullptr, SC_Static, false, false);
-
   setBlockHelperAttributesVisibility(blockInfo.CapturesNonExternalType, Fn, FI,
  CGM);
+  // This is necessary to avoid inheriting the previous line number.
+  FD->setImplicit();
   StartFunction(FD, ReturnTy, Fn, FI, args);
-  ApplyDebugLocation NL{*this, blockInfo.getBlockExpr()->getBeginLoc()};
+  auto AL = ApplyDebugLocation::CreateArtificial(*this);
+
   llvm::Type *structPtrTy = blockInfo.StructureType->getPointerTo();
 
   Address src = GetAddrOfLocalVar(&SrcDecl);
@@ -2227,10 +2229,12 @@
 
   setBlockHelperAttributesVisibility(blockInfo.CapturesNonExternalType, Fn, FI,
  CGM);
+  // This is necessary to avoid inheriting the previous line number.
+  FD->setImplicit();
   StartFunction(FD, ReturnTy, Fn, FI, args);
   markAsIgnoreThreadCheckingAtRuntime(Fn);
 
-  ApplyDebugLocation NL{*this, blockInfo.getBlockExpr()->getBeginLoc()};
+  auto AL = ApplyDebugLocation::CreateArtificial(*this);
 
   llvm::Type *structPtrTy = blockInfo.StructureType->getPointerTo();
 


Index: clang/test/CodeGenObjC/debug-info-blocks.m
===
--- clang/test/CodeGenObjC/debug-info-blocks.m
+++ clang/test/CodeGenObjC/debug-info-blocks.m
@@ -17,13 +17,17 @@
 // CHECK-NOT: ret
 // CHECK: call {{.*}}, !dbg ![[DBG_LINE:[0-9]+]]
 // CHECK-NOT: ret
-// CHECK: load {{.*}}, !dbg ![[COPY_LINE:[0-9]+]]
-// CHECK: ret void, !dbg ![[COPY_LINE]]
-// CHECK: define {{.*}} @__destroy_helper_block_{{.*}}(i8* %0)
+// CHECK: load {{.*}}, !dbg ![[DBG_LINE]]
+// CHECK: ret {{.*}}, !dbg ![[DBG_LINE]]
+// CHECK: define {{.*}} @__destroy_helper_block_{{.*}}(i8*
 // CHECK-NOT: ret
 // CHECK: load {{.*}}, !dbg ![[DESTROY_LINE:[0-9]+]]
-// CHECK: ret void, !dbg ![[DESTROY_LINE]]
+// CHECK: ret {{.*}}, !dbg ![[DESTROY_LINE]]
 
+// CHECK-DAG: [[DBG_LINE]] = !DILocation(line: 0, scope: ![[COPY_SP:[0-9]+]])
+// CHECK-DAG: [[COPY_SP]] = distinct !DISubprogram(name: "__copy_helper_block_
+// CHECK-DAG: [[DESTROY_LINE]] = !DILocation(line: 0, scope: ![[DESTROY_SP:[0-9]+]])
+// CHECK-DAG: [[DESTROY_SP]] = distinc

[clang] f23df1b - Comment parsing: Treat \ref as inline command

[Aaron Puchert via cfe-commits]

Author: Aaron Puchert
Date: 2020-03-05T19:44:34+01:00
New Revision: f23df1b2a323094e5a6869ef085f100fd065bc0d

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

LOG: Comment parsing: Treat \ref as inline command

Summary:
It's basically Doxygen's version of a link and can happen anywhere
inside of a paragraph. Fixes a bogus warning about empty paragraphs when
a parameter description starts with a link.

Reviewers: gribozavr2

Reviewed By: gribozavr2

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

Added: 


Modified: 
clang/include/clang/AST/CommentCommands.td
clang/test/Sema/warn-documentation.cpp

Removed: 




diff  --git a/clang/include/clang/AST/CommentCommands.td 
b/clang/include/clang/AST/CommentCommands.td
index d387df7ce570..fbbfc9f7e0b7 100644
--- a/clang/include/clang/AST/CommentCommands.td
+++ b/clang/include/clang/AST/CommentCommands.td
@@ -87,6 +87,7 @@ def P  : InlineCommand<"p">;
 def A  : InlineCommand<"a">;
 def E  : InlineCommand<"e">;
 def Em : InlineCommand<"em">;
+def Ref: InlineCommand<"ref">;
 def Anchor : InlineCommand<"anchor">;
 
 
//===--===//
@@ -205,7 +206,6 @@ def Paragraph : VerbatimLineCommand<"paragraph">;
 
 def Mainpage : VerbatimLineCommand<"mainpage">;
 def Subpage  : VerbatimLineCommand<"subpage">;
-def Ref  : VerbatimLineCommand<"ref">;
 
 def Relates : VerbatimLineCommand<"relates">;
 def Related : VerbatimLineCommand<"related">;

diff  --git a/clang/test/Sema/warn-documentation.cpp 
b/clang/test/Sema/warn-documentation.cpp
index 3091c2f34783..2b2a3d15304a 100644
--- a/clang/test/Sema/warn-documentation.cpp
+++ b/clang/test/Sema/warn-documentation.cpp
@@ -294,6 +294,9 @@ int test_param22(int x1, int x2, int x3);
 /// \retval 0 Blah blah.
 int test_param23(int a);
 
+/// \param a \ref test_param23 has an empty paragraph, this doesn't.
+int test_param24(int a);
+
 //===---
 // Test that we treat typedefs to some non-function types as functions for the
 // purposes of documentation comment parsing.



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

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

[Gabor Marton via Phabricator via cfe-commits]

martong added inline comments.



Comment at: clang/include/clang/CrossTU/CrossTranslationUnit.h:23
 #include "llvm/ADT/StringMap.h"
+#include "llvm/IR/DiagnosticInfo.h"
+#include "llvm/IR/OperandTraits.h"

Perhaps this include is needed only in the .cpp file?



Comment at: clang/include/clang/CrossTU/CrossTranslationUnit.h:97
 llvm::Expected>
-parseCrossTUIndex(StringRef IndexPath, StringRef CrossTUDir);
+parseCrossTUIndex(StringRef IndexPath);
 

Ah, so we use an absolute path for `IndexPath` from now on? If yes, could you 
please add that to the comments above?



Comment at: clang/lib/CrossTU/CrossTranslationUnit.cpp:121
+  return "Compile commands database contains multiple references to the "
+ "same sorce file.";
 }

typo: sorce -> source



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

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?



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

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


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