[PATCH] D124349: [analyzer] Get direct binding for specific punned case

[Gabor Marton via Phabricator via cfe-commits]

martong accepted this revision.
martong added a comment.

LGTM! Thanks Vince!




Comment at: clang/test/Analysis/array-punned-region.c:38
+  ff.b[0] = 3;
+  clang_analyzer_eval(*((int *)pff + 2) == 3); // expected-warning{{TRUE}}  // 
Or should this be `pff + 3` ???
+}




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124349

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

[PATCH] D124974: [clang] Include clang config.h in LangStandards.cpp

[Haojian Wu via Phabricator via cfe-commits]

hokein accepted this revision.
hokein 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/D124974/new/

https://reviews.llvm.org/D124974

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

[PATCH] D124447: [clang-tidy] Add infrastructure support for running on project-level information

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

Very good, this is a great initiative! Also, the quality of this patch is high, 
docs are great, even the release notes are updated. I would accept if I were 
that confident in Clang-tidy.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124447

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

[PATCH] D124774: [clang][ASTImporter][NFC]: Move clang::ImportError into own header.

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

balazske added a comment.

It looks mostly good now, apart from the line 1 comment problem. Usually in a 
review all phabricator diffs are created against the original commit or at 
least against an existing (in the current github repository) commit, here the 
last diff seems to be against an other commit. The next diff should be based on 
an existing commit (can be same as the original base). Otherwise the `arc 
patch` command may not work (I do not know if you use it) and it is difficult 
to compare changes related to the original code.




Comment at: clang/include/clang/AST/ASTImportError.h:2
+//===- ASTImportError.h -  Define errors while importing AST  ---*- C++
+//-*-===//
 //

One small problem: This line 2 is the end of line 1, this should be part of 
line 1. Line 1 should be exactly 80 characters long.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124774

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

[PATCH] D124995: [clang] Add static_cast to fix Bazel build.

[Adrian Kuegel via Phabricator via cfe-commits]

akuegel created this revision.
Herald added a project: All.
akuegel requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D124995

Files:
  clang/lib/ExtractAPI/ExtractAPIConsumer.cpp


Index: clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
===
--- clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
+++ clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
@@ -927,7 +927,8 @@
 HeaderContents += "\"\n";
   else
 HeaderContents += ">\n";
-  KnownInputFiles.emplace_back(*RelativeName, IsQuoted);
+  KnownInputFiles.emplace_back(static_cast>(*RelativeName),
+   IsQuoted);
 } else {
   HeaderContents += " \"";
   HeaderContents += FilePath;


Index: clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
===
--- clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
+++ clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
@@ -927,7 +927,8 @@
 HeaderContents += "\"\n";
   else
 HeaderContents += ">\n";
-  KnownInputFiles.emplace_back(*RelativeName, IsQuoted);
+  KnownInputFiles.emplace_back(static_cast>(*RelativeName),
+   IsQuoted);
 } else {
   HeaderContents += " \"";
   HeaderContents += FilePath;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D124726: Suggest typoed directives in preprocessor conditionals

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui added inline comments.



Comment at: clang/include/clang/Tooling/LevDistance.h:1
+//===--- LevDistance.h --*- C++ 
-*-===//
+//

ken-matsui wrote:
> aaron.ballman wrote:
> > You shouldn't need this file or the implementation file -- we have this 
> > functionality already in: 
> > https://github.com/llvm/llvm-project/blob/main/llvm/include/llvm/ADT/edit_distance.h
> >  and 
> > https://github.com/llvm/llvm-project/blob/main/llvm/include/llvm/ADT/StringRef.h#L240.
> I totally missed the implementations! Sorry.
> 
> But where should I put both `findSimilarStr` & `findSimilarStr`?
> It seems that their same implementations aren't there.
@aaron.ballman 

The computation results are not matched with my implementation and another 
language implementation.

So, several directives that could be suggested in my implementation are no 
longer possible to be suggested, such as `#id` -> `#if`, `#elid` -> `#elif`, 
and `#elsi` -> `#else`, when using `llvm::ComputeEditDistance`.

The implementation of `llvm::ComputeEditDistance` might be also correct, but 
some distances seem to be calculated longer, which causes fewer suggestions.

Should I keep going with this implementation or not?

---

[llvm::ComputeEditDistance](https://github.com/llvm/llvm-project/blob/main/llvm/include/llvm/ADT/edit_distance.h):

```
size_t Dist = Str1.edit_distance(Str2, false);
```

```
Str1: id, Str2: if, Dist: 2 # <-- here
Str1: id, Str2: ifdef, Dist: 3
Str1: id, Str2: ifndef, Dist: 4
Str1: ifd, Str2: if, Dist: 1
Str1: ifd, Str2: ifdef, Dist: 2
Str1: ifd, Str2: ifndef, Dist: 3
```

[My implementation](https://wandbox.org/permlink/zRjT41alOHdwcf00):

```
Str1: id, Str2: if, Dist: 1 # <-- here
Str1: id, Str2: ifdef, Dist: 3
Str1: id, Str2: ifndef, Dist: 4
Str1: ifd, Str2: if, Dist: 1
Str1: ifd, Str2: ifdef, Dist: 2
Str1: ifd, Str2: ifndef, Dist: 3
```

[Rustc implementation](https://wandbox.org/permlink/08SB08JFF5G4awx4):

```
Str1: id, Str2: if, Dist: 1 # <-- here
Str1: id, Str2: ifdef, Dist: 3
Str1: id, Str2: ifndef, Dist: 4
Str1: ifd, Str2: if, Dist: 1
Str1: ifd, Str2: ifdef, Dist: 2
Str1: ifd, Str2: ifndef, Dist: 3
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124726

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

[PATCH] D124845: StaticAnalyzer should inline overridden delete operator the same way as overridden new operator

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

Looks promising, I am close to accept this, thanks!




Comment at: clang/include/clang/Analysis/ConstructionContext.h:122-124
 assert(isa(E) || isa(E) ||
-   isa(E) || isa(E));
+   isa(E) || isa(E) ||
+   isa(E));

I think we could use the variadic `isa` template .



Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp:950
 
-  getCheckerManager().runCheckersForPostCall(Dst, DstPreCall, *Call, *this);
+  if (AMgr.getAnalyzerOptions().MayInlineCXXAllocator) {
+StmtNodeBuilder Bldr(DstPreCall, DstPostCall, *currBldrCtx);

Could you please also update the config option's documentation at 
`AnalyzerOptions.def`?



Comment at: clang/test/Analysis/cxxnewexpr-callback-inline.cpp:1
 // RUN: %clang_analyze_cc1 -analyzer-checker=debug.AnalysisOrder 
-analyzer-config 
c++-allocator-inlining=true,debug.AnalysisOrder:PreStmtCXXNewExpr=true,debug.AnalysisOrder:PostStmtCXXNewExpr=true,debug.AnalysisOrder:PreCall=true,debug.AnalysisOrder:PostCall=true,debug.AnalysisOrder:NewAllocator=true
 %s 2>&1 | FileCheck %s
 

It is hard to see how the two test files are different.
Ideally, the two test files `cxxnewexpr-callback-inline.cpp` and 
`cxxnewexpr-callback-noinline.cpp` could be merged into the very same test file 
with two RUN lines and with different `--check-prefix`. 

However, I see that this is kind of unrelated to this particular change, so, 
that could be done in a separate, follow-up patch, if you have the time and 
mood for that.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124845

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

[PATCH] D124982: [clang][OpenMP][DebugInfo] Debug support for variables in containing scope of OMP constructs

[Alok Kumar Sharma via Phabricator via cfe-commits]

alok updated this revision to Diff 427237.
alok added a comment.

re-based.


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

https://reviews.llvm.org/D124982

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGDebugInfo.h
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.h
  clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
  clang/lib/CodeGen/CGOpenMPRuntimeGPU.h
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/test/OpenMP/debug_containing_scope.c
  llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
  llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
  llvm/lib/CodeGen/AsmPrinter/DwarfUnit.h

Index: llvm/lib/CodeGen/AsmPrinter/DwarfUnit.h
===
--- llvm/lib/CodeGen/AsmPrinter/DwarfUnit.h
+++ llvm/lib/CodeGen/AsmPrinter/DwarfUnit.h
@@ -237,6 +237,7 @@
   DIE *getOrCreateNameSpace(const DINamespace *NS);
   DIE *getOrCreateModule(const DIModule *M);
   DIE *getOrCreateSubprogramDIE(const DISubprogram *SP, bool Minimal = false);
+  DIE *getOrCreateLexicalScopeDIE(const DILexicalBlock *LS);
 
   void applySubprogramAttributes(const DISubprogram *SP, DIE &SPDie,
  bool SkipSPAttributes = false);
Index: llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
===
--- llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
+++ llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
@@ -554,6 +554,8 @@
 return getOrCreateSubprogramDIE(SP);
   if (auto *M = dyn_cast(Context))
 return getOrCreateModule(M);
+  if (auto *LS = dyn_cast(Context))
+return getOrCreateLexicalScopeDIE(LS);
   return getDIE(Context);
 }
 
@@ -1181,6 +1183,17 @@
   return &SPDie;
 }
 
+DIE *DwarfUnit::getOrCreateLexicalScopeDIE(const DILexicalBlock *LS) {
+  DIE *ContextDIE = getOrCreateContextDIE(LS->getScope());
+
+  if (DIE *LSDie = getDIE(LS))
+return LSDie;
+
+  DIE &LSDie = createAndAddDIE(dwarf::DW_TAG_lexical_block, *ContextDIE, LS);
+
+  return &LSDie;
+}
+
 bool DwarfUnit::applySubprogramDefinitionAttributes(const DISubprogram *SP,
 DIE &SPDie, bool Minimal) {
   DIE *DeclDie = nullptr;
Index: llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
===
--- llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
+++ llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
@@ -686,6 +686,7 @@
 return nullptr;
 
   auto ScopeDIE = DIE::get(DIEValueAllocator, dwarf::DW_TAG_lexical_block);
+  insertDIE(Scope->getScopeNode(), ScopeDIE);
   if (Scope->isAbstractScope())
 return ScopeDIE;
 
Index: clang/test/OpenMP/debug_containing_scope.c
===
--- /dev/null
+++ clang/test/OpenMP/debug_containing_scope.c
@@ -0,0 +1,68 @@
+// This testcase checks parent child relationship for OpenMP generated
+// functions.
+
+// REQUIRES: x86_64-linux
+
+// RUN: %clang_cc1 -debug-info-kind=constructor -DSHARED -x c -verify -triple x86_64-pc-linux-gnu -fopenmp -emit-llvm %s -o - | FileCheck %s --check-prefix=CHECK
+// expected-no-diagnostics
+
+// CHECK-DAG: call void (ptr, i32, ptr, ...) @__kmpc_fork_call(ptr @8, i32 5, ptr @[[OUTLINED2:.omp_outlined..[0-9]]]
+// CHECK-LABEL: distinct !DICompileUnit
+// CHECK-DAG: [[FOO:![0-9]+]] = distinct !DISubprogram(name: "foo",
+// CHECK-DAG: [[LEX1:![0-9]+]] = distinct !DILexicalBlock(scope: [[FOO]]
+// CHECK-DAG: [[LEX2:![0-9]+]] = distinct !DILexicalBlock(scope: [[LEX1]]
+// CHECK-DAG: [[LEX3:![0-9]+]] = distinct !DILexicalBlock(scope: [[LEX2]]
+// CHECK-DAG: !DISubprogram(linkageName: ".omp_task_privates_map.", scope: [[LEX3]]
+// CHECK-DAG: !DISubprogram(linkageName: ".omp_task_entry.", scope: [[LEX3]]
+// CHECK-DAG: !DISubprogram(name: ".omp_outlined.", scope: [[LEX2]]
+// CHECK-DAG: !DISubprogram(name: ".omp_outlined._debug__", scope: [[LEX2]]
+// CHECK-DAG: !DISubprogram(name: "[[OUTLINED2]]", scope: [[LEX2]]
+
+extern int printf(const char *, ...);
+extern int rand(void);
+
+int global_var1;
+int global_var2 = 99;
+int foo(int n) {
+  int same_var = 5;
+  int other_var = 21;
+  int share = 9, priv, i;
+  global_var1 = 99;
+
+  if (n < 2)
+return n;
+  else {
+int same_var = rand() % 5;
+int local_var = 31;
+#pragma omp task shared(share) private(priv)
+{
+  priv = n;
+  printf("share = %d\n", share);
+  printf("global_var1 = %d\n", global_var1);
+  printf("global_var2 = %d\n", global_var2);
+  printf("same_var = %d\n", same_var);
+  printf("other_var = %d\n", other_var);
+  printf("local_var = %d\n", local_var);
+  share = priv + foo(n - 1);
+}
+#pragma omp taskwait
+
+#pragma omp parallel for
+for (i = 0; i < n; i++) {
+  share += i;
+  printf("share = %d\n", share);
+  printf("global_var1 = %d\n", global_var1);
+  printf("global_var2 = %d\n", global_var2);
+  printf

[clang] cc344d2 - [clang] Add static_cast to fix Bazel build.

[Adrian Kuegel via cfe-commits]

Author: Adrian Kuegel
Date: 2022-05-05T10:29:47+02:00
New Revision: cc344d262a2eaa65f9e380f5b1eeb03a7048fa39

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

LOG: [clang] Add static_cast to fix Bazel build.

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

Added: 


Modified: 
clang/lib/ExtractAPI/ExtractAPIConsumer.cpp

Removed: 




diff  --git a/clang/lib/ExtractAPI/ExtractAPIConsumer.cpp 
b/clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
index 70c8bac59ce97..a7b0a1ac98a78 100644
--- a/clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
+++ b/clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
@@ -927,7 +927,8 @@ bool 
ExtractAPIAction::PrepareToExecuteAction(CompilerInstance &CI) {
 HeaderContents += "\"\n";
   else
 HeaderContents += ">\n";
-  KnownInputFiles.emplace_back(*RelativeName, IsQuoted);
+  KnownInputFiles.emplace_back(static_cast>(*RelativeName),
+   IsQuoted);
 } else {
   HeaderContents += " \"";
   HeaderContents += FilePath;



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

[PATCH] D124995: [clang] Add static_cast to fix Bazel build.

[Adrian Kuegel via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rGcc344d262a2e: [clang] Add static_cast to fix Bazel build. 
(authored by akuegel).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124995

Files:
  clang/lib/ExtractAPI/ExtractAPIConsumer.cpp


Index: clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
===
--- clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
+++ clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
@@ -927,7 +927,8 @@
 HeaderContents += "\"\n";
   else
 HeaderContents += ">\n";
-  KnownInputFiles.emplace_back(*RelativeName, IsQuoted);
+  KnownInputFiles.emplace_back(static_cast>(*RelativeName),
+   IsQuoted);
 } else {
   HeaderContents += " \"";
   HeaderContents += FilePath;


Index: clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
===
--- clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
+++ clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
@@ -927,7 +927,8 @@
 HeaderContents += "\"\n";
   else
 HeaderContents += ">\n";
-  KnownInputFiles.emplace_back(*RelativeName, IsQuoted);
+  KnownInputFiles.emplace_back(static_cast>(*RelativeName),
+   IsQuoted);
 } else {
   HeaderContents += " \"";
   HeaderContents += FilePath;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D124690: [clangd] add inlay hints for std::forward-ed parameter packs

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In D124690#3492981 , @upsj wrote:

> I will take a different approach to the matchers: For each CallExpr involving 
> parameter packs, record the arguments (if they are a ParmVarDecl or 
> stdForward(ParmVarDecl)) and the matching ParmVarDecl of the FunctionDecl in 
> a map. Then I will compact that map to skip intermediate forwarding functions 
> like emplace_back -> allocator::construct -> constructor and in a second pass 
> resolve all previously unresolved forwarded parameters.

This makes sense to me.
This sounds more complicated, but if I'm reading correctly this multi-level 
forwarding isn't handled in the current version of the patch either?
At some point we'll want to extract this logic out so it can be used by hover 
etc, but not yet.




Comment at: clang-tools-extra/clangd/unittests/InlayHintTests.cpp:255
+  )cpp",
+   ExpectedHint{"a: ", "param"});
+}

upsj wrote:
> I haven't been able to figure out why, but for some reason the 
> `CXXConstructExpr` and `CXXTemporaryObjectExpr` are not being picked up by 
> the `RecursiveASTVisitor`, while in a similar situation below, the `CallExpr` 
> gets picked up by `VisitCallExpr`.
The AST here of the instantiated bar looks like
```
-FunctionDecl  line:5:4 used bar 'S *(int &&)'
  |-TemplateArgument type 'S'
  |-TemplateArgument pack
  | `-TemplateArgument type 'int'
  |-ParmVarDecl  col:18 used args 'int &&'
  `-CompoundStmt 
`-ReturnStmt 
  `-CXXNewExpr  'S *' Function 'operator new' 'void 
*(unsigned long)'
`-CXXConstructExpr  'S':'S' 'void (int)' list
  `-ImplicitCastExpr  'int':'int' 
`-DeclRefExpr  'int':'int' lvalue ParmVar 'args' 'int &&'
```

So there's no CXXTemporaryObjectExpr (the value here is a pointer), but should 
be a CXXConstructExpr. 

Are you sure you're traversing bar's instantiation/specializaion, as opposed to 
its templated/generic FunctionDecl? The AST of the templated FunctionDecl 
indeed has an InitListExpr in place of the CXXConstructExpr because it's not 
resolved yet.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124690

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

[PATCH] D124446: [clang-tidy] Add the misc-discarded-return-value check

[Gabor Marton via Phabricator via cfe-commits]

martong added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/misc/DiscardedReturnValueCheck.cpp:106-126
+// FIXME: Move ASTMatcher library.
+// Note: Taken from UnusedUsingDeclsCheck.
+AST_POLYMORPHIC_MATCHER_P(
+forEachTemplateArgument,
+AST_POLYMORPHIC_SUPPORTED_TYPES(ClassTemplateSpecializationDecl,
+TemplateSpecializationType, FunctionDecl),
+clang::ast_matchers::internal::Matcher, InnerMatcher) {

Could you please have this in an independent parent patch?



Comment at: clang-tools-extra/clang-tidy/misc/DiscardedReturnValueCheck.h:28
+public:
+  struct Function {
+std::size_t ConsumedCalls;

The name `Function` is a bit misleading to me, what about `FunctionInfo`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124446

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

[PATCH] D124690: [clangd] add inlay hints for std::forward-ed parameter packs

[Tobias Ribizel via Phabricator via cfe-commits]

upsj added a comment.

> ! In D124690#3493246 , @sammccall 
> wrote:
>
> This makes sense to me.
>  This sounds more complicated, but if I'm reading correctly this multi-level 
> forwarding isn't handled in the current version of the patch either?
>  At some point we'll want to extract this logic out so it can be used by 
> hover etc, but not yet.

Yes, currently only direct forwarding works. I originally looked at this when 
inlay hints were not yet available, and it seemed possible to do this for 
signature help as well, only most likely based on the template instantiation 
pattern instead of the instantiated function declaration.




Comment at: clang-tools-extra/clangd/unittests/InlayHintTests.cpp:255
+  )cpp",
+   ExpectedHint{"a: ", "param"});
+}

sammccall wrote:
> upsj wrote:
> > I haven't been able to figure out why, but for some reason the 
> > `CXXConstructExpr` and `CXXTemporaryObjectExpr` are not being picked up by 
> > the `RecursiveASTVisitor`, while in a similar situation below, the 
> > `CallExpr` gets picked up by `VisitCallExpr`.
> The AST here of the instantiated bar looks like
> ```
> -FunctionDecl  line:5:4 used bar 'S *(int &&)'
>   |-TemplateArgument type 'S'
>   |-TemplateArgument pack
>   | `-TemplateArgument type 'int'
>   |-ParmVarDecl  col:18 used args 'int &&'
>   `-CompoundStmt 
> `-ReturnStmt 
>   `-CXXNewExpr  'S *' Function 'operator new' 'void 
> *(unsigned long)'
> `-CXXConstructExpr  'S':'S' 'void (int)' list
>   `-ImplicitCastExpr  'int':'int' 
> `-DeclRefExpr  'int':'int' lvalue ParmVar 'args' 'int &&'
> ```
> 
> So there's no CXXTemporaryObjectExpr (the value here is a pointer), but 
> should be a CXXConstructExpr. 
> 
> Are you sure you're traversing bar's instantiation/specializaion, as opposed 
> to its templated/generic FunctionDecl? The AST of the templated FunctionDecl 
> indeed has an InitListExpr in place of the CXXConstructExpr because it's not 
> resolved yet.
It's quite possible this is related to the visitor not/only inconsistently 
traversing template instantiations due to shouldVisitTemplateInstantiations 
returning false. But I need to implement the new approach first, anyways.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124690

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

[PATCH] D124448: [clang-tidy] Add project-level analysis support to misc-discarded-return-value

[Gabor Marton via Phabricator via cfe-commits]

martong added inline comments.



Comment at: clang-tools-extra/clang-tidy/misc/DiscardedReturnValueCheck.cpp:132
+
+  auto IDIt = FunctionIDs.find(FD);
+  if (IDIt == FunctionIDs.end()) {

Please spell out the type instead of `auto`.



Comment at: 
clang-tools-extra/clang-tidy/misc/DiscardedReturnValueCheck.cpp:132-142
+  auto IDIt = FunctionIDs.find(FD);
+  if (IDIt == FunctionIDs.end()) {
+SmallString<128> ID;
+bool USRFailure = clang::index::generateUSRForDecl(FD, ID);
+if (USRFailure)
+  ID = FD->getDeclName().getAsString();
+assert(!ID.empty() && "Generating name for function failed");

martong wrote:
> Please spell out the type instead of `auto`.
This could be split out nicely in a member function, e.g. `findFunctionID`.



Comment at: clang-tools-extra/clang-tidy/misc/DiscardedReturnValueCheck.cpp:204
   // Once everything had been counted, emit the results.
-  for (const auto &Call : CallMap)
-diagnose(Call.first, Call.second);
+  if (getPhase() == MultipassProjectPhase::Diagnose)
+for (const auto &Call : CallMap)

How is it emitting dianoses then in the singleTU mode?



Comment at: clang-tools-extra/clang-tidy/misc/DiscardedReturnValueCheck.cpp:365
+  // not invalidate the counters we just loaded.
+  bool ShouldCountMatches = !CacheProjectDataLoadedSuccessfully.getValue();
+

?



Comment at: clang-tools-extra/clang-tidy/misc/DiscardedReturnValueCheck.h:70
   FunctionMapTy CallMap;
+  llvm::DenseMap FunctionIDs;
 

Some docs could be useful here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124448

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

[PATCH] D124244: [analyzer] add StoreToImmutable and ModelConstQualifiedReturn checkers

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

> The patch adds two new checkers.

I don't see any technical dependencies between the two, so, please split it 
into two independent patches.


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

https://reviews.llvm.org/D124244

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

[PATCH] D124658: [analyzer] Canonicalize SymIntExpr so the RHS is positive when possible

[Tomasz Kamiński via Phabricator via cfe-commits]

tomasz-kaminski-sonarsource updated this revision to Diff 427245.

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

https://reviews.llvm.org/D124658

Files:
  clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  clang/test/Analysis/additive-op-on-sym-int-expr.c
  clang/test/Analysis/expr-inspection.c

Index: clang/test/Analysis/expr-inspection.c
===
--- clang/test/Analysis/expr-inspection.c
+++ clang/test/Analysis/expr-inspection.c
@@ -11,7 +11,7 @@
 
 void foo(int x) {
   clang_analyzer_dump(x); // expected-warning{{reg_$0}}
-  clang_analyzer_dump(x + (-1)); // expected-warning{{(reg_$0) + -1}}
+  clang_analyzer_dump(x + (-1)); // expected-warning{{(reg_$0) - 1}}
   int y = 1;
   for (; y < 3; ++y) {
 clang_analyzer_numTimesReached(); // expected-warning{{2}}
Index: clang/test/Analysis/additive-op-on-sym-int-expr.c
===
--- /dev/null
+++ clang/test/Analysis/additive-op-on-sym-int-expr.c
@@ -0,0 +1,58 @@
+// RUN: %clang_analyze_cc1 -triple=x86_64-unknown-linux -analyzer-checker=core,debug.ExprInspection -analyzer-config eagerly-assume=false -verify %s
+
+void clang_analyzer_dump(int);
+void clang_analyzer_dumpL(long);
+void clang_analyzer_warnIfReached();
+
+void testInspect(int x) {
+  if ((x < 10) || (x > 100)) {
+return;
+  }
+  // x: [10, 100]
+
+  int i = x + 1;
+  long l = i - 10U;
+  clang_analyzer_dump(i);   // expected-warning-re {{(reg_${{[0-9]+}}) + 1 }}
+  clang_analyzer_dumpL(l);  // expected-warning-re {{(reg_${{[0-9]+}}) - 9U }} instead of + 4294967287U
+  clang_analyzer_dumpL(l + 0L); // expected-warning-re {{(reg_${{[0-9]+}}) - 9 }}  instead of + 4294967287
+
+  if ((l - 1000) > 0) {
+clang_analyzer_warnIfReached(); // no-warning
+  }
+  if (l > 1000) {
+clang_analyzer_warnIfReached(); // no-warning
+  }
+  if (l > 1000L) {
+clang_analyzer_warnIfReached(); // no-warning
+  }
+  if ((l + 0L) > 1000) {
+clang_analyzer_warnIfReached(); // no-warning
+  }
+
+  i = x - 1;
+  l = i + 10U;
+  clang_analyzer_dumpL(l); // expected-warning-re {{(reg_${{[0-9]+}}) + 9U }} instead of - 4294967287U
+
+  i = x + (-1);
+  l = i - 10U;
+  clang_analyzer_dumpL(l); // expected-warning-re {{(reg_${{[0-9]+}}) - 11U }} instead of + 4294967285U
+
+  i = x + 1U;
+  l = i - 10U;
+  clang_analyzer_dumpL(l); // expected-warning-re {{(reg_${{[0-9]+}}) - 9U }} instead of + 4294967287U
+}
+
+void testMin(int i, long l) {
+  clang_analyzer_dump(i + (-1));  // expected-warning-re {{(reg_${{[0-9]+}}) - 1 }} instead of + -1
+  clang_analyzer_dump(i - (-1));  // expected-warning-re {{(reg_${{[0-9]+}}) + 1 }} instead of - -1
+  clang_analyzer_dumpL(l + (-1)); // expected-warning-re {{(reg_${{[0-9]+}}) - 1 }} instead of + -1
+  clang_analyzer_dumpL(l - (-1)); // expected-warning-re {{(reg_${{[0-9]+}}) + 1 }} instead of - -1
+
+  int intMin = 1 << (sizeof(int) * 8 - 1); // INT_MIN, negative value is not representable
+  // Do not normalize representation if negation would not be representable
+  clang_analyzer_dump(i + intMin); // expected-warning-re {{(reg_${{[0-9]+}}) + -2147483648 }}
+  clang_analyzer_dump(i - intMin); // expected-warning-re {{(reg_${{[0-9]+}}) - -2147483648 }}
+  // Produced value has higher bit with (long) so negation if representable
+  clang_analyzer_dumpL(l + intMin); // expected-warning-re {{(reg_${{[0-9]+}}) - 2147483648 }} instead of + -2147483648
+  clang_analyzer_dumpL(l - intMin); // expected-warning-re {{(reg_${{[0-9]+}}) + 2147483648 }} instead of - -2147483648
+}
Index: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
===
--- clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
+++ clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
@@ -197,8 +197,27 @@
   if (RHS.isSigned() && !SymbolType->isSignedIntegerOrEnumerationType())
 ConvertedRHS = &BasicVals.Convert(SymbolType, RHS);
 }
-  } else
+  } else if (BinaryOperator::isAdditiveOp(op) && RHS.isNegative()) {
+// Change a+(-N) into a-N, and a-(-N) into a+N
+// Adjust addition/subtraction of negative value, to
+// subtraction/addition of the negated value.
+APSIntType resultIntTy = BasicVals.getAPSIntType(resultTy);
+assert(resultIntTy.getBitWidth() >= RHS.getBitWidth() &&
+   "The result operation type must have at least the same "
+   "number of bits as its operands.");
+
+llvm::APSInt ConvertedRHSValue = resultIntTy.convert(RHS);
+// Check if the negation of the RHS is representable:
+// * if resultIntTy is unsigned, then negation is always representable
+// * if resultIntTy is signed, and RHS is not the lowest representable
+//   signed value
+if (resultIntTy.isUnsigned() || !ConvertedRHSValue.isMinSignedValue()) {
+  ConvertedRHS = &BasicVals.getValue(-ConvertedRHSValue);
+  op = (op == BO_Add) ? BO_Sub : BO_Add;
+   

[PATCH] D124244: [analyzer] add StoreToImmutable and ModelConstQualifiedReturn checkers

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

Also, could you please update the `Static Analyzer` section of 
`clang/docs/ReleaseNotes.rst` with the new checkers and their description?


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

https://reviews.llvm.org/D124244

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

[PATCH] D120489: [analyzer] Done some changes to detect Uninitialized read by the char array manipulation functions

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

Maybe it is not too late to update the clang/docs/ReleaseNotes.rst? A new 
checker is certainly important for the users. Many thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120489

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

[PATCH] D124349: [analyzer] Get direct binding for specific punned case

[Vince Bridgers via Phabricator via cfe-commits]

vabridgers updated this revision to Diff 427246.
vabridgers added a comment.

fix a stupid leftover cut/paste mistake in the test case :/


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124349

Files:
  clang/lib/StaticAnalyzer/Core/RegionStore.cpp
  clang/test/Analysis/array-punned-region.c


Index: clang/test/Analysis/array-punned-region.c
===
--- /dev/null
+++ clang/test/Analysis/array-punned-region.c
@@ -0,0 +1,39 @@
+// RUN: %clang_analyze_cc1 
-analyzer-checker=core,alpha.core,debug.ExprInspection -verify -analyzer-config 
eagerly-assume=false -triple x86_64-pc-linux-gnu %s
+
+// RUN: %clang_analyze_cc1 
-analyzer-checker=core,alpha.core,debug.ExprInspection -verify -analyzer-config 
eagerly-assume=false -triple i386-pc-linux-gnu  %s
+
+int clang_analyzer_eval(int);
+
+typedef struct {
+  int a : 1;
+  int b[2];
+} BITFIELD_CAST;
+
+void array_struct_bitfield_1() {
+  BITFIELD_CAST ff = {0};
+  BITFIELD_CAST *pff = &ff;
+  clang_analyzer_eval(*((int *)pff + 1) == 0); // expected-warning{{TRUE}}
+  ff.b[0] = 3;
+  clang_analyzer_eval(*((int *)pff + 1) == 3); // expected-warning{{TRUE}}
+}
+
+int array_struct_bitfield_2() {
+  BITFIELD_CAST ff = {0};
+  BITFIELD_CAST *pff = &ff;
+  int a = *((int *)pff + 2); // expected-warning{{Assigned value is garbage or 
undefined [core.uninitialized.Assign]}}
+  return a;
+}
+
+typedef struct {
+  unsigned int a : 1;
+  unsigned int x : 31;
+  unsigned int c : 1;
+  int b[2];
+} mystruct;
+
+void array_struct_bitfield_3() {
+  mystruct ff;
+  mystruct *pff = &ff;
+  ff.b[0] = 3;
+  clang_analyzer_eval(*((int *)pff + 2) == 3); // expected-warning{{TRUE}}
+}
Index: clang/lib/StaticAnalyzer/Core/RegionStore.cpp
===
--- clang/lib/StaticAnalyzer/Core/RegionStore.cpp
+++ clang/lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -2147,8 +2147,13 @@
   return UnknownVal();
 
 // Additionally allow introspection of a block's internal layout.
-if (!hasPartialLazyBinding && !isa(R->getBaseRegion()))
+// Try to get direct binding if all other attempts failed thus far.
+// Else, return UndefinedVal()
+if (!hasPartialLazyBinding && !isa(R->getBaseRegion())) {
+  if (const Optional &V = B.getDefaultBinding(R))
+return *V;
   return UndefinedVal();
+}
   }
 
   // All other values are symbolic.


Index: clang/test/Analysis/array-punned-region.c
===
--- /dev/null
+++ clang/test/Analysis/array-punned-region.c
@@ -0,0 +1,39 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core,debug.ExprInspection -verify -analyzer-config eagerly-assume=false -triple x86_64-pc-linux-gnu %s
+
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core,debug.ExprInspection -verify -analyzer-config eagerly-assume=false -triple i386-pc-linux-gnu  %s
+
+int clang_analyzer_eval(int);
+
+typedef struct {
+  int a : 1;
+  int b[2];
+} BITFIELD_CAST;
+
+void array_struct_bitfield_1() {
+  BITFIELD_CAST ff = {0};
+  BITFIELD_CAST *pff = &ff;
+  clang_analyzer_eval(*((int *)pff + 1) == 0); // expected-warning{{TRUE}}
+  ff.b[0] = 3;
+  clang_analyzer_eval(*((int *)pff + 1) == 3); // expected-warning{{TRUE}}
+}
+
+int array_struct_bitfield_2() {
+  BITFIELD_CAST ff = {0};
+  BITFIELD_CAST *pff = &ff;
+  int a = *((int *)pff + 2); // expected-warning{{Assigned value is garbage or undefined [core.uninitialized.Assign]}}
+  return a;
+}
+
+typedef struct {
+  unsigned int a : 1;
+  unsigned int x : 31;
+  unsigned int c : 1;
+  int b[2];
+} mystruct;
+
+void array_struct_bitfield_3() {
+  mystruct ff;
+  mystruct *pff = &ff;
+  ff.b[0] = 3;
+  clang_analyzer_eval(*((int *)pff + 2) == 3); // expected-warning{{TRUE}}
+}
Index: clang/lib/StaticAnalyzer/Core/RegionStore.cpp
===
--- clang/lib/StaticAnalyzer/Core/RegionStore.cpp
+++ clang/lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -2147,8 +2147,13 @@
   return UnknownVal();
 
 // Additionally allow introspection of a block's internal layout.
-if (!hasPartialLazyBinding && !isa(R->getBaseRegion()))
+// Try to get direct binding if all other attempts failed thus far.
+// Else, return UndefinedVal()
+if (!hasPartialLazyBinding && !isa(R->getBaseRegion())) {
+  if (const Optional &V = B.getDefaultBinding(R))
+return *V;
   return UndefinedVal();
+}
   }
 
   // All other values are symbolic.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D124349: [analyzer] Get direct binding for specific punned case

[Vince Bridgers via Phabricator via cfe-commits]

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

Thanks @martong. I fixed the "typo" :/, landing. Best!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124349

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

[clang] df58018 - [analyzer] Get direct binding for specific punned case

[via cfe-commits]

Author: einvbri
Date: 2022-05-05T04:53:45-05:00
New Revision: df5801806d03c22099c85942134ca3004776016b

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

LOG: [analyzer] Get direct binding for specific punned case

Region store was not able to see through this case to the actual
initialized value of STRUCT ff. This change addresses this case by
getting the direct binding. This was found and debugged in a downstream
compiler, with debug guidance from @steakhal. A positive and negative
test case is added.

The specific case where this issue was exposed.

  typedef struct {
int a:1;
int b[2];
  } STRUCT;

  int main() {
STRUCT ff = {0};
STRUCT* pff = &ff;
int a = ((int)pff + 1);
return a;
  }

Reviewed By: steakhal, martong

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

Added: 
clang/test/Analysis/array-punned-region.c

Modified: 
clang/lib/StaticAnalyzer/Core/RegionStore.cpp

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Core/RegionStore.cpp 
b/clang/lib/StaticAnalyzer/Core/RegionStore.cpp
index 20b167c9b3b22..43924cd4b5424 100644
--- a/clang/lib/StaticAnalyzer/Core/RegionStore.cpp
+++ b/clang/lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -2147,8 +2147,13 @@ 
RegionStoreManager::getBindingForFieldOrElementCommon(RegionBindingsConstRef B,
   return UnknownVal();
 
 // Additionally allow introspection of a block's internal layout.
-if (!hasPartialLazyBinding && !isa(R->getBaseRegion()))
+// Try to get direct binding if all other attempts failed thus far.
+// Else, return UndefinedVal()
+if (!hasPartialLazyBinding && !isa(R->getBaseRegion())) {
+  if (const Optional &V = B.getDefaultBinding(R))
+return *V;
   return UndefinedVal();
+}
   }
 
   // All other values are symbolic.

diff  --git a/clang/test/Analysis/array-punned-region.c 
b/clang/test/Analysis/array-punned-region.c
new file mode 100644
index 0..d319fd7367ec5
--- /dev/null
+++ b/clang/test/Analysis/array-punned-region.c
@@ -0,0 +1,39 @@
+// RUN: %clang_analyze_cc1 
-analyzer-checker=core,alpha.core,debug.ExprInspection -verify -analyzer-config 
eagerly-assume=false -triple x86_64-pc-linux-gnu %s
+
+// RUN: %clang_analyze_cc1 
-analyzer-checker=core,alpha.core,debug.ExprInspection -verify -analyzer-config 
eagerly-assume=false -triple i386-pc-linux-gnu  %s
+
+int clang_analyzer_eval(int);
+
+typedef struct {
+  int a : 1;
+  int b[2];
+} BITFIELD_CAST;
+
+void array_struct_bitfield_1() {
+  BITFIELD_CAST ff = {0};
+  BITFIELD_CAST *pff = &ff;
+  clang_analyzer_eval(*((int *)pff + 1) == 0); // expected-warning{{TRUE}}
+  ff.b[0] = 3;
+  clang_analyzer_eval(*((int *)pff + 1) == 3); // expected-warning{{TRUE}}
+}
+
+int array_struct_bitfield_2() {
+  BITFIELD_CAST ff = {0};
+  BITFIELD_CAST *pff = &ff;
+  int a = *((int *)pff + 2); // expected-warning{{Assigned value is garbage or 
undefined [core.uninitialized.Assign]}}
+  return a;
+}
+
+typedef struct {
+  unsigned int a : 1;
+  unsigned int x : 31;
+  unsigned int c : 1;
+  int b[2];
+} mystruct;
+
+void array_struct_bitfield_3() {
+  mystruct ff;
+  mystruct *pff = &ff;
+  ff.b[0] = 3;
+  clang_analyzer_eval(*((int *)pff + 2) == 3); // expected-warning{{TRUE}}
+}



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

[PATCH] D124349: [analyzer] Get direct binding for specific punned case

[Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGdf5801806d03: [analyzer] Get direct binding for specific 
punned case (authored by einvbri ).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124349

Files:
  clang/lib/StaticAnalyzer/Core/RegionStore.cpp
  clang/test/Analysis/array-punned-region.c


Index: clang/test/Analysis/array-punned-region.c
===
--- /dev/null
+++ clang/test/Analysis/array-punned-region.c
@@ -0,0 +1,39 @@
+// RUN: %clang_analyze_cc1 
-analyzer-checker=core,alpha.core,debug.ExprInspection -verify -analyzer-config 
eagerly-assume=false -triple x86_64-pc-linux-gnu %s
+
+// RUN: %clang_analyze_cc1 
-analyzer-checker=core,alpha.core,debug.ExprInspection -verify -analyzer-config 
eagerly-assume=false -triple i386-pc-linux-gnu  %s
+
+int clang_analyzer_eval(int);
+
+typedef struct {
+  int a : 1;
+  int b[2];
+} BITFIELD_CAST;
+
+void array_struct_bitfield_1() {
+  BITFIELD_CAST ff = {0};
+  BITFIELD_CAST *pff = &ff;
+  clang_analyzer_eval(*((int *)pff + 1) == 0); // expected-warning{{TRUE}}
+  ff.b[0] = 3;
+  clang_analyzer_eval(*((int *)pff + 1) == 3); // expected-warning{{TRUE}}
+}
+
+int array_struct_bitfield_2() {
+  BITFIELD_CAST ff = {0};
+  BITFIELD_CAST *pff = &ff;
+  int a = *((int *)pff + 2); // expected-warning{{Assigned value is garbage or 
undefined [core.uninitialized.Assign]}}
+  return a;
+}
+
+typedef struct {
+  unsigned int a : 1;
+  unsigned int x : 31;
+  unsigned int c : 1;
+  int b[2];
+} mystruct;
+
+void array_struct_bitfield_3() {
+  mystruct ff;
+  mystruct *pff = &ff;
+  ff.b[0] = 3;
+  clang_analyzer_eval(*((int *)pff + 2) == 3); // expected-warning{{TRUE}}
+}
Index: clang/lib/StaticAnalyzer/Core/RegionStore.cpp
===
--- clang/lib/StaticAnalyzer/Core/RegionStore.cpp
+++ clang/lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -2147,8 +2147,13 @@
   return UnknownVal();
 
 // Additionally allow introspection of a block's internal layout.
-if (!hasPartialLazyBinding && !isa(R->getBaseRegion()))
+// Try to get direct binding if all other attempts failed thus far.
+// Else, return UndefinedVal()
+if (!hasPartialLazyBinding && !isa(R->getBaseRegion())) {
+  if (const Optional &V = B.getDefaultBinding(R))
+return *V;
   return UndefinedVal();
+}
   }
 
   // All other values are symbolic.


Index: clang/test/Analysis/array-punned-region.c
===
--- /dev/null
+++ clang/test/Analysis/array-punned-region.c
@@ -0,0 +1,39 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core,debug.ExprInspection -verify -analyzer-config eagerly-assume=false -triple x86_64-pc-linux-gnu %s
+
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core,debug.ExprInspection -verify -analyzer-config eagerly-assume=false -triple i386-pc-linux-gnu  %s
+
+int clang_analyzer_eval(int);
+
+typedef struct {
+  int a : 1;
+  int b[2];
+} BITFIELD_CAST;
+
+void array_struct_bitfield_1() {
+  BITFIELD_CAST ff = {0};
+  BITFIELD_CAST *pff = &ff;
+  clang_analyzer_eval(*((int *)pff + 1) == 0); // expected-warning{{TRUE}}
+  ff.b[0] = 3;
+  clang_analyzer_eval(*((int *)pff + 1) == 3); // expected-warning{{TRUE}}
+}
+
+int array_struct_bitfield_2() {
+  BITFIELD_CAST ff = {0};
+  BITFIELD_CAST *pff = &ff;
+  int a = *((int *)pff + 2); // expected-warning{{Assigned value is garbage or undefined [core.uninitialized.Assign]}}
+  return a;
+}
+
+typedef struct {
+  unsigned int a : 1;
+  unsigned int x : 31;
+  unsigned int c : 1;
+  int b[2];
+} mystruct;
+
+void array_struct_bitfield_3() {
+  mystruct ff;
+  mystruct *pff = &ff;
+  ff.b[0] = 3;
+  clang_analyzer_eval(*((int *)pff + 2) == 3); // expected-warning{{TRUE}}
+}
Index: clang/lib/StaticAnalyzer/Core/RegionStore.cpp
===
--- clang/lib/StaticAnalyzer/Core/RegionStore.cpp
+++ clang/lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -2147,8 +2147,13 @@
   return UnknownVal();
 
 // Additionally allow introspection of a block's internal layout.
-if (!hasPartialLazyBinding && !isa(R->getBaseRegion()))
+// Try to get direct binding if all other attempts failed thus far.
+// Else, return UndefinedVal()
+if (!hasPartialLazyBinding && !isa(R->getBaseRegion())) {
+  if (const Optional &V = B.getDefaultBinding(R))
+return *V;
   return UndefinedVal();
+}
   }
 
   // All other values are symbolic.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D124996: [clang][preprocessor] Fix unsigned-ness of utf8 char literals

[Timm Bäder via Phabricator via cfe-commits]

tbaeder created this revision.
tbaeder added reviewers: aaron.ballman, tahonermann.
Herald added a project: All.
tbaeder requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

UTF8 char literals are always unsigned.

Fixes https://github.com/llvm/llvm-project/issues/54886


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D124996

Files:
  clang/lib/Lex/PPExpressions.cpp
  clang/test/Lexer/utf8-char-literal.cpp


Index: clang/test/Lexer/utf8-char-literal.cpp
===
--- clang/test/Lexer/utf8-char-literal.cpp
+++ clang/test/Lexer/utf8-char-literal.cpp
@@ -26,3 +26,10 @@
  unsigned char : 1),
 "Surprise!");
 #endif
+
+/// Test u8 char literal preprocessor behavior
+#if __cplusplus > 201402L || __STDC_VERSION__ > 202000L
+#if u8'\xff' != 0xff
+#error u8 char literal is not unsigned
+#endif
+#endif
Index: clang/lib/Lex/PPExpressions.cpp
===
--- clang/lib/Lex/PPExpressions.cpp
+++ clang/lib/Lex/PPExpressions.cpp
@@ -407,10 +407,10 @@
 llvm::APSInt Val(NumBits);
 // Set the value.
 Val = Literal.getValue();
-// Set the signedness. UTF-16 and UTF-32 are always unsigned
+// Set the signedness. UTF-8, UTF-16 and UTF-32 are always unsigned
 if (Literal.isWide())
   Val.setIsUnsigned(!TargetInfo::isTypeSigned(TI.getWCharType()));
-else if (!Literal.isUTF16() && !Literal.isUTF32())
+else if (!Literal.isUTF8() && !Literal.isUTF16() && !Literal.isUTF32())
   Val.setIsUnsigned(!PP.getLangOpts().CharIsSigned);
 
 if (Result.Val.getBitWidth() > Val.getBitWidth()) {


Index: clang/test/Lexer/utf8-char-literal.cpp
===
--- clang/test/Lexer/utf8-char-literal.cpp
+++ clang/test/Lexer/utf8-char-literal.cpp
@@ -26,3 +26,10 @@
  unsigned char : 1),
 "Surprise!");
 #endif
+
+/// Test u8 char literal preprocessor behavior
+#if __cplusplus > 201402L || __STDC_VERSION__ > 202000L
+#if u8'\xff' != 0xff
+#error u8 char literal is not unsigned
+#endif
+#endif
Index: clang/lib/Lex/PPExpressions.cpp
===
--- clang/lib/Lex/PPExpressions.cpp
+++ clang/lib/Lex/PPExpressions.cpp
@@ -407,10 +407,10 @@
 llvm::APSInt Val(NumBits);
 // Set the value.
 Val = Literal.getValue();
-// Set the signedness. UTF-16 and UTF-32 are always unsigned
+// Set the signedness. UTF-8, UTF-16 and UTF-32 are always unsigned
 if (Literal.isWide())
   Val.setIsUnsigned(!TargetInfo::isTypeSigned(TI.getWCharType()));
-else if (!Literal.isUTF16() && !Literal.isUTF32())
+else if (!Literal.isUTF8() && !Literal.isUTF16() && !Literal.isUTF32())
   Val.setIsUnsigned(!PP.getLangOpts().CharIsSigned);
 
 if (Result.Val.getBitWidth() > Val.getBitWidth()) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D124774: [clang][ASTImporter][NFC]: Move clang::ImportError into own header.

[Shivam Rajput via Phabricator via cfe-commits]

phyBrackets updated this revision to Diff 427252.
phyBrackets added a comment.

address inline comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124774

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


Index: clang/include/clang/AST/ASTImportError.h
===
--- clang/include/clang/AST/ASTImportError.h
+++ clang/include/clang/AST/ASTImportError.h
@@ -1,4 +1,4 @@
-//===- ASTImportError.h -  Define errors while importing AST  ---*- C++
+//===- ASTImportError.h - Define errors while importing AST  ---*- C++
 //-*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.


Index: clang/include/clang/AST/ASTImportError.h
===
--- clang/include/clang/AST/ASTImportError.h
+++ clang/include/clang/AST/ASTImportError.h
@@ -1,4 +1,4 @@
-//===- ASTImportError.h -  Define errors while importing AST  ---*- C++
+//===- ASTImportError.h - Define errors while importing AST  ---*- C++
 //-*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D124774: [clang][ASTImporter][NFC]: Move clang::ImportError into own header.

[Shivam Rajput via Phabricator via cfe-commits]

phyBrackets added a comment.

I'm having a problem , this patch expected to address the previous inline 
comment but after making the specific change and committing it , when  I'm 
creating the patch using //arc diff --update // , the changes were 
suddenly gone and I thought that maybe the changes were there in the commit 
atleast but I think commit got reverted and there is nothing for diff but 
suddenly the patch were created .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124774

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

[PATCH] D124998: [AArch64][SVE] Add aarch64_sve_pcs attribute to Clang

[Matt Devereau via Phabricator via cfe-commits]

MattDevereau created this revision.
MattDevereau added reviewers: paulwalker-arm, peterwaller-arm, bsmith, 
DavidTruby, dtemirbulatov.
Herald added subscribers: ctetreau, psnobl, arphaman, kristof.beyls, tschuett.
Herald added a reviewer: efriedma.
Herald added a reviewer: aaron.ballman.
Herald added a project: All.
MattDevereau requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

[AArch64][SVE] Add aarch64_sve_pcs attribute to Clang

Enable function attribute aarch64_sve_pcs at the C level, which correspondes to 
aarch64_sve_vector_pcs at the LLVM IR level.

This requirement was created by this addition to the ARM C Language Extension:
https://github.com/ARM-software/acle/pull/194


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D124998

Files:
  clang/include/clang-c/Index.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/Specifiers.h
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/Type.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/Basic/Targets/AArch64.cpp
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/CodeGen/aarch64-svepcs.c
  clang/test/Sema/aarch64-svepcs.c
  clang/test/Sema/callingconv.c
  clang/tools/libclang/CXType.cpp

Index: clang/tools/libclang/CXType.cpp
===
--- clang/tools/libclang/CXType.cpp
+++ clang/tools/libclang/CXType.cpp
@@ -666,6 +666,7 @@
   TCALLINGCONV(X86RegCall);
   TCALLINGCONV(X86VectorCall);
   TCALLINGCONV(AArch64VectorCall);
+  TCALLINGCONV(AArch64SVEPcs);
   TCALLINGCONV(Win64);
   TCALLINGCONV(X86_64SysV);
   TCALLINGCONV(AAPCS);
Index: clang/test/Sema/callingconv.c
===
--- clang/test/Sema/callingconv.c
+++ clang/test/Sema/callingconv.c
@@ -52,6 +52,7 @@
 int __attribute__((pcs("foo"))) pcs7(void); // expected-error {{invalid PCS type}}
 
 int __attribute__((aarch64_vector_pcs)) aavpcs(void); // expected-warning {{'aarch64_vector_pcs' calling convention is not supported for this target}}
+int __attribute__((aarch64_sve_pcs)) aasvepcs(void); // expected-warning {{'aarch64_sve_pcs' calling convention is not supported for this target}}
 
 // PR6361
 void ctest3();
Index: clang/test/Sema/aarch64-svepcs.c
===
--- /dev/null
+++ clang/test/Sema/aarch64-svepcs.c
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -triple aarch64-linux-gnu -target-feature +sve -verify %s
+
+typedef __attribute__((aarch64_sve_pcs)) int invalid_typedef; // expected-warning {{'aarch64_sve_pcs' only applies to function types; type here is 'int'}}
+
+void __attribute__((aarch64_sve_pcs(0))) foo0(void); // expected-error {{'aarch64_sve_pcs' attribute takes no arguments}}
+
+void __attribute__((aarch64_sve_pcs, preserve_all)) foo1(void); // expected-error {{not compatible}}
+
+void __attribute__((cdecl)) foo2(void);// expected-note {{previous declaration is here}}
+void __attribute__((aarch64_sve_pcs)) foo2(void) {} // expected-error {{function declared 'aarch64_sve_pcs' here was previously declared 'cdecl'}}
+
+void foo3(void);   // expected-note {{previous declaration is here}}
+void __attribute__((aarch64_sve_pcs)) foo3(void) {} // expected-error {{function declared 'aarch64_sve_pcs' here was previously declared without calling convention}}
+
+typedef int (*fn_ty)(void);
+typedef int __attribute__((aarch64_sve_pcs)) (*aasvepcs_fn_ty)(void);
+void foo4(fn_ty ptr1, aasvepcs_fn_ty ptr2) {
+  ptr1 = ptr2; // expected-warning {{incompatible function pointer types}}
+}
Index: clang/test/CodeGen/aarch64-svepcs.c
===
--- /dev/null
+++ clang/test/CodeGen/aarch64-svepcs.c
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -triple aarch64-linux-gnu -emit-llvm -o - %s | FileCheck %s -check-prefix=CHECKC
+// RUN: %clang_cc1 -triple aarch64-linux-gnu -emit-llvm -x c++ -o - %s | FileCheck %s -check-prefix=CHECKCXX
+// RUN: %clang_cc1 -triple i686-pc-linux-gnu -verify %s
+
+void __attribute__((aarch64_sve_pcs)) f(int *); // expected-warning {{'aarch64_sve_pcs' calling convention is not supported for this target}}
+
+// CHECKC: define{{.*}} void @g(
+// CHECKCXX: define{{.*}} void @_Z1gPi(
+void g(int *a) {
+
+// CHECKC: call aarch64_sve_vector_pcs void @f(
+// CHECKCXX: call aarch64_sve_vector_pcs void @_Z1fPi
+  f(a);
+}
+
+// CHECKC: declare aarch64_sve_vector_pcs void @f(
+// CHECKCXX: declare aarch64_sve_vector_pcs void @_Z1fPi
+
+void __attribute__((aarch64_sve_pcs)) h(int *a){ // expected-warning {{'aarch64_sve_pcs' calling convention is not supported for this target}}
+// CHECKC: define{{.*}} aarch64_sve_vector_pcs void @h(
+// CHECKCXX: define{{.*}} aarch64_sve_vector_pcs void @_Z1hPi

[PATCH] D124774: [clang][ASTImporter][NFC]: Move clang::ImportError into own header.

[Shivam Rajput via Phabricator via cfe-commits]

phyBrackets updated this revision to Diff 427259.
phyBrackets added a comment.

address inline comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124774

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


Index: clang/include/clang/AST/ASTImportError.h
===
--- clang/include/clang/AST/ASTImportError.h
+++ clang/include/clang/AST/ASTImportError.h
@@ -1,5 +1,4 @@
-//===- ASTImportError.h - Define errors while importing AST  ---*- C++
-//-*-===//
+//===- ASTImportError.h - Define errors while importing AST *- C++-*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.


Index: clang/include/clang/AST/ASTImportError.h
===
--- clang/include/clang/AST/ASTImportError.h
+++ clang/include/clang/AST/ASTImportError.h
@@ -1,5 +1,4 @@
-//===- ASTImportError.h - Define errors while importing AST  ---*- C++
-//-*-===//
+//===- ASTImportError.h - Define errors while importing AST *- C++-*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D124774: [clang][ASTImporter][NFC]: Move clang::ImportError into own header.

[Shivam Rajput via Phabricator via cfe-commits]

phyBrackets added a comment.

In D124774#3493382 , @phyBrackets 
wrote:

> I'm having a problem , this patch expected to address the previous inline 
> comment but after making the specific change and committing it , when  I'm 
> creating the patch using //arc diff --update // , the changes 
> were suddenly gone and I thought that maybe the changes were there in the 
> commit atleast but I think commit got reverted and there is nothing for diff 
> but suddenly the patch were created .

I think there was the problem with number of the character in line 1 , Now it 
seems correct .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124774

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

[PATCH] D124774: [clang][ASTImporter][NFC]: Move clang::ImportError into own header.

[Shivam Rajput via Phabricator via cfe-commits]

phyBrackets updated this revision to Diff 427261.
phyBrackets added a comment.

Correct the number of character in line 1


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124774

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


Index: clang/include/clang/AST/ASTImportError.h
===
--- clang/include/clang/AST/ASTImportError.h
+++ clang/include/clang/AST/ASTImportError.h
@@ -1,4 +1,4 @@
-//===- ASTImportError.h - Define errors while importing AST *- C++-*-===//
+//===- ASTImportError.h - Define errors while importing AST -*- C++-*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.


Index: clang/include/clang/AST/ASTImportError.h
===
--- clang/include/clang/AST/ASTImportError.h
+++ clang/include/clang/AST/ASTImportError.h
@@ -1,4 +1,4 @@
-//===- ASTImportError.h - Define errors while importing AST *- C++-*-===//
+//===- ASTImportError.h - Define errors while importing AST -*- C++-*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D124726: Suggest typoed directives in preprocessor conditionals

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui updated this revision to Diff 427262.
ken-matsui added a comment.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Update codes as reviewed


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124726

Files:
  clang/include/clang/Basic/DiagnosticLexKinds.td
  clang/include/clang/Lex/Preprocessor.h
  clang/lib/Lex/PPDirectives.cpp
  clang/test/Preprocessor/suggest-typoed-directive.c
  llvm/include/llvm/ADT/StringRef.h
  llvm/unittests/ADT/StringRefTest.cpp

Index: llvm/unittests/ADT/StringRefTest.cpp
===
--- llvm/unittests/ADT/StringRefTest.cpp
+++ llvm/unittests/ADT/StringRefTest.cpp
@@ -566,6 +566,13 @@
 }
 
 TEST(StringRefTest, EditDistance) {
+  EXPECT_EQ(0U, StringRef("").edit_distance(""));
+  EXPECT_EQ(1U, StringRef("").edit_distance("aaab"));
+  EXPECT_EQ(2U, StringRef("aabc").edit_distance("aacb"));
+  EXPECT_EQ(2U, StringRef("aabc").edit_distance("abca"));
+  EXPECT_EQ(3U, StringRef("aabc").edit_distance("adef"));
+  EXPECT_EQ(4U, StringRef("abcd").edit_distance("efgh"));
+
   StringRef Hello("hello");
   EXPECT_EQ(2U, Hello.edit_distance("hill"));
 
@@ -584,6 +591,40 @@
"people soiled our green "));
 }
 
+TEST(StringRefTest, FindSimilarStr) {
+  {
+constexpr StringRef Candidates[] = {"aaab", "aaabc"};
+EXPECT_EQ(std::string("aaab"), StringRef("").find_similar_str(Candidates));
+  }
+  {
+constexpr StringRef Candidates[] = {"aab", "abc"};
+EXPECT_EQ(std::string("aab"), StringRef("aaa").find_similar_str(Candidates));
+  }
+  {
+constexpr StringRef Candidates[] = {"ab", "bc"};
+EXPECT_EQ(std::string("ab"), StringRef("aa").find_similar_str(Candidates));
+  }
+  {
+constexpr StringRef Candidates[] = {"b", "c"};
+EXPECT_EQ(None, StringRef("a").find_similar_str(Candidates));
+  }
+  { // macros
+constexpr StringRef Candidates[] = {
+"if", "ifdef", "ifndef", "elif", "elifdef", "elifndef", "else", "endif"
+};
+EXPECT_EQ(std::string("elifdef"), StringRef("elfidef").find_similar_str(Candidates));
+EXPECT_EQ(std::string("elifdef"), StringRef("elifdef").find_similar_str(Candidates));
+EXPECT_EQ(None, StringRef("special_compiler_directive").find_similar_str(Candidates));
+  }
+  { // case-insensitive
+constexpr StringRef Candidates[] = {
+"if", "ifdef", "ifndef", "elif", "elifdef", "elifndef", "else", "endif"
+};
+EXPECT_EQ(std::string("elifdef"), StringRef("elifdef").find_similar_str(Candidates));
+EXPECT_EQ(std::string("elifdef"), StringRef("ELIFDEF").find_similar_str(Candidates));
+  }
+}
+
 TEST(StringRefTest, Misc) {
   std::string Storage;
   raw_string_ostream OS(Storage);
Index: llvm/include/llvm/ADT/StringRef.h
===
--- llvm/include/llvm/ADT/StringRef.h
+++ llvm/include/llvm/ADT/StringRef.h
@@ -10,6 +10,7 @@
 #define LLVM_ADT_STRINGREF_H
 
 #include "llvm/ADT/DenseMapInfo.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/ADT/STLFunctionalExtras.h"
 #include "llvm/ADT/iterator_range.h"
 #include "llvm/Support/Compiler.h"
@@ -24,6 +25,7 @@
 #endif
 #include 
 #include 
+#include 
 
 // Declare the __builtin_strlen intrinsic for MSVC so it can be used in
 // constexpr context.
@@ -240,6 +242,46 @@
 unsigned edit_distance(StringRef Other, bool AllowReplacements = true,
unsigned MaxEditDistance = 0) const;
 
+/// Find a similar string in `Candidates`.
+template 
+Optional find_similar_str(const StringRef (&Candidates)[Size],
+   size_t Dist = 0) const {
+  // We need to check if `rng` has the exact case-insensitive string because the
+  // Levenshtein distance match does not care about it.
+  for (StringRef C : Candidates) {
+if (equals_insensitive(C)) {
+  return C.str();
+}
+  }
+
+  // Keep going with the Levenshtein distance match.
+  // If dist is given, use the dist for maxDist; otherwise, if word size is
+  // less than 3, use the word size minus 1 and if not, use the word size
+  // divided by 3.
+  size_t MaxDist = Dist != 0? Dist
+   : Length < 3 ? Length - 1
+: Length / 3;
+
+  std::vector> Cand;
+  for (StringRef C : Candidates) {
+size_t CurDist = edit_distance(C, false);
+if (CurDist <= MaxDist) {
+  Cand.emplace_back(C, CurDist);
+}
+  }
+
+  if (Cand.empty()) {
+return None;
+  } else if (Cand.size() == 1) {
+return Cand[0].first;
+  } else {
+auto SimilarStr = std::min_element(
+Cand.cbegin(), Cand.cend(),
+[](const auto &A, const auto &B) { return A.second < B.second; });
+return SimilarStr->first;
+   

[PATCH] D124447: [clang-tidy] Add infrastructure support for running on project-level information

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a reviewer: alexfh.
aaron.ballman added a comment.

Thanks, I think this is a neat idea! I haven't had the chance to review 
thoroughly yet, but wanted to point out that precommit CI is showing a fair 
amount of failures that I believe are related to your patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124447

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

[PATCH] D124726: Suggest typoed directives in preprocessor conditionals

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui updated this revision to Diff 427263.
ken-matsui added a comment.

Remove unnecessary includes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124726

Files:
  clang/include/clang/Basic/DiagnosticLexKinds.td
  clang/include/clang/Lex/Preprocessor.h
  clang/lib/Lex/PPDirectives.cpp
  clang/test/Preprocessor/suggest-typoed-directive.c
  llvm/include/llvm/ADT/StringRef.h
  llvm/unittests/ADT/StringRefTest.cpp

Index: llvm/unittests/ADT/StringRefTest.cpp
===
--- llvm/unittests/ADT/StringRefTest.cpp
+++ llvm/unittests/ADT/StringRefTest.cpp
@@ -566,6 +566,13 @@
 }
 
 TEST(StringRefTest, EditDistance) {
+  EXPECT_EQ(0U, StringRef("").edit_distance(""));
+  EXPECT_EQ(1U, StringRef("").edit_distance("aaab"));
+  EXPECT_EQ(2U, StringRef("aabc").edit_distance("aacb"));
+  EXPECT_EQ(2U, StringRef("aabc").edit_distance("abca"));
+  EXPECT_EQ(3U, StringRef("aabc").edit_distance("adef"));
+  EXPECT_EQ(4U, StringRef("abcd").edit_distance("efgh"));
+
   StringRef Hello("hello");
   EXPECT_EQ(2U, Hello.edit_distance("hill"));
 
@@ -584,6 +591,40 @@
"people soiled our green "));
 }
 
+TEST(StringRefTest, FindSimilarStr) {
+  {
+constexpr StringRef Candidates[] = {"aaab", "aaabc"};
+EXPECT_EQ(std::string("aaab"), StringRef("").find_similar_str(Candidates));
+  }
+  {
+constexpr StringRef Candidates[] = {"aab", "abc"};
+EXPECT_EQ(std::string("aab"), StringRef("aaa").find_similar_str(Candidates));
+  }
+  {
+constexpr StringRef Candidates[] = {"ab", "bc"};
+EXPECT_EQ(std::string("ab"), StringRef("aa").find_similar_str(Candidates));
+  }
+  {
+constexpr StringRef Candidates[] = {"b", "c"};
+EXPECT_EQ(None, StringRef("a").find_similar_str(Candidates));
+  }
+  { // macros
+constexpr StringRef Candidates[] = {
+"if", "ifdef", "ifndef", "elif", "elifdef", "elifndef", "else", "endif"
+};
+EXPECT_EQ(std::string("elifdef"), StringRef("elfidef").find_similar_str(Candidates));
+EXPECT_EQ(std::string("elifdef"), StringRef("elifdef").find_similar_str(Candidates));
+EXPECT_EQ(None, StringRef("special_compiler_directive").find_similar_str(Candidates));
+  }
+  { // case-insensitive
+constexpr StringRef Candidates[] = {
+"if", "ifdef", "ifndef", "elif", "elifdef", "elifndef", "else", "endif"
+};
+EXPECT_EQ(std::string("elifdef"), StringRef("elifdef").find_similar_str(Candidates));
+EXPECT_EQ(std::string("elifdef"), StringRef("ELIFDEF").find_similar_str(Candidates));
+  }
+}
+
 TEST(StringRefTest, Misc) {
   std::string Storage;
   raw_string_ostream OS(Storage);
Index: llvm/include/llvm/ADT/StringRef.h
===
--- llvm/include/llvm/ADT/StringRef.h
+++ llvm/include/llvm/ADT/StringRef.h
@@ -10,6 +10,7 @@
 #define LLVM_ADT_STRINGREF_H
 
 #include "llvm/ADT/DenseMapInfo.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/ADT/STLFunctionalExtras.h"
 #include "llvm/ADT/iterator_range.h"
 #include "llvm/Support/Compiler.h"
@@ -24,6 +25,7 @@
 #endif
 #include 
 #include 
+#include 
 
 // Declare the __builtin_strlen intrinsic for MSVC so it can be used in
 // constexpr context.
@@ -240,6 +242,46 @@
 unsigned edit_distance(StringRef Other, bool AllowReplacements = true,
unsigned MaxEditDistance = 0) const;
 
+/// Find a similar string in `Candidates`.
+template 
+Optional find_similar_str(const StringRef (&Candidates)[Size],
+   size_t Dist = 0) const {
+  // We need to check if `rng` has the exact case-insensitive string because the
+  // Levenshtein distance match does not care about it.
+  for (StringRef C : Candidates) {
+if (equals_insensitive(C)) {
+  return C.str();
+}
+  }
+
+  // Keep going with the Levenshtein distance match.
+  // If dist is given, use the dist for maxDist; otherwise, if word size is
+  // less than 3, use the word size minus 1 and if not, use the word size
+  // divided by 3.
+  size_t MaxDist = Dist != 0? Dist
+   : Length < 3 ? Length - 1
+: Length / 3;
+
+  std::vector> Cand;
+  for (StringRef C : Candidates) {
+size_t CurDist = edit_distance(C, false);
+if (CurDist <= MaxDist) {
+  Cand.emplace_back(C, CurDist);
+}
+  }
+
+  if (Cand.empty()) {
+return None;
+  } else if (Cand.size() == 1) {
+return Cand[0].first;
+  } else {
+auto SimilarStr = std::min_element(
+Cand.cbegin(), Cand.cend(),
+[](const auto &A, const auto &B) { return A.second < B.second; });
+return SimilarStr->first;
+  }
+}
+
 /// str - Get the contents as an std::string.
   

[PATCH] D124774: [clang][ASTImporter][NFC]: Move clang::ImportError into own header.

[Shivam Rajput via Phabricator via cfe-commits]

phyBrackets updated this revision to Diff 427264.
phyBrackets added a comment.

address the buildbot failing issue


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124774

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


Index: clang/include/clang/AST/ASTImportError.h
===
--- clang/include/clang/AST/ASTImportError.h
+++ clang/include/clang/AST/ASTImportError.h
@@ -1,4 +1,4 @@
-//===- ASTImportError.h - Define errors while importing AST -*- C++-*-===//
+//===- ASTImportError.h - Define errors while importing AST -*- C++ 
-*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.


Index: clang/include/clang/AST/ASTImportError.h
===
--- clang/include/clang/AST/ASTImportError.h
+++ clang/include/clang/AST/ASTImportError.h
@@ -1,4 +1,4 @@
-//===- ASTImportError.h - Define errors while importing AST -*- C++-*-===//
+//===- ASTImportError.h - Define errors while importing AST -*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D124687: [Tooling/DependencyScanning & Preprocessor] Refactor dependency scanning to record and use pre-lexed preprocessor directive tokens, instead of minimized sources

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added inline comments.



Comment at: clang/include/clang/Lex/PreprocessorOptions.h:213
+  std::function>(
+  FileEntryRef)>
+  DependencyDirectivesForFile;

You'll need to `#include "clang/Basic/FileEntry.h"`. Build with modules fails 
otherwise.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124687

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

[PATCH] D124690: [clangd] add inlay hints for std::forward-ed parameter packs

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clang-tools-extra/clangd/unittests/InlayHintTests.cpp:255
+  )cpp",
+   ExpectedHint{"a: ", "param"});
+}

upsj wrote:
> sammccall wrote:
> > upsj wrote:
> > > I haven't been able to figure out why, but for some reason the 
> > > `CXXConstructExpr` and `CXXTemporaryObjectExpr` are not being picked up 
> > > by the `RecursiveASTVisitor`, while in a similar situation below, the 
> > > `CallExpr` gets picked up by `VisitCallExpr`.
> > The AST here of the instantiated bar looks like
> > ```
> > -FunctionDecl  line:5:4 used bar 'S *(int &&)'
> >   |-TemplateArgument type 'S'
> >   |-TemplateArgument pack
> >   | `-TemplateArgument type 'int'
> >   |-ParmVarDecl  col:18 used args 'int &&'
> >   `-CompoundStmt 
> > `-ReturnStmt 
> >   `-CXXNewExpr  'S *' Function 'operator new' 'void 
> > *(unsigned long)'
> > `-CXXConstructExpr  'S':'S' 'void (int)' list
> >   `-ImplicitCastExpr  'int':'int' 
> > `-DeclRefExpr  'int':'int' lvalue ParmVar 'args' 'int 
> > &&'
> > ```
> > 
> > So there's no CXXTemporaryObjectExpr (the value here is a pointer), but 
> > should be a CXXConstructExpr. 
> > 
> > Are you sure you're traversing bar's instantiation/specializaion, as 
> > opposed to its templated/generic FunctionDecl? The AST of the templated 
> > FunctionDecl indeed has an InitListExpr in place of the CXXConstructExpr 
> > because it's not resolved yet.
> It's quite possible this is related to the visitor not/only inconsistently 
> traversing template instantiations due to shouldVisitTemplateInstantiations 
> returning false. But I need to implement the new approach first, anyways.
Ah, I think I see the confusion.

We don't want to gather all the information in a single traversal of the whole 
AST. Such a traversal would need to walk all instantiated templates including 
those from headers, which is far too much/too slow.

Instead, you want to analyze particular forwarding functions by walking through 
their bodies looking for the forwarding call. RecursiveASTVisitor is the tool 
for this job - you just call TraverseDecl(FunctionDecl) to limit the scope. The 
visitor should not visit template instantiations, but the traversal can still 
be rooted at an instantiation!

So something roughly like:

```
Optional> ultimateParams(FunctionDecl* Callee) {
  if (!Callee || !isa(Callee->getType()))
return None;
  if (not instantiated from template || Pattern doesn't use a pack)
return Callee->parameters();

  for (P : Pattern->parameters()) {
if (!P->isPack())
  Result.push_back(Callee->parameters[Pos++]);
else {
  if (PackExpansionSize = ...) {
if (ForwardedParams = forwardedParams(Callee, P))
  Result.append(*ForwardedParams);
else // failed, no param info for forwarded args
  Result.append(nullptr, PackExpansionSize);
Pos += PackExpansionSize;
  }
}
  }
}

Optional> forwardedParams(FunctionDecl *Callee, 
ParmVarDecl *Pack) {
  class ForwardFinder : RAV {
bool VisitCallExpr(CallExpr *E) {
  if (E->params() expands Pack) {
if (CallParams = ultimateParams(E->getCallee()->getAsFunctionDecl()))
  Result = CallParams.slice(section corresponding to Pack);
  }
}
  };
  ForwardFinder Finder;
  if (Callee->hasBody())
return Finder.VisitCallExpr(Callee);  
}
```

you could also memoize with a map, but I suspect simple recursion is going to 
be good enough


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124690

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

[PATCH] D124996: [clang][preprocessor] Fix unsigned-ness of utf8 char literals

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/test/Lexer/utf8-char-literal.cpp:16
 char f = u8'ab'; // expected-error {{Unicode character literals may not 
contain multiple characters}}
 #elif __STDC_VERSION__ > 202000L
 char a = u8'ñ';  // expected-error {{character too large for enclosing 
character literal type}}

I missed this one before. :-(



Comment at: clang/test/Lexer/utf8-char-literal.cpp:31
+/// Test u8 char literal preprocessor behavior
+#if __cplusplus > 201402L || __STDC_VERSION__ > 202000L
+#if u8'\xff' != 0xff

Uh oh.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124996

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

[PATCH] D124091: [clang][AArch64][SVE] Implement conditional operator for SVE vectors

[David Spickett via Phabricator via cfe-commits]

DavidSpickett added a comment.

I think that this has caused assert failures on the VLS (fixed sized vector) 
SVE bot.

https://lab.llvm.org/buildbot/#/builders/176/builds/1632/steps/12/logs/stdio

Based on the other SVE change being in llvm and seeing Sema in the backtrace.

These issues are not on any of the other AArch64 bots or the vector length 
agnostic bot. I appreciate that I'm pulling your change out of a list of 80 
without that much evidence so if it's clearly not the issue just let me know.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124091

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

[PATCH] D124996: [clang][preprocessor] Fix unsigned-ness of utf8 char literals

[Timm Bäder via Phabricator via cfe-commits]

tbaeder updated this revision to Diff 427267.
tbaeder marked 2 inline comments as done.

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

https://reviews.llvm.org/D124996

Files:
  clang/lib/Lex/PPExpressions.cpp
  clang/test/Lexer/utf8-char-literal.cpp


Index: clang/test/Lexer/utf8-char-literal.cpp
===
--- clang/test/Lexer/utf8-char-literal.cpp
+++ clang/test/Lexer/utf8-char-literal.cpp
@@ -13,7 +13,7 @@
 char d = u8'\u1234'; // expected-error {{character too large for enclosing 
character literal type}}
 char e = u8'ሴ'; // expected-error {{character too large for enclosing 
character literal type}}
 char f = u8'ab'; // expected-error {{Unicode character literals may not 
contain multiple characters}}
-#elif __STDC_VERSION__ > 202000L
+#elif __STDC_VERSION__ >= 202000L
 char a = u8'ñ';  // expected-error {{character too large for enclosing 
character literal type}}
 char b = u8'\x80';   // ok
 char c = u8'\u0080'; // expected-error {{universal character name refers to a 
control character}}
@@ -26,3 +26,10 @@
  unsigned char : 1),
 "Surprise!");
 #endif
+
+/// Test u8 char literal preprocessor behavior
+#if __cplusplus > 201402L || __STDC_VERSION__ >= 202000L
+#if u8'\xff' != 0xff
+#error u8 char literal is not unsigned
+#endif
+#endif
Index: clang/lib/Lex/PPExpressions.cpp
===
--- clang/lib/Lex/PPExpressions.cpp
+++ clang/lib/Lex/PPExpressions.cpp
@@ -407,10 +407,10 @@
 llvm::APSInt Val(NumBits);
 // Set the value.
 Val = Literal.getValue();
-// Set the signedness. UTF-16 and UTF-32 are always unsigned
+// Set the signedness. UTF-8, UTF-16 and UTF-32 are always unsigned
 if (Literal.isWide())
   Val.setIsUnsigned(!TargetInfo::isTypeSigned(TI.getWCharType()));
-else if (!Literal.isUTF16() && !Literal.isUTF32())
+else if (!Literal.isUTF8() && !Literal.isUTF16() && !Literal.isUTF32())
   Val.setIsUnsigned(!PP.getLangOpts().CharIsSigned);
 
 if (Result.Val.getBitWidth() > Val.getBitWidth()) {


Index: clang/test/Lexer/utf8-char-literal.cpp
===
--- clang/test/Lexer/utf8-char-literal.cpp
+++ clang/test/Lexer/utf8-char-literal.cpp
@@ -13,7 +13,7 @@
 char d = u8'\u1234'; // expected-error {{character too large for enclosing character literal type}}
 char e = u8'ሴ'; // expected-error {{character too large for enclosing character literal type}}
 char f = u8'ab'; // expected-error {{Unicode character literals may not contain multiple characters}}
-#elif __STDC_VERSION__ > 202000L
+#elif __STDC_VERSION__ >= 202000L
 char a = u8'ñ';  // expected-error {{character too large for enclosing character literal type}}
 char b = u8'\x80';   // ok
 char c = u8'\u0080'; // expected-error {{universal character name refers to a control character}}
@@ -26,3 +26,10 @@
  unsigned char : 1),
 "Surprise!");
 #endif
+
+/// Test u8 char literal preprocessor behavior
+#if __cplusplus > 201402L || __STDC_VERSION__ >= 202000L
+#if u8'\xff' != 0xff
+#error u8 char literal is not unsigned
+#endif
+#endif
Index: clang/lib/Lex/PPExpressions.cpp
===
--- clang/lib/Lex/PPExpressions.cpp
+++ clang/lib/Lex/PPExpressions.cpp
@@ -407,10 +407,10 @@
 llvm::APSInt Val(NumBits);
 // Set the value.
 Val = Literal.getValue();
-// Set the signedness. UTF-16 and UTF-32 are always unsigned
+// Set the signedness. UTF-8, UTF-16 and UTF-32 are always unsigned
 if (Literal.isWide())
   Val.setIsUnsigned(!TargetInfo::isTypeSigned(TI.getWCharType()));
-else if (!Literal.isUTF16() && !Literal.isUTF32())
+else if (!Literal.isUTF8() && !Literal.isUTF16() && !Literal.isUTF32())
   Val.setIsUnsigned(!PP.getLangOpts().CharIsSigned);
 
 if (Result.Val.getBitWidth() > Val.getBitWidth()) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D124715: Use QoS class Utility for ThreadPriority::Low on Mac

[Stefan Haller via Phabricator via cfe-commits]

stefanhaller added a comment.

In D124715#3491985 , @sammccall wrote:

> I'm still concerned some users will find this a large regression and we won't 
> have a good workaround:
>
> - it'll use a lot more battery than before

On Intel Macs, I'm not sure that's true. While it does saturate the CPUs 
noticeably more with Utility than it does with Background, it will also be 
finished much quicker, so I guess the total power consumption will probably be 
roughly the same.

On M1  Macs, it's true that it will use a lot more 
battery. However, M1  Macs are also much more 
energy efficient in general, so it doesn't matter nearly as much. I don't think 
it will be perceived as a regression there.

(On the contrary, people who switch from Intel to M1 
 Macs perceive it as a regression that indexing is 
so slow, even though the machine should be so much faster. I have plenty of 
colleagues who are in that situation, and have heard this complaint several 
times.)

> What do you think about keeping `Background` in the enum along with `Low`, 
> and switching clangd to `Low`?

Personally I don't feel this is necessary, but I can of course change the patch 
if there's agreement that that's what we should do.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124715

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

[PATCH] D124966: Thread safety analysis: Handle compound assignment and ->* overloads

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/Analysis/ThreadSafety.cpp:1994
+  case OO_Equal:
+  case OO_PlusEqual:
+  case OO_MinusEqual:

If we're going to be handling these, should we also be handling overloads of 
`++` and `--`?



Comment at: clang/test/SemaCXX/warn-thread-safety-analysis.cpp:4344
 
+  Data& operator+=(int);
+

We should probably add test coverage for all the newly supported operators.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124966

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

[PATCH] D124221: Reimplement `__builtin_dump_struct` in Sema.

[Aaron Ballman via Phabricator via cfe-commits]

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

Aside from a minor nit, I think this LGTM. Thank you for working on this!




Comment at: clang/lib/Sema/SemaChecking.cpp:476
+analyze_printf::PrintfSpecifier Specifier;
+if (Specifier.fixType(T, S.getLangOpts(), S.Context, 
/*IsObjCLiteral=*/false)) {
+  // We were able to guess how to format this.

Note for future reference: I don't think `fixType()` does anything special to 
help us out with arrays.



Comment at: clang/lib/Sema/SemaChecking.cpp:507
+   unsigned Depth) {
+// FIXME: Decide what to do if RD is a union. At least we should probably
+// turn off printing `const char*` members with `%s`, because that is very

My suggestion is to print the value of all fields and treat any string-like 
types as `%p` rather than `%s`. I'm fine with this being done later.



Comment at: clang/lib/Sema/SemaChecking.cpp:542
+  auto *FD = IFD ? IFD->getAnonField() : dyn_cast(D);
+  if (!FD || (FD->isUnnamedBitfield() || FD->isAnonymousStructOrUnion()))
+continue;





Comment at: clang/lib/Sema/SemaChecking.cpp:433
+
+  llvm::StringRef getFormatSpecifier(QualType T) {
+if (auto *BT = T->getAs()) {

yihanaa wrote:
> yihanaa wrote:
> > erichkeane wrote:
> > > rsmith wrote:
> > > > yihanaa wrote:
> > > > > rsmith wrote:
> > > > > > rsmith wrote:
> > > > > > > aaron.ballman wrote:
> > > > > > > > yihanaa wrote:
> > > > > > > > > I think this is better maintained in 
> > > > > > > > > "clang/AST/FormatString.h". For example 
> > > > > > > > > analyze_printf::PrintfSpecifier can get format specifier, 
> > > > > > > > > what do you all think about?
> > > > > > > > +1 to this suggestion -- my hope is that we could generalize it 
> > > > > > > > more then as I notice there are missing specifiers for things 
> > > > > > > > like intmax_t, size_t, ptrdiff_t, _Decimal types, etc. Plus, 
> > > > > > > > that will hopefully make it easier for __builtin_dump_struct to 
> > > > > > > > benefit when new format specifiers are added, such as ones for 
> > > > > > > > printing a _BitInt.
> > > > > > > I am somewhat uncertain: every one of these is making arbitrary 
> > > > > > > choices about how to format the value, so it's not clear to me 
> > > > > > > that this is general logic rather than something specific to 
> > > > > > > `__builtin_dump_struct`. For example, using `%f` rather than one 
> > > > > > > of the myriad other ways of formatting `double` is somewhat 
> > > > > > > arbitrary. Using `%s` for any `const char*` is *extremely* 
> > > > > > > arbitrary and will be wrong and will cause crashes in some cases, 
> > > > > > > but it may be the pragmatically correct choice for a dumping 
> > > > > > > utility. A general-purpose mechanism would use `%p` for all kinds 
> > > > > > > of pointer.
> > > > > > > 
> > > > > > > We could perhaps factor out the formatting for cases where there 
> > > > > > > is a clear canonical default formatting, such as for integer 
> > > > > > > types and probably `%p` for pointers, then call that from here 
> > > > > > > with some special-casing, but without a second consumer for that 
> > > > > > > functionality it's really not clear to me what form it should 
> > > > > > > take.
> > > > > > I went ahead and did this, mostly to match concurrent changes to 
> > > > > > the old implementation. There are a few cases where our existing 
> > > > > > "guess a format specifier" logic does the wrong thing for dumping 
> > > > > > purposes, which I've explicitly handled -- things like wanting to 
> > > > > > dump a `char` / `signed char` / `unsigned char` member as a number 
> > > > > > rather than as a (potentially non-printable or whitespace) 
> > > > > > character.
> > > > >  When I was patching that old implementation, I found that for 
> > > > > uint8_t, int8_t, Clang's existing "guess a format specifier" logic 
> > > > > would treat the value as an integer, but for unsigned char, signed 
> > > > > char, char types, it would Treat it as a character, please look at 
> > > > > this example ( https://godbolt.org/z/ooqn4468T ), I guess this 
> > > > > existing logic may have made some special judgment.
> > > > Yeah. I think in the case where we see some random call to `printf`, 
> > > > `%c` is probably the right thing to guess here, but it doesn't seem 
> > > > appropriate to me to use this in a dumping routine. If we could dump as 
> > > > `'x'` for printable characters and as `'\xAB'` for everything else, 
> > > > that'd be great, but `printf` can't do that itself and I'm not sure we 
> > > > want to be injecting calls to `isprint` or whatever to make that work. 
> > > > Dumping as an integer always seems like it's probably the least-bad 
> > > > option.
> > > > 
> > > > Somewhat related: I wonder if 

[PATCH] D124996: [clang][preprocessor] Fix unsigned-ness of utf8 char literals

[Aaron Ballman via Phabricator via cfe-commits]

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

Presuming precommit CI comes back green, this LGTM! Can you also add a release 
note for the bug fix when landing, please?


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

https://reviews.llvm.org/D124996

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

[PATCH] D124658: [analyzer] Canonicalize SymIntExpr so the RHS is positive when possible

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

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

Seems right. Thanks.


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

https://reviews.llvm.org/D124658

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

[PATCH] D124850: [Sema][SVE2] Move/simplify Sema testing for SVE2 ACLE builtins

[Rosie Sumpter via Phabricator via cfe-commits]

RosieSumpter updated this revision to Diff 427275.
RosieSumpter marked 3 inline comments as done.
RosieSumpter added a comment.

- Changed operand names to be more descriptive
- Made int/uint/float variables global
- Moved bfloat tests into a separate file


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

https://reviews.llvm.org/D124850

Files:
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_aba.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_abalb.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_abalt.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_abdlb.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_abdlt.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_adalp.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_adclb.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_adclt.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_addhnb.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_addhnt.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_addlb.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_addlbt.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_addlt.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_addp.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_addwb.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_addwt.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_aesd.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_aese.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_aesimc.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_aesmc.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_bcax.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_bdep.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_bext.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_bgrp.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_bsl.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_bsl1n.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_bsl2n.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_cadd.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_cdot.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_cmla.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_cvtlt.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_cvtnt.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_cvtx.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_cvtxnt.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_eor3.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_eorbt.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_eortb.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_hadd.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_histcnt.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_histseg.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_hsub.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_hsubr.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_ldnt1.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_ldnt1sb.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_ldnt1sh.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_ldnt1sw.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_ldnt1ub.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_ldnt1uh.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_ldnt1uw.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_logb.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_match.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_maxnmp.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_maxp.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_minnmp.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_minp.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_mla.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_mlalb.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_mlalt.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_mls.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_mlslb.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_mlslt.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_movlb.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_movlt.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_mul.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_mullb.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_mullt.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_nbsl.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_nmatch.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_pmul.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_pmullb.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_pmullb_128.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_pmullt.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_pmullt_128.c
  clang/test/CodeGen/aarch

[PATCH] D124658: [analyzer] Canonicalize SymIntExpr so the RHS is positive when possible

[Tomasz Kamiński via Phabricator via cfe-commits]

tomasz-kaminski-sonarsource added a comment.

I do not have commit rights. Would it be possible to you to commit the changes 
on my behalf?


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

https://reviews.llvm.org/D124658

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

[PATCH] D124983: [HLSL} add -fcgl option flag.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/include/clang/Driver/Options.td:6765
  "as_6_5, as_6_6, as_6_7">;
+def fcgl : DXCFlag<"fcgl">,
+  HelpText<"Generate high-level code only. Without any dxil related passes.">;

Any particular reason for this name? Naively, I have no idea what this does or 
how it relates to code generation.



Comment at: clang/include/clang/Driver/Options.td:6766
+def fcgl : DXCFlag<"fcgl">,
+  HelpText<"Generate high-level code only. Without any dxil related passes.">;





Comment at: clang/unittests/Driver/ToolChainTest.cpp:613
 
+TEST(DxcModeTest, ValidatorFCGL) {
+  IntrusiveRefCntPtr DiagID(new DiagnosticIDs());

Any reason not to use the typical lit tests for this? It seems like you should 
be able to add a Driver test that does `-###` and looks for the expected 
pass-down flags. And then you can add a -cc1 test to ensure that the `-fcgl` 
flag isn't supported there.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124983

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

[PATCH] D124996: [clang][preprocessor] Fix unsigned-ness of utf8 char literals

[Timm Bäder via Phabricator via cfe-commits]

tbaeder updated this revision to Diff 427283.

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

https://reviews.llvm.org/D124996

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Lex/PPExpressions.cpp
  clang/test/Lexer/utf8-char-literal.cpp


Index: clang/test/Lexer/utf8-char-literal.cpp
===
--- clang/test/Lexer/utf8-char-literal.cpp
+++ clang/test/Lexer/utf8-char-literal.cpp
@@ -13,7 +13,7 @@
 char d = u8'\u1234'; // expected-error {{character too large for enclosing 
character literal type}}
 char e = u8'ሴ'; // expected-error {{character too large for enclosing 
character literal type}}
 char f = u8'ab'; // expected-error {{Unicode character literals may not 
contain multiple characters}}
-#elif __STDC_VERSION__ > 202000L
+#elif __STDC_VERSION__ >= 202000L
 char a = u8'ñ';  // expected-error {{character too large for enclosing 
character literal type}}
 char b = u8'\x80';   // ok
 char c = u8'\u0080'; // expected-error {{universal character name refers to a 
control character}}
@@ -26,3 +26,10 @@
  unsigned char : 1),
 "Surprise!");
 #endif
+
+/// Test u8 char literal preprocessor behavior
+#if __cplusplus > 201402L || __STDC_VERSION__ >= 202000L
+#if u8'\xff' != 0xff
+#error u8 char literal is not unsigned
+#endif
+#endif
Index: clang/lib/Lex/PPExpressions.cpp
===
--- clang/lib/Lex/PPExpressions.cpp
+++ clang/lib/Lex/PPExpressions.cpp
@@ -407,10 +407,10 @@
 llvm::APSInt Val(NumBits);
 // Set the value.
 Val = Literal.getValue();
-// Set the signedness. UTF-16 and UTF-32 are always unsigned
+// Set the signedness. UTF-8, UTF-16 and UTF-32 are always unsigned
 if (Literal.isWide())
   Val.setIsUnsigned(!TargetInfo::isTypeSigned(TI.getWCharType()));
-else if (!Literal.isUTF16() && !Literal.isUTF32())
+else if (!Literal.isUTF8() && !Literal.isUTF16() && !Literal.isUTF32())
   Val.setIsUnsigned(!PP.getLangOpts().CharIsSigned);
 
 if (Result.Val.getBitWidth() > Val.getBitWidth()) {
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -290,6 +290,8 @@
   template parameter, to conform to the Itanium C++ ABI and be compatible with
   GCC. This breaks binary compatibility with code compiled with earlier 
versions
   of clang; use the ``-fclang-abi-compat=14`` option to get the old mangling.
+- Preprocessor character literals with a ``u8`` are now correctly treated as
+  unsigned character literals. This fixes `Issue 54886 
`_.
 
 C++20 Feature Support
 ^


Index: clang/test/Lexer/utf8-char-literal.cpp
===
--- clang/test/Lexer/utf8-char-literal.cpp
+++ clang/test/Lexer/utf8-char-literal.cpp
@@ -13,7 +13,7 @@
 char d = u8'\u1234'; // expected-error {{character too large for enclosing character literal type}}
 char e = u8'ሴ'; // expected-error {{character too large for enclosing character literal type}}
 char f = u8'ab'; // expected-error {{Unicode character literals may not contain multiple characters}}
-#elif __STDC_VERSION__ > 202000L
+#elif __STDC_VERSION__ >= 202000L
 char a = u8'ñ';  // expected-error {{character too large for enclosing character literal type}}
 char b = u8'\x80';   // ok
 char c = u8'\u0080'; // expected-error {{universal character name refers to a control character}}
@@ -26,3 +26,10 @@
  unsigned char : 1),
 "Surprise!");
 #endif
+
+/// Test u8 char literal preprocessor behavior
+#if __cplusplus > 201402L || __STDC_VERSION__ >= 202000L
+#if u8'\xff' != 0xff
+#error u8 char literal is not unsigned
+#endif
+#endif
Index: clang/lib/Lex/PPExpressions.cpp
===
--- clang/lib/Lex/PPExpressions.cpp
+++ clang/lib/Lex/PPExpressions.cpp
@@ -407,10 +407,10 @@
 llvm::APSInt Val(NumBits);
 // Set the value.
 Val = Literal.getValue();
-// Set the signedness. UTF-16 and UTF-32 are always unsigned
+// Set the signedness. UTF-8, UTF-16 and UTF-32 are always unsigned
 if (Literal.isWide())
   Val.setIsUnsigned(!TargetInfo::isTypeSigned(TI.getWCharType()));
-else if (!Literal.isUTF16() && !Literal.isUTF32())
+else if (!Literal.isUTF8() && !Literal.isUTF16() && !Literal.isUTF32())
   Val.setIsUnsigned(!PP.getLangOpts().CharIsSigned);
 
 if (Result.Val.getBitWidth() > Val.getBitWidth()) {
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -290,6 +290,8 @@
   template parameter, to conform to the Itanium C++ ABI and be compatible with
   GCC. This breaks binary compatibility with code compiled with earlier versions

[PATCH] D120290: [Clang][OpenMP] Add the codegen support for `atomic compare capture`

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:6168
   llvm::Value *EVal = CGF.EmitScalarExpr(E);
+  if (auto CI = dyn_cast(EVal))
+EVal = CGF.Builder.CreateIntCast(

1. `auto *CI`
2. What if this is not a constant, but just a value with int type? Is this 
possible?



Comment at: clang/lib/Sema/SemaOpenMP.cpp:11796
   X = ThenBO->getLHS();
-  D = ThenBO->getRHS();
+  D = ThenBO->getRHS()->IgnoreImpCasts();
 

Same question as before. It is assignment BO, do you really need to remove 
casts here?



Comment at: clang/lib/Sema/SemaOpenMP.cpp:11810
   if (checkIfTwoExprsAreSame(ContextRef, X, BO->getLHS())) {
-E = BO->getRHS();
+E = BO->getRHS()->IgnoreImpCasts();
   } else if (checkIfTwoExprsAreSame(ContextRef, X, BO->getRHS())) {

Maybe, IgnoreImplicitAsWritten?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120290

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

[PATCH] D111548: [Clang] Add the `annotate_type` attribute

[Martin Böhme via Phabricator via cfe-commits]

mboehme added inline comments.



Comment at: clang/test/SemaCXX/annotate-type.cpp:2
+// RUN: %clang_cc1 %s -std=c++17 -fsyntax-only -verify
+
+struct S1 {

aaron.ballman wrote:
> mboehme wrote:
> > aaron.ballman wrote:
> > > mboehme wrote:
> > > > aaron.ballman wrote:
> > > > > mboehme wrote:
> > > > > > aaron.ballman wrote:
> > > > > > > mboehme wrote:
> > > > > > > > aaron.ballman wrote:
> > > > > > > > > mboehme wrote:
> > > > > > > > > > aaron.ballman wrote:
> > > > > > > > > > > Can you also add some test coverage for applying the 
> > > > > > > > > > > attribute to a declaration instead of a type or not 
> > > > > > > > > > > giving it any arguments? Also, should test arguments 
> > > > > > > > > > > which are not a constant expression.
> > > > > > > > > > I've added tests as you suggested, though I put most of 
> > > > > > > > > > them in Sema/annotate-type.c, as they're not specific to 
> > > > > > > > > > C++.
> > > > > > > > > > 
> > > > > > > > > > Thanks for prompting me to do this -- the tests caused me 
> > > > > > > > > > to notice and fix a number of bugs.
> > > > > > > > > > 
> > > > > > > > > > I haven't managed to diagnose the case when the attribute 
> > > > > > > > > > appears at the beginning of the declaration. It seems to me 
> > > > > > > > > > that, at the point where I've added the check, this case is 
> > > > > > > > > > indistinguishable from an attribute that appears on the 
> > > > > > > > > > type. In both cases, the `TAL` is `TAL_DeclSpec`, and the 
> > > > > > > > > > attribute is contained in `DeclSpec::getAttributes()`. This 
> > > > > > > > > > is because `Parser::ParseSimpleDeclaration` forwards the 
> > > > > > > > > > declaration attributes to the `DeclSpec`:
> > > > > > > > > > 
> > > > > > > > > > https://github.com/llvm/llvm-project/blob/main/clang/lib/Parse/ParseDecl.cpp#L1851
> > > > > > > > > > 
> > > > > > > > > > I believe if I wanted to prohibit this case, I would need 
> > > > > > > > > > to add a check to `Parser::ParseStatementOrDeclaration` and 
> > > > > > > > > > prohibit any occurrences of `annotate_type` there. However, 
> > > > > > > > > > this seems the wrong place to do this because it doesn't 
> > > > > > > > > > contain any specific processing for other attributes.
> > > > > > > > > > 
> > > > > > > > > > I've noticed that Clang also doesn't prohibit other type 
> > > > > > > > > > attributes (even ones with C++ 11 syntax) from being 
> > > > > > > > > > applied to declarations. For example: 
> > > > > > > > > > https://godbolt.org/z/Yj1zWY7nn
> > > > > > > > > > 
> > > > > > > > > > How do you think I should proceed here? I think the 
> > > > > > > > > > underlying issue is that Clang doesn't always distinguish 
> > > > > > > > > > cleanly between declaration attributes and type attributes, 
> > > > > > > > > > and fixing this might be nontrivial.
> > > > > > > > > > How do you think I should proceed here? I think the 
> > > > > > > > > > underlying issue is that Clang doesn't always distinguish 
> > > > > > > > > > cleanly between declaration attributes and type attributes, 
> > > > > > > > > > and fixing this might be nontrivial.
> > > > > > > > > 
> > > > > > > > > This is a general issue with attribute processing. I would 
> > > > > > > > > imagine that SemaDeclAttr.cpp should be able to diagnose that 
> > > > > > > > > case when the attribute only applies to types and not 
> > > > > > > > > declarations, but it'd take some investigation for me to be 
> > > > > > > > > sure.
> > > > > > > > > 
> > > > > > > > > Because this issue isn't new to your situation, I'd recommend 
> > > > > > > > > filing an issue about the general problem and then we can 
> > > > > > > > > solve that later.
> > > > > > > > I've done some more investigation myself, and I think I've come 
> > > > > > > > up with a solution; actually, two potential solutions. I have 
> > > > > > > > draft patches for both of these; I wanted to run these by you 
> > > > > > > > here first, so I haven't opened a bug yet.
> > > > > > > > 
> > > > > > > > I'd appreciate your input on how you'd prefer me to proceed 
> > > > > > > > with this. I do think it makes sense to do this work now 
> > > > > > > > because otherwise, people will start putting `annotate_type` in 
> > > > > > > > places where it doesn't belong, and then we'll have to keep 
> > > > > > > > supporting that in the future.
> > > > > > > > 
> > > > > > > > **My preferred solution**
> > > > > > > > 
> > > > > > > > https://reviews.llvm.org/D124081
> > > > > > > > 
> > > > > > > > This adds a function `DiagnoseCXX11NonDeclAttrs()` and calls it 
> > > > > > > > in all places where we should reject attributes that can't be 
> > > > > > > > put on declarations. (As part of this work, I noticed that 
> > > > > > > > `gsl::suppress` should be a `DeclOrStmtAttr`, not just a 
> > > > > > > > `StmtAttr`.)
> > > > > > > > 
> > > > > > > > Advantages:
> > > > > > > > - Not very invasive.
> > > > > > > 

[PATCH] D125006: [pseudo] Support parsing variant target symbols.

[Haojian Wu via Phabricator via cfe-commits]

hokein created this revision.
hokein added a reviewer: sammccall.
Herald added a subscriber: mgrang.
Herald added a project: All.
hokein requested review of this revision.
Herald added a subscriber: alextsao1999.
Herald added a project: clang-tools-extra.

With this patch, we're able to parse smaller chunks of C++ code (statement,
declaration), rather than translation-unit.

The target symbol is listed in the grammar in a form of `_ :=
statement`, each target symbol has a dedicated state (`_ := • statement`).
We create and track all these separate states in the LRTable. When we
start parsing, we lookup the corresponding state to start the parser.

LR pasing table changes with this patch:

- number of states: 1467 -> 1471
- number of actions: 82891 -> 83578
- size of the table (bytes): 334248 -> 336996


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125006

Files:
  clang-tools-extra/pseudo/include/clang-pseudo/GLR.h
  clang-tools-extra/pseudo/include/clang-pseudo/LRGraph.h
  clang-tools-extra/pseudo/include/clang-pseudo/LRTable.h
  clang-tools-extra/pseudo/lib/GLR.cpp
  clang-tools-extra/pseudo/lib/LRGraph.cpp
  clang-tools-extra/pseudo/lib/LRTable.cpp
  clang-tools-extra/pseudo/lib/LRTableBuild.cpp
  clang-tools-extra/pseudo/lib/cxx.bnf
  clang-tools-extra/pseudo/test/glr-variant-start.cpp
  clang-tools-extra/pseudo/tool/ClangPseudo.cpp
  clang-tools-extra/pseudo/unittests/GLRTest.cpp

Index: clang-tools-extra/pseudo/unittests/GLRTest.cpp
===
--- clang-tools-extra/pseudo/unittests/GLRTest.cpp
+++ clang-tools-extra/pseudo/unittests/GLRTest.cpp
@@ -344,7 +344,8 @@
   const TokenStream &Tokens = cook(lex("{ abc", LOptions), LOptions);
   auto LRTable = LRTable::buildSLR(*G);
 
-  const ForestNode &Parsed = glrParse(Tokens, {*G, LRTable, Arena, GSStack});
+  const ForestNode &Parsed =
+  glrParse(Tokens, {*G, LRTable, Arena, GSStack}, id("test"));
   // Verify that there is no duplicated sequence node of `expr := IDENTIFIER`
   // in the forest, see the `#1` and `=#1` in the dump string.
   EXPECT_EQ(Parsed.dumpRecursive(*G),
@@ -381,7 +382,8 @@
   const TokenStream &Tokens = cook(lex("IDENTIFIER", LOptions), LOptions);
   auto LRTable = LRTable::buildSLR(*G);
 
-  const ForestNode &Parsed = glrParse(Tokens, {*G, LRTable, Arena, GSStack});
+  const ForestNode &Parsed =
+  glrParse(Tokens, {*G, LRTable, Arena, GSStack}, id("test"));
   EXPECT_EQ(Parsed.dumpRecursive(*G),
 "[  0, end) test := \n"
 "[  0, end) ├─test := IDENTIFIER\n"
Index: clang-tools-extra/pseudo/tool/ClangPseudo.cpp
===
--- clang-tools-extra/pseudo/tool/ClangPseudo.cpp
+++ clang-tools-extra/pseudo/tool/ClangPseudo.cpp
@@ -38,6 +38,9 @@
   desc("Print directive structure of source code"));
 static opt PrintStatistics("print-statistics", desc("Print GLR parser statistics"));
 static opt PrintForest("print-forest", desc("Print parse forest"));
+static opt ParseSymbol("parse-symbol",
+desc("specify the target symbol to parse"),
+init("translation-unit"));
 
 static std::string readOrDie(llvm::StringRef Path) {
   llvm::ErrorOr> Text =
@@ -50,6 +53,14 @@
   return Text.get()->getBuffer().str();
 }
 
+static clang::pseudo::SymbolID
+findNonterminalID(llvm::StringRef Name, const clang::pseudo::Grammar &G) {
+  for (clang::pseudo::SymbolID ID = 0; ID < G.table().Nonterminals.size(); ++ID)
+if (G.table().Nonterminals[ID].Name == Name)
+  return ID;
+  return 0;
+}
+
 int main(int argc, char *argv[]) {
   llvm::cl::ParseCommandLineOptions(argc, argv, "");
 
@@ -96,9 +107,9 @@
 if (ParseableStream) {
   clang::pseudo::ForestArena Arena;
   clang::pseudo::GSS GSS;
-  auto &Root =
-  glrParse(*ParseableStream,
-   clang::pseudo::ParseParams{*G, LRTable, Arena, GSS});
+  auto &Root = glrParse(*ParseableStream,
+clang::pseudo::ParseParams{*G, LRTable, Arena, GSS},
+findNonterminalID(ParseSymbol, *G));
   if (PrintForest)
 llvm::outs() << Root.dumpRecursive(*G, /*Abbreviated=*/true);
 
Index: clang-tools-extra/pseudo/test/glr-variant-start.cpp
===
--- /dev/null
+++ clang-tools-extra/pseudo/test/glr-variant-start.cpp
@@ -0,0 +1,9 @@
+// RUN: clang-pseudo -grammar=%cxx-bnf-file -source=%s --parse-symbol=statement-seq --print-forest | FileCheck %s
+
+a + a;
+// CHECK:  statement-seq~expression-statement := expression ;
+// CHECK-NEXT: ├─expression~additive-expression := additive-expression + multiplicative-expression
+// CHECK-NEXT: │ ├─additive-expression~IDENTIFIER :=
+// CHECK-NEXT: │ ├─+ :=
+// CHECK-NEXT: │ └─multiplicative-expression~IDENTIFIER :=
+// CHECK-NEXT: └─; :=
Index

[PATCH] D124971: [Frontend] give createInvocationFromCommandLine an options struct

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

Thanks this LGTM. Just two questions:

- Why not migrate rest of the usages and drop the old function completely in 
this patch
- What about moving the function from clang/Frontend to Tooling, Support or 
FrontendTool? this is clearly only inteded to be used by tools and not part of 
the Frontend. There's actually Tooling/Tooling.h which has a `newInvocation` 
that takes in cc1 args, rather than driver args. so these two seem like 
siblings.




Comment at: clang/include/clang/Frontend/Utils.h:232
+
+/// Deprecated version of createInvocation with individual optional args.
 std::unique_ptr createInvocationFromCommandLine(

what's the reason for not deleting this all together from open source. there 
isn't very many references.
is it just to keep the patch small so that relevant changes are more visible ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124971

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

[PATCH] D111548: [Clang] Add the `annotate_type` attribute

[Martin Böhme via Phabricator via cfe-commits]

mboehme added a comment.

In D111548#3483326 , @xbolva00 wrote:

> Please answer points in “ Contributing Extensions to Clang “
> https://clang.llvm.org/get_involved.html

I've added these to the RFC:

https://discourse.llvm.org/t/rfc-new-attribute-annotate-type-iteration-2/61378/9?u=martinboehme


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111548

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

[PATCH] D111548: [Clang] Add the `annotate_type` attribute

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D111548#3483326 , @xbolva00 wrote:

>>> The intent is to allow adding arbitrary annotations to types for use in 
>>> static analysis tools
>
> This patch should not land until we see some real use cases to justify new 
> hundreds of lines of code. We should more careful and take maintenance cost 
> really into account and not merge every experimental cool research extensions 
> automatically.
>
> Please answer points in “ Contributing Extensions to Clang “
> https://clang.llvm.org/get_involved.html

Sorry, I had missed this. The justification is from the original RFC on adding 
lifetime annotation checking to the clang static analyzer 
(https://discourse.llvm.org/t/rfc-new-attribute-annotate-type-iteration-2). I 
think this code has utility as a general-purpose mechanism in the same manner 
that the existing declaration annotation attribute does. As attribute code 
owner, I think this meets the bar for a contribution as an extension.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111548

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

[PATCH] D120290: [Clang][OpenMP] Add the codegen support for `atomic compare capture`

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 added inline comments.



Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:6168
   llvm::Value *EVal = CGF.EmitScalarExpr(E);
+  if (auto CI = dyn_cast(EVal))
+EVal = CGF.Builder.CreateIntCast(

ABataev wrote:
> 1. `auto *CI`
> 2. What if this is not a constant, but just a value with int type? Is this 
> possible?
> What if this is not a constant, but just a value with int type? Is this 
> possible?
I'm thinking of a similar case.
```
void foo() {
  int e, d;
  char x, v;
  if (x == e)
x = d;
}
```
In this case, there are two casts:
1. In expression `x == e`, cast `x` to `int`.
2. In expression `x = d`, cast `d` to `char`.
We cannot ignore both of them. For the 2nd, no question. For the expression `x 
== e`, the problem is in `e` instead of `x`. `e` here is always `int`, further 
causing issue in codegen.

I'm thinking for those lvalues, we can only set them where they are used as 
lvalues in case any type lifting. For those rvalues, if their types are not 
same as `x`, but compatible with `x`, we have to insert cast, including 
truncate, before feed them into codegen. Does it sound good?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120290

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

[PATCH] D124669: [flang][driver] Add support for -save-temps

[Diana Picus via Phabricator via cfe-commits]

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

LGTM, thanks! It would be nice to rebase the patch and see the pre-commit CI 
passing, but then again you're the one dealing with the buildbots if you break 
anything, so do as you prefer :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124669

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

[PATCH] D124998: [AArch64][SVE] Add aarch64_sve_pcs attribute to Clang

[Peter Waller via Phabricator via cfe-commits]

peterwaller-arm accepted this revision.
peterwaller-arm added a comment.
This revision is now accepted and ready to land.

Looks good to me with minor nits.




Comment at: clang/include/clang-c/Index.h:3448
+  CXCallingConv_AArch64SVEPcs= 17,
+  CXCallingConv_SwiftAsync = 18,
 

It shouldn't matter in principle (... "but in practice" ...) we should probably 
avoid renumbering existing things in the enum and instead add to the end of it.

Nit, this is missing a space before the equals.
Nit, SVE is an acronym, so is PCS, so capitalization should be consistent 
between the two. I see 'PCS' capitalized in AAPCS for example so probably all 
upper case makes the sense.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124998

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

[PATCH] D124996: [clang][preprocessor] Fix unsigned-ness of utf8 char literals

[Timm Bäder via Phabricator via cfe-commits]

tbaeder updated this revision to Diff 427289.

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

https://reviews.llvm.org/D124996

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Lex/PPExpressions.cpp
  clang/test/Lexer/utf8-char-literal.cpp


Index: clang/test/Lexer/utf8-char-literal.cpp
===
--- clang/test/Lexer/utf8-char-literal.cpp
+++ clang/test/Lexer/utf8-char-literal.cpp
@@ -13,7 +13,7 @@
 char d = u8'\u1234'; // expected-error {{character too large for enclosing 
character literal type}}
 char e = u8'ሴ'; // expected-error {{character too large for enclosing 
character literal type}}
 char f = u8'ab'; // expected-error {{Unicode character literals may not 
contain multiple characters}}
-#elif __STDC_VERSION__ > 202000L
+#elif __STDC_VERSION__ >= 202000L
 char a = u8'ñ';  // expected-error {{character too large for enclosing 
character literal type}}
 char b = u8'\x80';   // ok
 char c = u8'\u0080'; // expected-error {{universal character name refers to a 
control character}}
@@ -26,3 +26,10 @@
  unsigned char : 1),
 "Surprise!");
 #endif
+
+/// Test u8 char literal preprocessor behavior
+#if __cplusplus > 201402L || __STDC_VERSION__ >= 202000L
+#if u8'\xff' != 0xff
+#error u8 char literal is not unsigned
+#endif
+#endif
Index: clang/lib/Lex/PPExpressions.cpp
===
--- clang/lib/Lex/PPExpressions.cpp
+++ clang/lib/Lex/PPExpressions.cpp
@@ -407,10 +407,10 @@
 llvm::APSInt Val(NumBits);
 // Set the value.
 Val = Literal.getValue();
-// Set the signedness. UTF-16 and UTF-32 are always unsigned
+// Set the signedness. UTF-8, UTF-16 and UTF-32 are always unsigned
 if (Literal.isWide())
   Val.setIsUnsigned(!TargetInfo::isTypeSigned(TI.getWCharType()));
-else if (!Literal.isUTF16() && !Literal.isUTF32())
+else if (!Literal.isUTF8() && !Literal.isUTF16() && !Literal.isUTF32())
   Val.setIsUnsigned(!PP.getLangOpts().CharIsSigned);
 
 if (Result.Val.getBitWidth() > Val.getBitWidth()) {
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -290,6 +290,8 @@
   template parameter, to conform to the Itanium C++ ABI and be compatible with
   GCC. This breaks binary compatibility with code compiled with earlier 
versions
   of clang; use the ``-fclang-abi-compat=14`` option to get the old mangling.
+- Preprocessor character literals with a ``u8`` prefix are now correctly 
treated as
+  unsigned character literals. This fixes `Issue 54886 
`_.
 
 C++20 Feature Support
 ^


Index: clang/test/Lexer/utf8-char-literal.cpp
===
--- clang/test/Lexer/utf8-char-literal.cpp
+++ clang/test/Lexer/utf8-char-literal.cpp
@@ -13,7 +13,7 @@
 char d = u8'\u1234'; // expected-error {{character too large for enclosing character literal type}}
 char e = u8'ሴ'; // expected-error {{character too large for enclosing character literal type}}
 char f = u8'ab'; // expected-error {{Unicode character literals may not contain multiple characters}}
-#elif __STDC_VERSION__ > 202000L
+#elif __STDC_VERSION__ >= 202000L
 char a = u8'ñ';  // expected-error {{character too large for enclosing character literal type}}
 char b = u8'\x80';   // ok
 char c = u8'\u0080'; // expected-error {{universal character name refers to a control character}}
@@ -26,3 +26,10 @@
  unsigned char : 1),
 "Surprise!");
 #endif
+
+/// Test u8 char literal preprocessor behavior
+#if __cplusplus > 201402L || __STDC_VERSION__ >= 202000L
+#if u8'\xff' != 0xff
+#error u8 char literal is not unsigned
+#endif
+#endif
Index: clang/lib/Lex/PPExpressions.cpp
===
--- clang/lib/Lex/PPExpressions.cpp
+++ clang/lib/Lex/PPExpressions.cpp
@@ -407,10 +407,10 @@
 llvm::APSInt Val(NumBits);
 // Set the value.
 Val = Literal.getValue();
-// Set the signedness. UTF-16 and UTF-32 are always unsigned
+// Set the signedness. UTF-8, UTF-16 and UTF-32 are always unsigned
 if (Literal.isWide())
   Val.setIsUnsigned(!TargetInfo::isTypeSigned(TI.getWCharType()));
-else if (!Literal.isUTF16() && !Literal.isUTF32())
+else if (!Literal.isUTF8() && !Literal.isUTF16() && !Literal.isUTF32())
   Val.setIsUnsigned(!PP.getLangOpts().CharIsSigned);
 
 if (Result.Val.getBitWidth() > Val.getBitWidth()) {
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -290,6 +290,8 @@
   template parameter, to conform to the Itanium C++ ABI and be compatible with
   GCC. This breaks binary compatibility with code compiled with earlier 

[PATCH] D112621: [analyzer][solver] Introduce reasoning for not equal to operator

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

@manas gentle ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112621

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

[PATCH] D124998: [AArch64][SVE] Add aarch64_sve_pcs attribute to Clang

[Peter Waller via Phabricator via cfe-commits]

peterwaller-arm added inline comments.



Comment at: clang/include/clang-c/Index.h:3448
+  CXCallingConv_AArch64SVEPcs= 17,
+  CXCallingConv_SwiftAsync = 18,
 

peterwaller-arm wrote:
> It shouldn't matter in principle (... "but in practice" ...) we should 
> probably avoid renumbering existing things in the enum and instead add to the 
> end of it.
> 
> Nit, this is missing a space before the equals.
> Nit, SVE is an acronym, so is PCS, so capitalization should be consistent 
> between the two. I see 'PCS' capitalized in AAPCS for example so probably all 
> upper case makes the sense.
> 
I retract my sloppy "it shouldn't matter in principle [at the source level]", 
of course it does matter, and it likely matters in this case (see 'alias for 
compatibility' comment above).

To be more specific, changing the enum is an ABI break, and breaks if these 
things are ever serialized and therefore not something you want to do.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124998

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

[PATCH] D124970: [Driver] Make "upgrade" of -include to include-pch optional; disable in clangd

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

thanks, LGTM!

as discussed offline, let's move forward with what you proposed (i.e. this 
patch, and then flip the default while making libclang opt-in)




Comment at: clang/include/clang/Driver/Driver.h:275
+  /// Whether to probe for PCH files on disk,
+  /// in order to upgrade -include foo.h to -include-pch foo.h.pch.
+  unsigned ProbePrecompiled : 1;

nit: reflow the comment



Comment at: clang/lib/Frontend/CreateInvocationFromCommandLine.cpp:54
   TheDriver.setCheckInputsExist(false);
+  // Don't probe on the filesystem for PCHes of -include'd headers - we may 
have
+  // a VFS, they may be misversioned, etc.

this comment feels a little bit outdated. we're definitely not turning this off 
and also probing is done in VFS now.
So maybe drop this and rather have a comment in header, pointing out possible 
failure modes due to version mismatch when this is turned on?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124970

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

[PATCH] D124971: [Frontend] give createInvocationFromCommandLine an options struct

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In D124971#3493657 , @kadircet wrote:

> Thanks this LGTM. Just two questions:
>
> - Why not migrate rest of the usages and drop the old function completely in 
> this patch

In order to keep the patch small. I can do this as a followup.

> - What about moving the function from clang/Frontend to Tooling, Support or 
> FrontendTool? this is clearly only inteded to be used by tools and not part 
> of the Frontend. There's actually Tooling/Tooling.h which has a 
> `newInvocation` that takes in cc1 args, rather than driver args. so these two 
> seem like siblings.

Sounds reasonable to me, but I'm not personally planning to work on it.
Note that ASTUnit depends on this, so there's some layering mess to clean up.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124971

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

[clang] 40c1372 - [Frontend] give createInvocationFromCommandLine an options struct

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2022-05-05T15:12:07+02:00
New Revision: 40c13720a4b977d4347bbde53c52a4d0703823c2

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

LOG: [Frontend] give createInvocationFromCommandLine an options struct

It's accumulating way too many optional params (see D124970)

While here, improve the name and the documentation.

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

Added: 


Modified: 
clang-tools-extra/clangd/Compiler.cpp
clang/include/clang/Frontend/Utils.h
clang/lib/Frontend/CreateInvocationFromCommandLine.cpp
clang/unittests/Frontend/UtilsTest.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/Compiler.cpp 
b/clang-tools-extra/clangd/Compiler.cpp
index 5779c627bc92b..a762b31f1dba1 100644
--- a/clang-tools-extra/clangd/Compiler.cpp
+++ b/clang-tools-extra/clangd/Compiler.cpp
@@ -91,12 +91,13 @@ buildCompilerInvocation(const ParseInputs &Inputs, 
clang::DiagnosticConsumer &D,
   for (const auto &S : Inputs.CompileCommand.CommandLine)
 ArgStrs.push_back(S.c_str());
 
-  auto VFS = Inputs.TFS->view(Inputs.CompileCommand.Directory);
-  llvm::IntrusiveRefCntPtr CommandLineDiagsEngine =
+  CreateInvocationOptions CIOpts;
+  CIOpts.VFS = Inputs.TFS->view(Inputs.CompileCommand.Directory);
+  CIOpts.CC1Args = CC1Args;
+  CIOpts.RecoverOnError = true;
+  CIOpts.Diags =
   CompilerInstance::createDiagnostics(new DiagnosticOptions, &D, false);
-  std::unique_ptr CI = createInvocationFromCommandLine(
-  ArgStrs, CommandLineDiagsEngine, std::move(VFS),
-  /*ShouldRecoverOnErrors=*/true, CC1Args);
+  std::unique_ptr CI = createInvocation(ArgStrs, CIOpts);
   if (!CI)
 return nullptr;
   // createInvocationFromCommandLine sets DisableFree.

diff  --git a/clang/include/clang/Frontend/Utils.h 
b/clang/include/clang/Frontend/Utils.h
index a05584bfe5514..14116eed9e704 100644
--- a/clang/include/clang/Frontend/Utils.h
+++ b/clang/include/clang/Frontend/Utils.h
@@ -22,7 +22,6 @@
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/StringSet.h"
-#include "llvm/Option/OptSpecifier.h"
 #include "llvm/Support/FileCollector.h"
 #include "llvm/Support/VirtualFileSystem.h"
 #include 
@@ -190,18 +189,47 @@ IntrusiveRefCntPtr
 createChainedIncludesSource(CompilerInstance &CI,
 IntrusiveRefCntPtr &Reader);
 
-/// createInvocationFromCommandLine - Construct a compiler invocation object 
for
-/// a command line argument vector.
+/// Optional inputs to createInvocation.
+struct CreateInvocationOptions {
+  /// Receives diagnostics encountered while parsing command-line flags.
+  /// If not provided, these are printed to stderr.
+  IntrusiveRefCntPtr Diags = nullptr;
+  /// Used e.g. to probe for system headers locations.
+  /// If not provided, the real filesystem is used.
+  /// FIXME: the driver does perform some non-virtualized IO.
+  IntrusiveRefCntPtr VFS = nullptr;
+  /// Whether to attempt to produce a non-null (possibly incorrect) invocation
+  /// if any errors were encountered.
+  /// By default, always return null on errors.
+  bool RecoverOnError = false;
+  /// If set, the target is populated with the cc1 args produced by the driver.
+  /// This may be populated even if createInvocation returns nullptr.
+  std::vector *CC1Args = nullptr;
+};
+
+/// Interpret clang arguments in preparation to parse a file.
+///
+/// This simulates a number of steps Clang takes when its driver is invoked:
+/// - choosing actions (e.g compile + link) to run
+/// - probing the system for settings like standard library locations
+/// - spawning a cc1 subprocess to compile code, with more explicit arguments
+/// - in the cc1 process, assembling those arguments into a CompilerInvocation
+///   which is used to configure the parser
 ///
-/// \param ShouldRecoverOnErrors - whether we should attempt to return a
-/// non-null (and possibly incorrect) CompilerInvocation if any errors were
-/// encountered. When this flag is false, always return null on errors.
+/// This simulation is lossy, e.g. in some situations one driver run would
+/// result in multiple parses. (Multi-arch, CUDA, ...).
+/// This function tries to select a reasonable invocation that tools should 
use.
 ///
-/// \param CC1Args - if non-null, will be populated with the args to cc1
-/// expanded from \p Args. May be set even if nullptr is returned.
+/// Args[0] should be the driver name, such as "clang" or "/usr/bin/g++".
+/// Absolute path is preferred - this affects searching for system headers.
 ///
-/// \return A CompilerInvocation, or nullptr if none was built for the given
-/// argument vector.
+/// May return nullptr if an invocation could not be determined.
+/// See CreateInvocationOptions::ShouldRecoverOnErrors to try 

[PATCH] D124971: [Frontend] give createInvocationFromCommandLine an options struct

[Sam McCall via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rG40c13720a4b9: [Frontend] give 
createInvocationFromCommandLine an options struct (authored by sammccall).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124971

Files:
  clang-tools-extra/clangd/Compiler.cpp
  clang/include/clang/Frontend/Utils.h
  clang/lib/Frontend/CreateInvocationFromCommandLine.cpp
  clang/unittests/Frontend/UtilsTest.cpp

Index: clang/unittests/Frontend/UtilsTest.cpp
===
--- clang/unittests/Frontend/UtilsTest.cpp
+++ clang/unittests/Frontend/UtilsTest.cpp
@@ -23,12 +23,12 @@
   std::vector Args = {"clang", "--target=macho", "-arch",  "i386",
 "-arch", "x86_64", "foo.cpp"};
   clang::IgnoringDiagConsumer D;
-  llvm::IntrusiveRefCntPtr CommandLineDiagsEngine =
-  clang::CompilerInstance::createDiagnostics(new DiagnosticOptions, &D,
- false);
-  std::unique_ptr CI = createInvocationFromCommandLine(
-  Args, CommandLineDiagsEngine, new llvm::vfs::InMemoryFileSystem(),
-  /*ShouldRecoverOnErrors=*/true);
+  CreateInvocationOptions Opts;
+  Opts.RecoverOnError = true;
+  Opts.Diags = clang::CompilerInstance::createDiagnostics(new DiagnosticOptions,
+  &D, false);
+  Opts.VFS = new llvm::vfs::InMemoryFileSystem();
+  std::unique_ptr CI = createInvocation(Args, Opts);
   ASSERT_TRUE(CI);
   EXPECT_THAT(CI->TargetOpts->Triple, testing::StartsWith("i386-"));
 }
Index: clang/lib/Frontend/CreateInvocationFromCommandLine.cpp
===
--- clang/lib/Frontend/CreateInvocationFromCommandLine.cpp
+++ clang/lib/Frontend/CreateInvocationFromCommandLine.cpp
@@ -26,16 +26,13 @@
 using namespace clang;
 using namespace llvm::opt;
 
-std::unique_ptr clang::createInvocationFromCommandLine(
-ArrayRef ArgList, IntrusiveRefCntPtr Diags,
-IntrusiveRefCntPtr VFS, bool ShouldRecoverOnErorrs,
-std::vector *CC1Args) {
+std::unique_ptr
+clang::createInvocation(ArrayRef ArgList,
+CreateInvocationOptions Opts) {
   assert(!ArgList.empty());
-  if (!Diags.get()) {
-// No diagnostics engine was provided, so create our own diagnostics object
-// with the default options.
-Diags = CompilerInstance::createDiagnostics(new DiagnosticOptions);
-  }
+  auto Diags = Opts.Diags
+   ? std::move(Opts.Diags)
+   : CompilerInstance::createDiagnostics(new DiagnosticOptions);
 
   SmallVector Args(ArgList.begin(), ArgList.end());
 
@@ -47,7 +44,7 @@
 
   // FIXME: We shouldn't have to pass in the path info.
   driver::Driver TheDriver(Args[0], llvm::sys::getDefaultTargetTriple(), *Diags,
-   "clang LLVM compiler", VFS);
+   "clang LLVM compiler", Opts.VFS);
 
   // Don't check that inputs exist, they may have been remapped.
   TheDriver.setCheckInputsExist(false);
@@ -81,7 +78,7 @@
 }
   }
 
-  bool PickFirstOfMany = OffloadCompilation || ShouldRecoverOnErorrs;
+  bool PickFirstOfMany = OffloadCompilation || Opts.RecoverOnError;
   if (Jobs.size() == 0 || (Jobs.size() > 1 && !PickFirstOfMany)) {
 SmallString<256> Msg;
 llvm::raw_svector_ostream OS(Msg);
@@ -98,11 +95,20 @@
   }
 
   const ArgStringList &CCArgs = Cmd->getArguments();
-  if (CC1Args)
-*CC1Args = {CCArgs.begin(), CCArgs.end()};
+  if (Opts.CC1Args)
+*Opts.CC1Args = {CCArgs.begin(), CCArgs.end()};
   auto CI = std::make_unique();
   if (!CompilerInvocation::CreateFromArgs(*CI, CCArgs, *Diags, Args[0]) &&
-  !ShouldRecoverOnErorrs)
+  !Opts.RecoverOnError)
 return nullptr;
   return CI;
 }
+
+std::unique_ptr clang::createInvocationFromCommandLine(
+ArrayRef Args, IntrusiveRefCntPtr Diags,
+IntrusiveRefCntPtr VFS, bool ShouldRecoverOnErrors,
+std::vector *CC1Args) {
+  return createInvocation(
+  Args,
+  CreateInvocationOptions{Diags, VFS, ShouldRecoverOnErrors, CC1Args});
+}
Index: clang/include/clang/Frontend/Utils.h
===
--- clang/include/clang/Frontend/Utils.h
+++ clang/include/clang/Frontend/Utils.h
@@ -22,7 +22,6 @@
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/StringSet.h"
-#include "llvm/Option/OptSpecifier.h"
 #include "llvm/Support/FileCollector.h"
 #include "llvm/Support/VirtualFileSystem.h"
 #include 
@@ -190,18 +189,47 @@
 createChainedIncludesSource(CompilerInstance &CI,
 IntrusiveRefCntPtr &Reader);
 
-/// createInvocationFromCommandLine - Construct a compiler invocation object for
-/// a command line argument vector.
+/

[PATCH] D124955: [clang-tidy] Make header-guard check a little looser on comment whitespace

[Haojian Wu via Phabricator via cfe-commits]

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

I think `"// FOO_BAR_H"` in your message should be `"//  FOO_BAR_H" ` (two 
space).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124955

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

[PATCH] D124932: [clang][dataflow] Track `optional` contents in `optional` model.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel updated this revision to Diff 427296.
ymandel added a comment.

Fixed the FIXME


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124932

Files:
  clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
  clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
@@ -2150,8 +2150,195 @@
   UnorderedElementsAre(Pair("check-1", "safe"), Pair("check-2", "safe")));
 }
 
+TEST_P(UncheckedOptionalAccessTest, NestedOptionalInOptional) {
+  ExpectLatticeChecksFor(
+  R"(
+#include "unchecked_optional_access_test.h"
+
+using Foo = $ns::$optional;
+
+void target($ns::$optional foo) {
+  if (foo && *foo) {
+foo->value();
+/*[[access]]*/
+  }
+}
+  )",
+  UnorderedElementsAre(Pair("access", "safe")));
+}
+
+TEST_P(UncheckedOptionalAccessTest, NestedOptionalInOptionalStruct) {
+  ExpectLatticeChecksFor(
+  R"(
+#include "unchecked_optional_access_test.h"
+
+struct Foo {
+  $ns::$optional opt;
+};
+
+void target(const $ns::$optional& foo) {
+  if (foo && foo->opt) {
+foo->opt.value();
+/*[[access]]*/
+  }
+}
+  )",
+  UnorderedElementsAre(Pair("access", "safe")));
+
+  // `reset` changes the state.
+  ExpectLatticeChecksFor(
+  R"(
+#include "unchecked_optional_access_test.h"
+
+struct Foo {
+  $ns::$optional opt;
+};
+
+void target($ns::$optional& foo) {
+  if (foo && foo->opt) {
+foo->opt.reset();
+foo->opt.value();
+/*[[reset]]*/
+  }
+}
+  )",
+  UnorderedElementsAre(Pair("reset", "unsafe: input.cc:11:9")));
+
+  // `has_value` and `operator*`.
+  ExpectLatticeChecksFor(
+  R"(
+#include "unchecked_optional_access_test.h"
+
+struct Foo {
+  $ns::$optional opt;
+};
+
+void target($ns::$optional& foo) {
+  if (foo && foo->opt.has_value()) {
+*foo->opt;
+/*[[other-ops]]*/
+  }
+}
+  )",
+  UnorderedElementsAre(Pair("other-ops", "safe")));
+
+  // `operator->`.
+  ExpectLatticeChecksFor(
+  R"(
+#include "unchecked_optional_access_test.h"
+
+struct Foo {
+  $ns::$optional opt;
+};
+
+void target($ns::$optional& foo) {
+  if (foo && foo->opt.has_value()) {
+foo->opt->empty();
+/*[[arrow]]*/
+  }
+}
+  )",
+  UnorderedElementsAre(Pair("arrow", "safe")));
+
+  // Accessing the nested optional in the wrong branch.
+  ExpectLatticeChecksFor(
+  R"(
+#include "unchecked_optional_access_test.h"
+
+struct Foo {
+  $ns::$optional opt;
+};
+
+void target($ns::$optional& foo) {
+  if (!foo.has_value()) return;
+  if (!foo->opt.has_value()) {
+foo->opt.value();
+/*[[not-set]]*/
+  }
+}
+  )",
+  UnorderedElementsAre(Pair("not-set", "unsafe: input.cc:11:9")));
+}
+
+TEST_P(UncheckedOptionalAccessTest, NestedOptionalInOptionalStructField) {
+  // Non-standard assignment.
+  ExpectLatticeChecksFor(
+  R"(
+#include "unchecked_optional_access_test.h"
+
+struct Foo {
+  $ns::$optional opt;
+};
+
+struct Bar {
+  $ns::$optional member;
+};
+
+Bar createBar();
+
+void target() {
+  Bar bar = createBar();
+  bar.member->opt = "a";
+  /*[[ns-assign]]*/
+}
+  )",
+  UnorderedElementsAre(Pair("ns-assign", "unsafe: input.cc:16:7")));
+
+  // Resetting the nested optional.
+  ExpectLatticeChecksFor(
+  R"(
+#include "unchecked_optional_access_test.h"
+
+struct Member {
+  $ns::$optional opt;
+};
+
+struct Foo {
+  $ns::$optional member;
+};
+
+Foo createFoo();
+
+void target() {
+  Foo foo = createFoo();
+  foo.member->opt.reset();
+  /*[[nested-reset]]*/
+}
+  )",
+  UnorderedElementsAre(Pair("nested-reset", "unsafe: input.cc:16:7")));
+}
+
+TEST_P(UncheckedOptionalAccessTest, NestedOptionalInitialization) {
+  // FIXME: Fix when to initialize `value`. All unwrapping should be safe in
+  // this example, but `value` initialization is done multiple times during the
+  // fixpoint iterations and joining the environment won't correctly merge them.
+  ExpectLatticeChecksFor(
+  R"(
+#include "unchecked_optional_access_test.h"
+
+using Foo = $ns::$optional;
+
+void target($ns::$optional foo, bool b) {
+  if (!foo.has_value()) return;
+  if (b) {
+if (!foo->has_value()) return;
+// We have created `foo.value()`.
+foo->value();
+  } else {
+if (!foo->has_value()) return;
+// We have created `foo.val

[PATCH] D124998: [AArch64][SVE] Add aarch64_sve_pcs attribute to Clang

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/include/clang-c/Index.h:3448
+  CXCallingConv_AArch64SVEPcs= 17,
+  CXCallingConv_SwiftAsync = 18,
 

peterwaller-arm wrote:
> peterwaller-arm wrote:
> > It shouldn't matter in principle (... "but in practice" ...) we should 
> > probably avoid renumbering existing things in the enum and instead add to 
> > the end of it.
> > 
> > Nit, this is missing a space before the equals.
> > Nit, SVE is an acronym, so is PCS, so capitalization should be consistent 
> > between the two. I see 'PCS' capitalized in AAPCS for example so probably 
> > all upper case makes the sense.
> > 
> I retract my sloppy "it shouldn't matter in principle [at the source level]", 
> of course it does matter, and it likely matters in this case (see 'alias for 
> compatibility' comment above).
> 
> To be more specific, changing the enum is an ABI break, and breaks if these 
> things are ever serialized and therefore not something you want to do.
+1 -- I was just making that comment when you beat me to it.



Comment at: clang/include/clang/Basic/Specifiers.h:283
 CC_AArch64VectorCall, // __attribute__((aarch64_vector_pcs))
+CC_AArch64SVEPcs, // __attribute__((aarch64_sve_pcs))
   };

I continue to be concerned with how many custom calling conventions we're 
adding. We're running out of bits to store them and when we bump up against the 
next limitation (32 possible calling conventions, this patch brings us up to 
20), we're going to lose a significant amount of performance due to extra 
memory overhead pressure (which, in turn, will greatly reduce our maximum 
template instantiation depth) with no clear path forward. We've looked into 
this before and once we hit that 32 calling convention limit, we won't be 
adding any new calling conventions until we solve the memory pressure problem.

Are there other alternatives than making a new calling convention? (e.g., can 
this be handled with a backend pass somehow, or some other implementation 
strategy?)
Is this calling convention going to be used often enough to warrant adding it? 
(e.g., will this be showing up in system headers or other major third-party 
library headers, or is this a one-off for people trying to eek the last ounce 
of performance out of a function call?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124998

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

[PATCH] D124932: [clang][dataflow] Track `optional` contents in `optional` model.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel added a comment.

In D124932#3492495 , @xazax.hun wrote:

> Overall looks good to me. I am curious what will the strategy be to properly 
> support construction. Do you plan to introduce a customization point to 
> `Env.createValue` to give checks/models a way to set properties up? Or do you 
> have something else in mind?

If we want to go the eager route, than I think that's the "right" solution.  
Anything else is brittle, since it is easy to miss a construction point.  
However, I'm more inclined to move the whole framework to a lazy initialization 
process, if we can devise one.




Comment at: 
clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp:219
+/// its location. Returns nullptr if the value can't be represented.
+StorageLocation *initializeUnwrappedValue(const Expr &Expr,
+  StructValue &OptionalVal,

xazax.hun wrote:
> I recall some problems with convergence due to creating fresh locations 
> lazily in multiple branches. I wonder if doubling down before finding a 
> proper solution could make it harder to fix this in the near future. If this 
> is just a temporary solution until construction is better supported, it might 
> be fine.
Excellent question. Yes, you're right about the convergence issue this falls 
into the same category.  That said, we intentionally reuse the same location in 
all branches, which mitigates that issue in this case -- we are at least 
assured that the two branches will have the contents value at the same 
location. In fact, the FIXME I added is wrong (and I've adjusted it 
accordingly). The property will be shared by all blocks that share the same 
value representing the `Optional` (which for the most part is done eagerly).

To your larger point, about the advisability of this approach, even for the 
short term:
1. I'm pushing on this largely because I have the data to support that it works 
in practice. We've done multiple surveys of significant size (~1000 files, 10k 
optional accesses) and found it does very well. So, this is not much of an 
issue in practice, in large part because the contents of optionals don't tend 
to further influence the outcome of the check (e.g. in the case of nested 
optionals).
2. I thought of redoing this in an eager fashion, but realized that ultimately 
we want to move everything to a lazy model, so it seemed less than ideal to 
start trying to be eager here.




Comment at: 
clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp:286
   // Record that this unwrap is *not* provably safe.
   State.Lattice.getSourceLocations().insert(ObjectExpr->getBeginLoc());
 }

xazax.hun wrote:
> I wonder how well this strategy works in practice. If we have many `unwrap` 
> calls to the same `optional`, we might end up emitting lots of diagnostics. 
> An alternative approach is to assume that the `optional` has a value after 
> the first unwrap. This would ensure that we only report a particular problem 
> once and can reduce the noise.
I see your point, and agree that there does seem room for significantly 
reducing warnings, even if all are correct. But, in practice, we haven't gotten 
complaints (which is a little surprising, actually). I wonder if it would be 
good to pair the report with the surrounding compound statement, and limit to 
one per compound statement. I fear that shutting it off entirely after the 
first issue might be _too_ quiet. Either way, this is definitely something that 
we'll be actively tweaking.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124932

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

[PATCH] D124998: [AArch64][SVE] Add aarch64_sve_pcs attribute to Clang

[Aaron Ballman via Phabricator via cfe-commits]

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

Making it clear there are necessary changes (at least to unbreak the C ABI).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124998

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

[PATCH] D124715: Use QoS class Utility for ThreadPriority::Low on Mac

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

I agree that clangd's indexing should probably be higher priority than whatever 
FS indexing is going on (because clangd's indexing only start when user 
actually starts working on a project, hence this indicates some level of 
"interactiveness"). so I feel like the right default is Utility, rather than 
Background.
But we had this behavior for a long while and people didn't complain up until 
M1s. So this is definitely going to result in some behavior change for intel 
users, even if we agree on this is making the state better for M1s.
Unfortunately I am not in a position to say what does the Utility vs Background 
imply for intel chips for sure, to tell the behavior change isn't going to be a 
regression. If this renders next clangd unusable by intel mac folks, it would 
be really unfortunate.

So I am also in favor of keeping the Background and switch the default in 
clangd to Low, while providing a flag to turn this back into Background. 
Hopefully we can retire the flag in between.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124715

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

[PATCH] D124998: [AArch64][SVE] Add aarch64_sve_pcs attribute to Clang

[Thorsten via Phabricator via cfe-commits]

tschuett added inline comments.



Comment at: clang/include/clang-c/Index.h:3448
+  CXCallingConv_AArch64SVEPcs= 17,
+  CXCallingConv_SwiftAsync = 18,
 

aaron.ballman wrote:
> peterwaller-arm wrote:
> > peterwaller-arm wrote:
> > > It shouldn't matter in principle (... "but in practice" ...) we should 
> > > probably avoid renumbering existing things in the enum and instead add to 
> > > the end of it.
> > > 
> > > Nit, this is missing a space before the equals.
> > > Nit, SVE is an acronym, so is PCS, so capitalization should be consistent 
> > > between the two. I see 'PCS' capitalized in AAPCS for example so probably 
> > > all upper case makes the sense.
> > > 
> > I retract my sloppy "it shouldn't matter in principle [at the source 
> > level]", of course it does matter, and it likely matters in this case (see 
> > 'alias for compatibility' comment above).
> > 
> > To be more specific, changing the enum is an ABI break, and breaks if these 
> > things are ever serialized and therefore not something you want to do.
> +1 -- I was just making that comment when you beat me to it.
Could you please add your CC as 18 and add a comment the next CC will 19 and do 
not touch the other ones.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124998

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

[PATCH] D124998: [AArch64][SVE] Add aarch64_sve_pcs attribute to Clang

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: clang/include/clang/Basic/Specifiers.h:283
 CC_AArch64VectorCall, // __attribute__((aarch64_vector_pcs))
+CC_AArch64SVEPcs, // __attribute__((aarch64_sve_pcs))
   };

aaron.ballman wrote:
> I continue to be concerned with how many custom calling conventions we're 
> adding. We're running out of bits to store them and when we bump up against 
> the next limitation (32 possible calling conventions, this patch brings us up 
> to 20), we're going to lose a significant amount of performance due to extra 
> memory overhead pressure (which, in turn, will greatly reduce our maximum 
> template instantiation depth) with no clear path forward. We've looked into 
> this before and once we hit that 32 calling convention limit, we won't be 
> adding any new calling conventions until we solve the memory pressure problem.
> 
> Are there other alternatives than making a new calling convention? (e.g., can 
> this be handled with a backend pass somehow, or some other implementation 
> strategy?)
> Is this calling convention going to be used often enough to warrant adding 
> it? (e.g., will this be showing up in system headers or other major 
> third-party library headers, or is this a one-off for people trying to eek 
> the last ounce of performance out of a function call?)
I agree with this, our list of calling conventions needs to find a way to get 
smaller, not larger.  our `ExtInfo` is already shocking/frighteningly large use 
of bits with calling conventions taking 5.  IMO, we should reduce this to 4 
bits if at all possible, particularly since we're likely to need more bits in 
the function-type thanks to the C++ committee.

Internally, Ive been fighting pretty hard to keep us from extending this list 
unless at all possible, and I think we need to do so here.  


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124998

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

[PATCH] D119147: [AIX][clang][driver] Check the command string to the linker for exportlist opts and

[Digger Lin via Phabricator via cfe-commits]

DiggerLin updated this revision to Diff 427297.
DiggerLin marked an inline comment as done.
DiggerLin added a comment.

address MaskRay's comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119147

Files:
  clang/include/clang/Driver/Job.h
  clang/lib/Driver/Job.cpp
  clang/lib/Driver/ToolChains/AIX.cpp
  clang/test/Driver/aix-ld.c

Index: clang/test/Driver/aix-ld.c
===
--- clang/test/Driver/aix-ld.c
+++ clang/test/Driver/aix-ld.c
@@ -596,17 +596,30 @@
 // RUN:   | FileCheck --check-prefix=CHECK-LD-LIBSTDCXX %s
 // CHECK-LD-LIBSTDCXX: LLVM ERROR: linking libstdc++ unimplemented on AIX
 
-// Check powerpc64-ibm-aix7.1.0.0, 32-bit. -shared.
+// Check powerpc-ibm-aix7.1.0.0, 32-bit. -shared.
 // RUN: %clangxx -x c++ %s 2>&1 -### \
-// RUN:-resource-dir=%S/Inputs/resource_dir \
-// RUN:-shared \
+// RUN:-resource-dir=%S/Inputs/resource_dir -shared \
+// RUN:--target=powerpc-ibm-aix7.1.0.0 \
+// RUN:--sysroot %S/Inputs/aix_ppc_tree \
+// RUN:--unwindlib=libunwind \
+// RUN:   | FileCheck --check-prefix=CHECK-LD32-SHARED %s
+
+// Check powerpc-ibm-aix7.1.0.0, 32-bit. -shared (with exp option strings in other opt).
+// RUN: %clangxx -x c++ %s 2>&1 -### \
+// RUN:-resource-dir=%S/Inputs/resource_dir -shared \
 // RUN:--target=powerpc-ibm-aix7.1.0.0 \
 // RUN:--sysroot %S/Inputs/aix_ppc_tree \
+// RUN:-ccc-install-dir %T/open_xl_aix_install/opt/IBM/openxlC/%open_xl_vrm/bin \
 // RUN:--unwindlib=libunwind \
+// RUN:-Wl,-Z/expall/expfull/a-bE:/a-bexport:/ \
 // RUN:   | FileCheck --check-prefix=CHECK-LD32-SHARED %s
+
 // CHECK-LD32-SHARED: "-cc1" "-triple" "powerpc-ibm-aix7.1.0.0"
 // CHECK-LD32-SHARED: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
 // CHECK-LD32-SHARED: "-isysroot" "[[SYSROOT:[^"]+]]"
+// CHECK-LD32-SHARED: "{{.*}}llvm-nm"
+// CHECK-LD32-SHARED: "--export-symbols"
+// CHECK-LD32-SHARED: "-X" "32"
 // CHECK-LD32-SHARED: "{{.*}}ld{{(.exe)?}}"
 // CHECK-LD32-SHARED: "-bM:SRE"
 // CHECK-LD32-SHARED: "-bnoentry"
@@ -623,10 +636,56 @@
 // CHECK-LD32-SHARED: "-lm"
 // CHECK-LD32-SHARED: "-lc"
 
+// Check powerpc-ibm-aix7.1.0.0, 32-bit. -shared with export list.
+// RUN: %clangxx -x c++ %s 2>&1 -### \
+// RUN:-resource-dir=%S/Inputs/resource_dir -shared \
+// RUN:--target=powerpc-ibm-aix7.1.0.0 \
+// RUN:--sysroot %S/Inputs/aix_ppc_tree \
+// RUN:-ccc-install-dir %T/open_xl_aix_install/opt/IBM/openxlC/%open_xl_vrm/bin \
+// RUN:-Wl,-bE:input.exp \
+// RUN:   | FileCheck --check-prefix=CHECK-LD32-SHARED-EXPORTS %s
+
+// Check powerpc-ibm-aix7.1.0.0, 32-bit. -shared with export list (no -Wl, variant).
+// RUN: %clangxx -x c++ %s 2>&1 -### \
+// RUN:-resource-dir=%S/Inputs/resource_dir -shared \
+// RUN:--target=powerpc-ibm-aix7.1.0.0 \
+// RUN:--sysroot %S/Inputs/aix_ppc_tree \
+// RUN:-ccc-install-dir %T/open_xl_aix_install/opt/IBM/openxlC/%open_xl_vrm/bin \
+// RUN:-bE:input.exp \
+// RUN:   | FileCheck --check-prefix=CHECK-LD32-SHARED-EXPORTS %s
+
+// Check powerpc-ibm-aix7.1.0.0, 32-bit. -shared with export list (-Xlinker variant).
+// RUN: %clangxx -x c++ %s 2>&1 -### \
+// RUN:-resource-dir=%S/Inputs/resource_dir -shared \
+// RUN:--target=powerpc-ibm-aix7.1.0.0 \
+// RUN:--sysroot %S/Inputs/aix_ppc_tree \
+// RUN:-ccc-install-dir %T/open_xl_aix_install/opt/IBM/openxlC/%open_xl_vrm/bin \
+// RUN:-Xlinker -bE:input.exp \
+// RUN:   | FileCheck --check-prefix=CHECK-LD32-SHARED-EXPORTS %s
+
+// CHECK-LD32-SHARED-EXPORTS: "-cc1" "-triple" "powerpc-ibm-aix7.1.0.0"
+// CHECK-LD32-SHARED-EXPORTS: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
+// CHECK-LD32-SHARED-EXPORTS: "-isysroot" "[[SYSROOT:[^"]+]]"
+// CHECK-LD32-SHARED-EXPORTS-NOT: "{{.*}}llvm-nm"
+// CHECK-LD32-SHARED-EXPORTS-NOT: "-X"
+// CHECK-LD32-SHARED-EXPORTS-NOT: "32"
+// CHECK-LD32-SHARED-EXPORTS: "{{.*}}ld{{(.exe)?}}"
+// CHECK-LD32-SHARED-EXPORTS: "-bM:SRE"
+// CHECK-LD32-SHARED-EXPORTS: "-bnoentry"
+// CHECK-LD32-SHARED-EXPORTS: "-b32"
+// CHECK-LD32-SHARED-EXPORTS: "-bpT:0x1000" "-bpD:0x2000"
+// CHECK-LD32-SHARED-EXPORTS-NOT: "[[SYSROOT]]/usr/lib{{/|}}crt0.o"
+// CHECK-LD32-SHARED-EXPORTS-NOT: "[[SYSROOT]]/usr/lib{{/|}}crti.o"
+// CHECK-LD32-SHARED-EXPORTS: "-b{{(" ")?}}E:input.exp"
+// CHECK-LD32-SHARED-EXPORTS-NOT: "-bE:{{[^"]+}}"
+// CHECK-LD32-SHARED-EXPORTS: "-lc++"
+// CHECK-LD32-SHARED-EXPORTS: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc.a"
+// CHECK-LD32-SHARED-EXPORTS: "-lm"
+// CHECK-LD32-SHARED-EXPORTS: "-lc"
+
 // Check powerpc64-ibm-aix7.1.0.0, 64-bit. -shared.
 // RUN: %clangxx -x c++ %s 2>&1 -### \
-// RUN:-resource-d

[PATCH] D124998: [AArch64][SVE] Add aarch64_sve_pcs attribute to Clang

[Paul Walker via Phabricator via cfe-commits]

paulwalker-arm added a comment.

Just wanted to say this is not a new calling convention as such, but rather an 
existing one that is generally auto-detected based on function signature.  The 
problem we're trying to solve here is that we need a way to allow a user to 
force the calling convention when the function signature would not normally 
choose it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124998

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

[PATCH] D124221: Reimplement `__builtin_dump_struct` in Sema.

[Erich Keane via Phabricator via cfe-commits]

erichkeane accepted this revision.
erichkeane added a comment.

FWIW, I'm in favor of the patch as it sits.

As a followup: So I was thinking about the "%s" specifier for string types.  
Assuming char-ptr types are all strings is a LITTLE dangerous, but more so the 
way we're doing it.  Its a shame we don't have some way of setting a 'max' 
limit to the number of characters we have for 2 reasons:

1- For safety: If the char-ptr points to non-null-terminated memory, it'll stop 
us from just arbitrarily printing into space by limiting at least the NUMBER of 
characters we print into nonsense.
2- For readability: printing a 'long' string likely makes this output look like 
nonsense and breaks everything up.  Limiting us to only a few characters is 
likely a good idea.
3- : It might discourage SOME level of attempts 
at using this for reflection, or at least make it a little harder.

What I would love would be for something like a 10 char max:

  struct S {
 char *C;
   };
   S s { "The Rest of this string is cut off"};
   print as:
   struct U20A a = {
 .c = 0x1234 "The Rest o"
   };

Sadly, I don't see something like that in printf specifiers?  Unless someone 
smarter than me can come up with some trickery.  PERHAPS have the max-limit 
compile-time configurable, but I don't feel strongly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124221

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

[PATCH] D122126: [LoopVectorize] Don't interleave when the number of runtime checks exceeds the threshold

[Tiehu Zhang via Phabricator via cfe-commits]

TiehuZhang updated this revision to Diff 427300.
TiehuZhang added a comment.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Fix the failed case (optimization-remark-options.c), because the remark info 
should be updated


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122126

Files:
  clang/test/Frontend/optimization-remark-options.c
  llvm/lib/Transforms/Vectorize/LoopVectorizationPlanner.h
  llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
  
llvm/test/Transforms/LoopVectorize/PowerPC/interleaved-pointer-runtime-check-unprofitable.ll

Index: llvm/test/Transforms/LoopVectorize/PowerPC/interleaved-pointer-runtime-check-unprofitable.ll
===
--- /dev/null
+++ llvm/test/Transforms/LoopVectorize/PowerPC/interleaved-pointer-runtime-check-unprofitable.ll
@@ -0,0 +1,90 @@
+; RUN: opt -mtriple=powerpc64-unknown-linux-gnu -mcpu=a2 -S -loop-vectorize  < %s -o - | FileCheck %s
+; RUN: opt -mtriple=powerpc64-unknown-linux-gnu -mcpu=a2 -S -loop-vectorize -force-vector-interleave=2 < %s -o - | FileCheck -check-prefix=CHECK-INTERLEAVE %s
+ 
+; The case will do aggressive interleave on PowerPC, resulting in a lot of memory checks.
+; (On the A2, always unroll aggressively. In fact, if aggressive interleaving is enabled,
+; similar issues may occur on other targets).
+; Interleaving should also be restricted by the threshold of memory checks similar to VF.
+; (e.g., runtime-memory-check-threshold, default 8).
+ 
+; CHECK-LABEL: @eddy_diff_caleddy_
+; CHECK-NOT: vector.memcheck
+ 
+; CHECK-INTERLEAVE-LABEL: @eddy_diff_caleddy_
+; CHECK-INTERLEAVE: vector.memcheck
+ 
+define fastcc void @eddy_diff_caleddy_(i64* %wet_cl, i64 %0, i32 %ncol.cast.val) {
+entry:
+  %trip.count = add nuw i32 %ncol.cast.val, 1
+  %wide.trip.count = zext i32 %ncol.cast.val to i64
+  %1 = shl i64 %0, 1
+  %2 = mul i64 %0, 3
+  %3 = shl i64 %0, 2
+  %4 = mul i64 %0, 5
+  %5 = mul i64 %0, 6
+  %6 = mul i64 %0, 7
+  %7 = shl i64 %0, 3
+  %8 = mul i64 %0, 9
+  %9 = mul i64 %0, 10
+  %10 = mul i64 %0, 11
+  %11 = mul i64 %0, 12
+  br label %loop.body
+ 
+loop.body:
+  %indvars.iv774 = phi i64 [ 0, %entry ], [ %indvars.iv.next775, %loop.body ]
+  %12 = add nsw i64 %indvars.iv774, -5
+  %13 = add i64 %12, %0
+  %14 = getelementptr i64, i64* %wet_cl, i64 %13
+  %15 = bitcast i64* %14 to double*
+  store double 0.00e+00, double* %15, align 8
+  %16 = add i64 %12, %1
+  %17 = getelementptr i64, i64* %wet_cl, i64 %16
+  %18 = bitcast i64* %17 to double*
+  store double 0.00e+00, double* %18, align 8
+  %19 = add i64 %12, %2
+  %20 = getelementptr i64, i64* %wet_cl, i64 %19
+  %21 = bitcast i64* %20 to double*
+  store double 0.00e+00, double* %21, align 8
+  %22 = add i64 %12, %3
+  %23 = getelementptr i64, i64* %wet_cl, i64 %22
+  %24 = bitcast i64* %23 to double*
+  store double 0.00e+00, double* %24, align 8
+  %25 = add i64 %12, %4
+  %26 = getelementptr i64, i64* %wet_cl, i64 %25
+  %27 = bitcast i64* %26 to double*
+  store double 0.00e+00, double* %27, align 8
+  %28 = add i64 %12, %5
+  %29 = getelementptr i64, i64* %wet_cl, i64 %28
+  %30 = bitcast i64* %29 to double*
+  store double 0.00e+00, double* %30, align 8
+  %31 = add i64 %12, %6
+  %32 = getelementptr i64, i64* %wet_cl, i64 %31
+  %33 = bitcast i64* %32 to double*
+  store double 0.00e+00, double* %33, align 8
+  %34 = add i64 %12, %7
+  %35 = getelementptr i64, i64* %wet_cl, i64 %34
+  %36 = bitcast i64* %35 to double*
+  store double 0.00e+00, double* %36, align 8
+  %37 = add i64 %12, %8
+  %38 = getelementptr i64, i64* %wet_cl, i64 %37
+  %39 = bitcast i64* %38 to double*
+  store double 0.00e+00, double* %39, align 8
+  %40 = add i64 %12, %9
+  %41 = getelementptr i64, i64* %wet_cl, i64 %40
+  %42 = bitcast i64* %41 to double*
+  store double 0.00e+00, double* %42, align 8
+  %43 = add i64 %12, %10
+  %44 = getelementptr i64, i64* %wet_cl, i64 %43
+  %45 = bitcast i64* %44 to double*
+  store double 0.00e+00, double* %45, align 8
+  %46 = add i64 %12, %11
+  %47 = getelementptr i64, i64* %wet_cl, i64 %46
+  %48 = bitcast i64* %47 to double*
+  store double 0.00e+00, double* %48, align 8
+  %indvars.iv.next775 = add nuw nsw i64 %indvars.iv774, 1
+  %exitcond778.not = icmp eq i64 %indvars.iv.next775, %wide.trip.count
+  br i1 %exitcond778.not, label %loop.end, label %loop.body
+ 
+loop.end:
+  ret void
+}
Index: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
===
--- llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
+++ llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
@@ -7450,6 +7450,14 @@
   return VectorizationFactor::Disabled();
 }
 
+bool LoopVectorizationPlanner::requiresTooManyRuntimeChecks() {
+  unsigned NumRuntimePointerChecks = Requirements.getNumRuntimePointerChecks();
+  return (NumRuntimePointerCheck

[PATCH] D124774: [clang][ASTImporter][NFC]: Move clang::ImportError into own header.

[Shivam Rajput via Phabricator via cfe-commits]

phyBrackets added a comment.

is there still any problem ? I'm not sure ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124774

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

[PATCH] D119675: Also remove the empty StoredDeclsList entry from the lookup table

[Gabor Marton via Phabricator via cfe-commits]

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

LGTM! And please accept my apologies @v.g.vassilev for taking it months to 
review this small change.


Repository:
  rC Clang

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

https://reviews.llvm.org/D119675

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

[clang] c894e85 - In MSVC compatibility mode, handle unqualified templated base class initialization

[Marco Antognini via cfe-commits]

Author: Fred Tingaud
Date: 2022-05-05T16:03:39+02:00
New Revision: c894e85fc64dd8d83b460de81080fff93c5ca334

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

LOG: In MSVC compatibility mode, handle unqualified templated base class 
initialization

Before C++20, MSVC was supporting not mentioning the template argument of the 
base class when initializing a class inheriting a templated base class.
So the following code compiled correctly:
```
template 
class Base {
};

template 
class Derived : public Base {
public:
  Derived() : Base() {}
};

void test() {
Derived d;
}
```
See https://godbolt.org/z/Pxxe7nccx for a conformance view.

This patch adds support for such construct when in MSVC compatibility mode.

Reviewed By: rnk

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

Added: 
clang/test/SemaTemplate/ms-unqualified-base-class.cpp

Modified: 
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaDeclCXX.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 45540fae58538..614bacc34af51 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -5509,6 +5509,9 @@ def err_found_later_in_class : Error<"member %0 used 
before its declaration">;
 def ext_found_later_in_class : ExtWarn<
   "use of member %0 before its declaration is a Microsoft extension">,
   InGroup;
+def ext_unqualified_base_class : ExtWarn<
+  "unqualified base initializer of class templates is a Microsoft extension">,
+  InGroup;
 def note_dependent_member_use : Note<
   "must qualify identifier to find this declaration in dependent base class">;
 def err_not_found_by_two_phase_lookup : Error<"call to function %0 that is 
neither "

diff  --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index b45e8e396b31a..a16fe0b1b72a4 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -4307,6 +4307,15 @@ Sema::BuildMemInitializer(Decl *ConstructorD,
 }
   }
 
+  if (getLangOpts().MSVCCompat && !getLangOpts().CPlusPlus20) {
+auto UnqualifiedBase = R.getAsSingle();
+if (UnqualifiedBase) {
+  Diag(IdLoc, diag::ext_unqualified_base_class)
+  << SourceRange(IdLoc, Init->getSourceRange().getEnd());
+  BaseType = UnqualifiedBase->getInjectedClassNameSpecialization();
+}
+  }
+
   // If no results were found, try to correct typos.
   TypoCorrection Corr;
   MemInitializerValidatorCCC CCC(ClassDecl);

diff  --git a/clang/test/SemaTemplate/ms-unqualified-base-class.cpp 
b/clang/test/SemaTemplate/ms-unqualified-base-class.cpp
new file mode 100644
index 0..6d9a072769c63
--- /dev/null
+++ b/clang/test/SemaTemplate/ms-unqualified-base-class.cpp
@@ -0,0 +1,85 @@
+// RUN: %clang_cc1 -std=c++17 -fms-compatibility -fsyntax-only 
-verify=before,expected %s
+// RUN: %clang_cc1 -std=c++17 -fms-compatibility -fdelayed-template-parsing 
-fsyntax-only -verify=before,expected %s
+// RUN: %clang_cc1 -std=c++20 -fms-compatibility -fsyntax-only 
-verify=after,expected %s
+// RUN: %clang_cc1 -std=c++20 -fms-compatibility -fdelayed-template-parsing 
-fsyntax-only -verify=after,expected %s
+
+template 
+class Base {
+};
+
+template 
+class Based {}; // Trying to trick the typo detection
+
+template 
+class Derived : public Base {
+public:
+  // after-error@+1 {{member initializer 'Base' does not name a non-static 
data member or base class}}
+  Derived() : Base() {} // before-warning {{unqualified base initializer of 
class templates is a Microsoft extension}}
+private:
+  int Baze; // Trying to trick the typo detection
+};
+
+template  struct AggregateBase {
+  T i;
+};
+
+template 
+struct AggregateDerived : public AggregateBase {
+  int i;
+
+  // after-error@+1 {{member initializer 'AggregateBase' does not name a 
non-static data member or base class}}
+  AggregateDerived(T j) : AggregateBase{4}, i{j} {} // before-warning 
{{unqualified base initializer of class templates is a Microsoft extension}}
+  int f() {
+return i + AggregateBase::i; // expected-warning {{use of undeclared 
identifier 'AggregateBase'; unqualified lookup into dependent bases of class 
template 'AggregateDerived' is a Microsoft extension}}
+  }
+};
+
+template  struct MultiTypesBase {
+};
+
+template 
+struct MultiTypesDerived : public MultiTypesBase {
+  // after-error@+1 {{member initializer 'MultiTypesBase' does not name a 
non-static data member or base class}}
+  MultiTypesDerived() : MultiTypesBase{} {} // before-warning {{unqualified 
base initializer of class templates is a Microsoft extension}}
+};
+
+template  struct IntegerBase {
+};
+
+temp

[PATCH] D124666: In MSVC compatibility mode, handle unqualified templated base class initialization

[Marco Antognini via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGc894e85fc64d: In MSVC compatibility mode, handle unqualified 
templated base class… (authored by frederic-tingaud-sonarsource, committed by 
mantognini).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124666

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/SemaTemplate/ms-unqualified-base-class.cpp

Index: clang/test/SemaTemplate/ms-unqualified-base-class.cpp
===
--- /dev/null
+++ clang/test/SemaTemplate/ms-unqualified-base-class.cpp
@@ -0,0 +1,85 @@
+// RUN: %clang_cc1 -std=c++17 -fms-compatibility -fsyntax-only -verify=before,expected %s
+// RUN: %clang_cc1 -std=c++17 -fms-compatibility -fdelayed-template-parsing -fsyntax-only -verify=before,expected %s
+// RUN: %clang_cc1 -std=c++20 -fms-compatibility -fsyntax-only -verify=after,expected %s
+// RUN: %clang_cc1 -std=c++20 -fms-compatibility -fdelayed-template-parsing -fsyntax-only -verify=after,expected %s
+
+template 
+class Base {
+};
+
+template 
+class Based {}; // Trying to trick the typo detection
+
+template 
+class Derived : public Base {
+public:
+  // after-error@+1 {{member initializer 'Base' does not name a non-static data member or base class}}
+  Derived() : Base() {} // before-warning {{unqualified base initializer of class templates is a Microsoft extension}}
+private:
+  int Baze; // Trying to trick the typo detection
+};
+
+template  struct AggregateBase {
+  T i;
+};
+
+template 
+struct AggregateDerived : public AggregateBase {
+  int i;
+
+  // after-error@+1 {{member initializer 'AggregateBase' does not name a non-static data member or base class}}
+  AggregateDerived(T j) : AggregateBase{4}, i{j} {} // before-warning {{unqualified base initializer of class templates is a Microsoft extension}}
+  int f() {
+return i + AggregateBase::i; // expected-warning {{use of undeclared identifier 'AggregateBase'; unqualified lookup into dependent bases of class template 'AggregateDerived' is a Microsoft extension}}
+  }
+};
+
+template  struct MultiTypesBase {
+};
+
+template 
+struct MultiTypesDerived : public MultiTypesBase {
+  // after-error@+1 {{member initializer 'MultiTypesBase' does not name a non-static data member or base class}}
+  MultiTypesDerived() : MultiTypesBase{} {} // before-warning {{unqualified base initializer of class templates is a Microsoft extension}}
+};
+
+template  struct IntegerBase {
+};
+
+template 
+struct IntegerDerived : public IntegerBase {
+  // after-error@+1 {{member initializer 'IntegerBase' does not name a non-static data member or base class}}
+  IntegerDerived() : IntegerBase{} {} // before-warning {{unqualified base initializer of class templates is a Microsoft extension}}
+};
+
+template  struct ConformingBase {
+  T i;
+};
+
+template 
+struct ConformingDerived : public ConformingBase {
+  int i;
+
+  ConformingDerived(T j) : ConformingBase{4}, i{j} {}
+  int f() {
+return i + ConformingBase::i;
+  }
+};
+
+int main() {
+  int I;
+  Derived t;
+
+  AggregateDerived AD{2};
+  AD.AggregateBase::i = 3;
+  I = AD.f();
+
+  MultiTypesDerived MTD;
+
+  IntegerDerived<4> ID;
+
+  ConformingDerived CD{2};
+  I = CD.f();
+
+  return I;
+}
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -4307,6 +4307,15 @@
 }
   }
 
+  if (getLangOpts().MSVCCompat && !getLangOpts().CPlusPlus20) {
+auto UnqualifiedBase = R.getAsSingle();
+if (UnqualifiedBase) {
+  Diag(IdLoc, diag::ext_unqualified_base_class)
+  << SourceRange(IdLoc, Init->getSourceRange().getEnd());
+  BaseType = UnqualifiedBase->getInjectedClassNameSpecialization();
+}
+  }
+
   // If no results were found, try to correct typos.
   TypoCorrection Corr;
   MemInitializerValidatorCCC CCC(ClassDecl);
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -5509,6 +5509,9 @@
 def ext_found_later_in_class : ExtWarn<
   "use of member %0 before its declaration is a Microsoft extension">,
   InGroup;
+def ext_unqualified_base_class : ExtWarn<
+  "unqualified base initializer of class templates is a Microsoft extension">,
+  InGroup;
 def note_dependent_member_use : Note<
   "must qualify identifier to find this declaration in dependent base class">;
 def err_not_found_by_two_phase_lookup : Error<"call to function %0 that is neither "
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo

[PATCH] D124774: [clang][ASTImporter][NFC]: Move clang::ImportError into own header.

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

balazske added a comment.

The code looks correct but the diff is not OK. I think that you have separate 
commits for every change (instead of changing the first with `git commit 
--amend` when a new change is made). In this case `arc diff --update  main` 
may work (if you have a separate branch for the changes). (I did not found a 
place in LLVM docs where this part of the process is exactly described.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124774

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

[PATCH] D125011: [MSVC] Add support for pragma alloc_text

[Stephen Long via Phabricator via cfe-commits]

steplong created this revision.
Herald added a project: All.
steplong requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

`#pragma alloc_text` is a MSVC pragma that names the code section where 
functions should be placed. It only
applies to functions with C linkage.

https://docs.microsoft.com/en-us/cpp/preprocessor/alloc-text?view=msvc-170


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125011

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/ParsePragma.cpp
  clang/lib/Sema/SemaAttr.cpp
  clang/lib/Sema/SemaDecl.cpp

Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -10220,8 +10220,10 @@
 
   // If this is a function definition, check if we have to apply optnone due to
   // a pragma.
-  if(D.isFunctionDefinition())
+  if(D.isFunctionDefinition()) {
 AddRangeBasedOptnone(NewFD);
+AddSectionMSAllocText(NewFD);
+  }
 
   // If this is the first declaration of an extern C variable, update
   // the map of such variables.
Index: clang/lib/Sema/SemaAttr.cpp
===
--- clang/lib/Sema/SemaAttr.cpp
+++ clang/lib/Sema/SemaAttr.cpp
@@ -741,6 +741,18 @@
   CurInitSegLoc = PragmaLocation;
 }
 
+void Sema::ActOnPragmaMSAllocText(SourceLocation PragmaLocation,
+  StringRef Section,
+  const SmallVector &Functions) {
+  if (!CurContext->getRedeclContext()->isFileContext()) {
+Diag(PragmaLocation, diag::err_pragma_alloc_text_scope);
+return;
+  }
+
+  for (auto &Function : Functions)
+FunctionToSectionMap[Function] = Section;
+}
+
 void Sema::ActOnPragmaUnused(const Token &IdTok, Scope *curScope,
  SourceLocation PragmaLoc) {
 
@@ -1072,6 +1084,17 @@
 AddOptnoneAttributeIfNoConflicts(FD, OptimizeOffPragmaLocation);
 }
 
+void Sema::AddSectionMSAllocText(FunctionDecl *FD) {
+  if (!FD->getIdentifier())
+return;
+
+  StringRef Name = FD->getName();
+  auto It = FunctionToSectionMap.find(Name);
+  if (It != FunctionToSectionMap.end())
+if (FD->isExternC() && !FD->hasAttr())
+  FD->addAttr(SectionAttr::CreateImplicit(Context, It->second));
+}
+
 void Sema::AddOptnoneAttributeIfNoConflicts(FunctionDecl *FD,
 SourceLocation Loc) {
   // Don't add a conflicting attribute. No diagnostic is needed.
Index: clang/lib/Parse/ParsePragma.cpp
===
--- clang/lib/Parse/ParsePragma.cpp
+++ clang/lib/Parse/ParsePragma.cpp
@@ -445,6 +445,8 @@
 PP.AddPragmaHandler(MSCodeSeg.get());
 MSSection = std::make_unique("section");
 PP.AddPragmaHandler(MSSection.get());
+MSAllocText = std::make_unique("alloc_text");
+PP.AddPragmaHandler(MSAllocText.get());
 MSRuntimeChecks = std::make_unique();
 PP.AddPragmaHandler(MSRuntimeChecks.get());
 MSIntrinsic = std::make_unique();
@@ -554,6 +556,8 @@
 MSCodeSeg.reset();
 PP.RemovePragmaHandler(MSSection.get());
 MSSection.reset();
+PP.RemovePragmaHandler(MSAllocText.get());
+MSAllocText.reset();
 PP.RemovePragmaHandler(MSRuntimeChecks.get());
 MSRuntimeChecks.reset();
 PP.RemovePragmaHandler(MSIntrinsic.get());
@@ -912,7 +916,8 @@
 .Case("const_seg", &Parser::HandlePragmaMSSegment)
 .Case("code_seg", &Parser::HandlePragmaMSSegment)
 .Case("section", &Parser::HandlePragmaMSSection)
-.Case("init_seg", &Parser::HandlePragmaMSInitSeg);
+.Case("init_seg", &Parser::HandlePragmaMSInitSeg)
+.Case("alloc_text", &Parser::HandlePragmaMSAllocText);
 
   if (!(this->*Handler)(PragmaName, PragmaLocation)) {
 // Pragma handling failed, and has been diagnosed.  Slurp up the tokens
@@ -1149,6 +1154,72 @@
   return true;
 }
 
+bool Parser::HandlePragmaMSAllocText(StringRef PragmaName,
+ SourceLocation PragmaLocation) {
+  Token FirstTok = Tok;
+  if (Tok.isNot(tok::l_paren)) {
+PP.Diag(Tok.getLocation(), diag::warn_pragma_expected_lparen)
+<< PragmaName;
+return false;
+  }
+  PP.Lex(Tok);
+
+  if (Tok.isNot(tok::string_literal)) {
+PP.Diag(PragmaLocation, diag::warn_pragma_expected_section_name)
+<< PragmaName;
+return false;
+  }
+  ExprResult StringResult = ParseStringLiteralExpression();
+  if (StringResult.isInvalid())
+return false; // Already diagnosed.
+  StringLiteral *SegmentName = cast(StringResult.get());
+  if (SegmentName->getCharByteWidth() != 1) {
+PP.Diag(PragmaLocation, diag::warn_pragma_expected_non_wide_string)
+<< PragmaName;
+return false;
+  }
+
+  if (Tok.isNot(tok::comma)) {
+PP.Diag(Tok.getLocation(), diag::warn_pragma_expected_comma) << Pra

[PATCH] D124816: [Tooling] use different FileManager for each CWD

[Shi Chen via Phabricator via cfe-commits]

Kale updated this revision to Diff 427313.
Kale added a comment.

Fix path in compile_commands.json for windows.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124816

Files:
  clang/include/clang/Basic/FileManager.h
  clang/include/clang/Tooling/Tooling.h
  clang/lib/Tooling/Tooling.cpp
  
clang/test/Tooling/multiple-source-include-different-header-with-same-relative-path.cpp

Index: clang/test/Tooling/multiple-source-include-different-header-with-same-relative-path.cpp
===
--- /dev/null
+++ clang/test/Tooling/multiple-source-include-different-header-with-same-relative-path.cpp
@@ -0,0 +1,42 @@
+// This test case presents a circumstance that the information related one file
+// is misused by another.
+//
+// The path tree of the test directory is as follors.
+//   .
+//   ├── a
+//   │   ├── a.c
+//   │   └── config.h
+//   ├── b
+//   │   ├── b.c
+//   │   └── config.h
+//   └── compile_commands.json
+//
+// Both source files (a/a.c and b/b.c) includes the config.h file of their own
+// directory with `#include "config.h"`. However, the two config.h files are
+// different. File a/config.h is longer than b/config.h. Both a/a.c and b/b.c
+// are compiled in their own directory, which is recorded in the compilation
+// database.
+//
+// When using ClangTool to parse these two source files one by one, since the
+// file name of both header files are the same, the FileManager will confuse
+// with them and using the file entry of a/config.h for b/config.h. And the
+// wrong file length will lead to a buffer overflow when reading the file.
+//
+// In this test case, to avoid buffer overflow, we use the leading '\0' in an
+// empty buffer to trigger the problem. We set a/config.h as an empty line
+// comment, and leave b/config.h empty. Firstly, a/config.h is read and cached,
+// then when reading b/config.h, if the size of a/config.h is used, the first
+// two chars are read and the first one must be a '\0'.
+
+// RUN: rm -rf %t
+// RUN: mkdir -p %t/a
+// RUN: mkdir -p %t/b
+// RUN: echo '#include "config.h"' > %t/a/a.c
+// RUN: echo '#include "config.h"' > %t/b/b.c
+// RUN: echo '//' > %t/a/config.h
+// RUN: echo ''   > %t/b/config.h
+// RUN: echo '[{"arguments": ["cc", "-c", "-o", "a.o", "a.c"], "directory": "%t/a", "file": "a.c"}, {"arguments": ["cc", "-c", "-o", "b.o", "b.c"], "directory": "%t/b", "file": "b.c"}]' | sed -e 's/\\//g' > %t/compile_commands.json
+
+// The following two test RUNs should have no output.
+// RUN: cd %t && clang-check a/a.c b/b.c 2>&1 | count 0
+// RUN: cd %t && clang-check b/b.c a/a.c 2>&1 | count 0
Index: clang/lib/Tooling/Tooling.cpp
===
--- clang/lib/Tooling/Tooling.cpp
+++ clang/lib/Tooling/Tooling.cpp
@@ -208,7 +208,7 @@
   StringRef FileNameRef = FileName.toNullTerminatedStringRef(FileNameStorage);
 
   llvm::IntrusiveRefCntPtr Files(
-  new FileManager(FileSystemOptions(), VFS));
+  new ToolingFileManager(FileSystemOptions(), VFS));
   ArgumentsAdjuster Adjuster = getClangStripDependencyFileAdjuster();
   ToolInvocation Invocation(
   getSyntaxOnlyToolArgs(ToolName, Adjuster(Args, FileNameRef), FileNameRef),
@@ -436,7 +436,8 @@
   OverlayFileSystem(new llvm::vfs::OverlayFileSystem(std::move(BaseFS))),
   InMemoryFileSystem(new llvm::vfs::InMemoryFileSystem),
   Files(Files ? Files
-  : new FileManager(FileSystemOptions(), OverlayFileSystem)) {
+  : new ToolingFileManager(FileSystemOptions(),
+   OverlayFileSystem)) {
   OverlayFileSystem->pushOverlay(InMemoryFileSystem);
   appendArgumentsAdjuster(getClangStripOutputAdjuster());
   appendArgumentsAdjuster(getClangSyntaxOnlyAdjuster());
@@ -656,7 +657,7 @@
   new llvm::vfs::InMemoryFileSystem);
   OverlayFileSystem->pushOverlay(InMemoryFileSystem);
   llvm::IntrusiveRefCntPtr Files(
-  new FileManager(FileSystemOptions(), OverlayFileSystem));
+  new ToolingFileManager(FileSystemOptions(), OverlayFileSystem));
 
   ToolInvocation Invocation(
   getSyntaxOnlyToolArgs(ToolName, Adjuster(Args, FileName), FileName),
Index: clang/include/clang/Tooling/Tooling.h
===
--- clang/include/clang/Tooling/Tooling.h
+++ clang/include/clang/Tooling/Tooling.h
@@ -65,6 +65,69 @@
 
 class CompilationDatabase;
 
+class ToolingFileManager : public FileManager {
+  // Use a FileManager for each different CWD
+  mutable std::map fileManagers;
+
+public:
+  ToolingFileManager(const FileSystemOptions &FileSystemOpts,
+ IntrusiveRefCntPtr FS = nullptr)
+  : FileManager(FileSystemOpts, FS){};
+
+  size_t getNumUniqueRealFiles() const override {
+return getRealFileManager()->getNumUniqu

[PATCH] D125012: [clang] createInvocationFromCommandLine -> createInvocation, delete former. NFC

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
sammccall added a reviewer: kadircet.
Herald added a subscriber: arphaman.
Herald added a project: All.
sammccall requested review of this revision.
Herald added projects: clang, LLDB.
Herald added subscribers: lldb-commits, cfe-commits.

(Followup from 40c13720a4b977d4347bbde53c52a4d0703823c2 
)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125012

Files:
  clang/include/clang/Frontend/Utils.h
  clang/lib/Frontend/ASTUnit.cpp
  clang/lib/Frontend/CreateInvocationFromCommandLine.cpp
  clang/tools/c-index-test/core_main.cpp
  clang/tools/diagtool/ShowEnabledWarnings.cpp
  clang/tools/libclang/Indexing.cpp
  clang/unittests/Frontend/ASTUnitTest.cpp
  clang/unittests/Frontend/CompilerInstanceTest.cpp
  clang/unittests/Serialization/ModuleCacheTest.cpp
  clang/unittests/Tooling/Syntax/TokensTest.cpp
  clang/unittests/Tooling/Syntax/TreeTestBase.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp

Index: lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
@@ -663,9 +663,11 @@
llvm::make_range(compiler_invocation_arguments.begin(),
 compiler_invocation_arguments.end()));
 
+  CreateInvocationOptions CIOpts;
+  CIOpts.Diags = diagnostics_engine;
   std::shared_ptr invocation =
-  clang::createInvocationFromCommandLine(compiler_invocation_argument_cstrs,
- diagnostics_engine);
+  clang::createInvocation(compiler_invocation_argument_cstrs,
+  std::move(CIOpts));
 
   if (!invocation)
 return nullptr;
Index: clang/unittests/Tooling/Syntax/TreeTestBase.cpp
===
--- clang/unittests/Tooling/Syntax/TreeTestBase.cpp
+++ clang/unittests/Tooling/Syntax/TreeTestBase.cpp
@@ -134,7 +134,10 @@
 ArgsCStr.push_back(arg.c_str());
   }
 
-  Invocation = createInvocationFromCommandLine(ArgsCStr, Diags, FS);
+  CreateInvocationOptions CIOpts;
+  CIOpts.Diags = Diags;
+  CIOpts.VFS = FS;
+  Invocation = createInvocation(ArgsCStr, std::move(CIOpts));
   assert(Invocation);
   Invocation->getFrontendOpts().DisableFree = false;
   Invocation->getPreprocessorOpts().addRemappedFile(
Index: clang/unittests/Tooling/Syntax/TokensTest.cpp
===
--- clang/unittests/Tooling/Syntax/TokensTest.cpp
+++ clang/unittests/Tooling/Syntax/TokensTest.cpp
@@ -125,7 +125,10 @@
   Diags->setClient(new IgnoringDiagConsumer);
 std::vector Args = {"tok-test", "-std=c++03", "-fsyntax-only",
   FileName};
-auto CI = createInvocationFromCommandLine(Args, Diags, FS);
+CreateInvocationOptions CIOpts;
+CIOpts.Diags = Diags;
+CIOpts.VFS = FS;
+auto CI = createInvocation(Args, std::move(CIOpts));
 assert(CI);
 CI->getFrontendOpts().DisableFree = false;
 CI->getPreprocessorOpts().addRemappedFile(
Index: clang/unittests/Serialization/ModuleCacheTest.cpp
===
--- clang/unittests/Serialization/ModuleCacheTest.cpp
+++ clang/unittests/Serialization/ModuleCacheTest.cpp
@@ -10,6 +10,7 @@
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/CompilerInvocation.h"
 #include "clang/Frontend/FrontendActions.h"
+#include "clang/Frontend/Utils.h"
 #include "clang/Lex/HeaderSearch.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/Support/FileSystem.h"
@@ -95,13 +96,15 @@
   MCPArg.append(ModuleCachePath);
   IntrusiveRefCntPtr Diags =
   CompilerInstance::createDiagnostics(new DiagnosticOptions());
+  CreateInvocationOptions CIOpts;
+  CIOpts.Diags = Diags;
 
   // First run should pass with no errors
   const char *Args[] = {"clang","-fmodules",  "-Fframeworks",
 MCPArg.c_str(), "-working-directory", TestDir.c_str(),
 "test.m"};
   std::shared_ptr Invocation =
-  createInvocationFromCommandLine(Args, Diags);
+  createInvocation(Args, CIOpts);
   ASSERT_TRUE(Invocation);
   CompilerInstance Instance;
   Instance.setDiagnostics(Diags.get());
@@ -124,7 +127,7 @@
  "-Fframeworks",  MCPArg.c_str(), "-working-directory",
  TestDir.c_str(), "test.m"};
   std::shared_ptr Invocation2 =
-  createInvocationFromCommandLine(Args2, Diags);
+  createInvocation(Args2, CIOpts);
   ASSERT_TRUE(Invocation2);
   CompilerInstance Instance2(Instance.getPCHContainerOperations(),
  &Instance.getModuleCache());
@@ -142,13 +145,15 @@
   MCPArg.append(ModuleCachePath);
  

[PATCH] D121120: [clang-tidy] New check for safe usage of `std::optional` and like types.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel created this revision.
Herald added subscribers: carlosgalvezp, mgorny.
Herald added a project: All.
Eugene.Zelenko retitled this revision from "[clang][tidy] New check for safe 
usage of `std::optional` and like types." to "[clang-tidy] New check for safe 
usage of `std::optional` and like types.".
Eugene.Zelenko added reviewers: alexfh, aaron.ballman, njames93, 
LegalizeAdulthood.
Eugene.Zelenko edited projects, added clang-tools-extra; removed All.
Herald added a subscriber: xazax.hun.
Herald added a project: All.
ymandel removed reviewers: alexfh, aaron.ballman, njames93, LegalizeAdulthood.
ymandel removed rG LLVM Github Monorepo as the repository for this revision.
ymandel removed subscribers: xazax.hun, Eugene.Zelenko, mgorny, carlosgalvezp.
ymandel added a comment.
ymandel updated this revision to Diff 413492.
ymandel updated this revision to Diff 413493.
ymandel updated this revision to Diff 427298.
ymandel added reviewers: sgatev, xazax.hun, aaron.ballman.
ymandel edited subscribers, added: kinu; removed: sgatev.
Herald added a subscriber: rnkovacs.
ymandel published this revision for review.
Herald added a subscriber: cfe-commits.

Thanks, Eugene -- I'm actually still working on this, filling out the FIXMEs, 
etc.   I uploaded it with `arc` as a draft and its clearly marked as such. Is 
there something more I should have done to preven it from being sent out?


ymandel added a comment.

Filled out all relevant docs/comments.


ymandel added a comment.

tweaks


ymandel added a comment.

Rebased




Comment at: 
clang-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.cpp:85
+
+  const auto ExitBlockState =
+  (*BlockToOutputState)[Context->getCFG().getExit().getBlockID()];

Please don't use `auto` if type is not spelled in same statement or iterator.



Comment at: 
clang-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.cpp:90
+
+  const auto ExitBlockLattice = ExitBlockState.getValue().Lattice;
+  for (const SourceLocation &Loc : ExitBlockLattice.getSourceLocations()) {

Ditto.



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:109
+
+  FIXME: add release notes.
+

Please add description. Hint: should be same as first statement in 
documentation.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/bugprone-unchecked-optional-access.rst:6
+
+FIXME: Describe what patterns does the check detect and why. Give examples.

Please do that.


This check verifies the safety of access to `std::optional` and related
types (including `absl::optional`). It is based on a corresponding Clang
Dataflow Analysis, which does most of the work. This check merely runs it and
converts its findings into diagnostics.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D121120

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/bugprone-unchecked-optional-access.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/Inputs/absl/types/optional.h
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-unchecked-optional-access.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-unchecked-optional-access.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-unchecked-optional-access.cpp
@@ -0,0 +1,112 @@
+// RUN: %check_clang_tidy %s bugprone-unchecked-optional-access %t -- -- -I %S/Inputs/
+
+#include "absl/types/optional.h"
+
+void unchecked_value_access(const absl::optional &opt) {
+  opt.value();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: unchecked access to optional value [bugprone-unchecked-optional-access]
+}
+
+void unchecked_deref_operator_access(const absl::optional &opt) {
+  *opt;
+  // CHECK-MESSAGES: :[[@LINE-1]]:4: warning: unchecked access to optional value
+}
+
+struct Foo {
+  void foo() const {}
+};
+
+void unchecked_arrow_operator_access(const absl::optional &opt) {
+  opt->foo();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: unchecked access to optional value
+}
+
+void f2(const absl::optional &opt) {
+  if (opt.has_value()) {
+opt.value();
+  }
+}
+
+template 
+void function_template_without_user(const absl::optional &opt) {
+  opt.value(); // no-warning
+}
+
+template 
+void function_template_with_user(const absl::optional &opt) {
+  opt.value();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: unchecked access to optional value
+}
+
+void function_template_user(const absl::optional &opt) {
+  // Instantiate the f3 function template so that it gets matched by the check.
+  

[PATCH] D124774: [clang][ASTImporter][NFC]: Move clang::ImportError into own header.

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

In D124774#3493868 , @balazske wrote:

> The code looks correct but the diff is not OK. I think that you have separate 
> commits for every change (instead of changing the first with `git commit 
> --amend` when a new change is made). In this case `arc diff --update  
> main` may work (if you have a separate branch for the changes). (I did not 
> found a place in LLVM docs where this part of the process is exactly 
> described.)

Yes, you should see that your baseline is valid. Below, the highlighted commit 
does not exists in the remote (in the upstream repository). 
F22978398: image.png 

I suggest the following.

  git remote add llvm g...@github.com:llvm/llvm-project.git
  git fetch llvm
  git rebase -i llvm/main //choose your relevant commits
  arc diff --update D124774 llvm/main


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124774

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

[PATCH] D124816: [Tooling] use different FileManager for each CWD

[Shi Chen via Phabricator via cfe-commits]

Kale updated this revision to Diff 427317.
Kale added a comment.

Format all overriding functions


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124816

Files:
  clang/include/clang/Basic/FileManager.h
  clang/include/clang/Tooling/Tooling.h
  clang/lib/Tooling/Tooling.cpp
  
clang/test/Tooling/multiple-source-include-different-header-with-same-relative-path.cpp

Index: clang/test/Tooling/multiple-source-include-different-header-with-same-relative-path.cpp
===
--- /dev/null
+++ clang/test/Tooling/multiple-source-include-different-header-with-same-relative-path.cpp
@@ -0,0 +1,42 @@
+// This test case presents a circumstance that the information related one file
+// is misused by another.
+//
+// The path tree of the test directory is as follors.
+//   .
+//   ├── a
+//   │   ├── a.c
+//   │   └── config.h
+//   ├── b
+//   │   ├── b.c
+//   │   └── config.h
+//   └── compile_commands.json
+//
+// Both source files (a/a.c and b/b.c) includes the config.h file of their own
+// directory with `#include "config.h"`. However, the two config.h files are
+// different. File a/config.h is longer than b/config.h. Both a/a.c and b/b.c
+// are compiled in their own directory, which is recorded in the compilation
+// database.
+//
+// When using ClangTool to parse these two source files one by one, since the
+// file name of both header files are the same, the FileManager will confuse
+// with them and using the file entry of a/config.h for b/config.h. And the
+// wrong file length will lead to a buffer overflow when reading the file.
+//
+// In this test case, to avoid buffer overflow, we use the leading '\0' in an
+// empty buffer to trigger the problem. We set a/config.h as an empty line
+// comment, and leave b/config.h empty. Firstly, a/config.h is read and cached,
+// then when reading b/config.h, if the size of a/config.h is used, the first
+// two chars are read and the first one must be a '\0'.
+
+// RUN: rm -rf %t
+// RUN: mkdir -p %t/a
+// RUN: mkdir -p %t/b
+// RUN: echo '#include "config.h"' > %t/a/a.c
+// RUN: echo '#include "config.h"' > %t/b/b.c
+// RUN: echo '//' > %t/a/config.h
+// RUN: echo ''   > %t/b/config.h
+// RUN: echo '[{"arguments": ["cc", "-c", "-o", "a.o", "a.c"], "directory": "%t/a", "file": "a.c"}, {"arguments": ["cc", "-c", "-o", "b.o", "b.c"], "directory": "%t/b", "file": "b.c"}]' | sed -e 's/\\//g' > %t/compile_commands.json
+
+// The following two test RUNs should have no output.
+// RUN: cd %t && clang-check a/a.c b/b.c 2>&1 | count 0
+// RUN: cd %t && clang-check b/b.c a/a.c 2>&1 | count 0
Index: clang/lib/Tooling/Tooling.cpp
===
--- clang/lib/Tooling/Tooling.cpp
+++ clang/lib/Tooling/Tooling.cpp
@@ -208,7 +208,7 @@
   StringRef FileNameRef = FileName.toNullTerminatedStringRef(FileNameStorage);
 
   llvm::IntrusiveRefCntPtr Files(
-  new FileManager(FileSystemOptions(), VFS));
+  new ToolingFileManager(FileSystemOptions(), VFS));
   ArgumentsAdjuster Adjuster = getClangStripDependencyFileAdjuster();
   ToolInvocation Invocation(
   getSyntaxOnlyToolArgs(ToolName, Adjuster(Args, FileNameRef), FileNameRef),
@@ -436,7 +436,8 @@
   OverlayFileSystem(new llvm::vfs::OverlayFileSystem(std::move(BaseFS))),
   InMemoryFileSystem(new llvm::vfs::InMemoryFileSystem),
   Files(Files ? Files
-  : new FileManager(FileSystemOptions(), OverlayFileSystem)) {
+  : new ToolingFileManager(FileSystemOptions(),
+   OverlayFileSystem)) {
   OverlayFileSystem->pushOverlay(InMemoryFileSystem);
   appendArgumentsAdjuster(getClangStripOutputAdjuster());
   appendArgumentsAdjuster(getClangSyntaxOnlyAdjuster());
@@ -656,7 +657,7 @@
   new llvm::vfs::InMemoryFileSystem);
   OverlayFileSystem->pushOverlay(InMemoryFileSystem);
   llvm::IntrusiveRefCntPtr Files(
-  new FileManager(FileSystemOptions(), OverlayFileSystem));
+  new ToolingFileManager(FileSystemOptions(), OverlayFileSystem));
 
   ToolInvocation Invocation(
   getSyntaxOnlyToolArgs(ToolName, Adjuster(Args, FileName), FileName),
Index: clang/include/clang/Tooling/Tooling.h
===
--- clang/include/clang/Tooling/Tooling.h
+++ clang/include/clang/Tooling/Tooling.h
@@ -65,6 +65,67 @@
 
 class CompilationDatabase;
 
+class ToolingFileManager : public FileManager {
+  // Use a FileManager for each different CWD
+  mutable std::map fileManagers;
+
+public:
+  ToolingFileManager(const FileSystemOptions &FileSystemOpts,
+ IntrusiveRefCntPtr FS = nullptr)
+  : FileManager(FileSystemOpts, FS){};
+
+  size_t getNumUniqueRealFiles() const override {
+return getRealFileManager()->getNumUniqueRealFiles();
+

[PATCH] D124996: [clang][preprocessor] Fix unsigned-ness of utf8 char literals

[Tom Honermann via Phabricator via cfe-commits]

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

I think changes are needed to make this behavior dependent on whether `char8_t` 
support is active or not.




Comment at: clang/lib/Lex/PPExpressions.cpp:413
   Val.setIsUnsigned(!TargetInfo::isTypeSigned(TI.getWCharType()));
-else if (!Literal.isUTF16() && !Literal.isUTF32())
+else if (!Literal.isUTF8() && !Literal.isUTF16() && !Literal.isUTF32())
   Val.setIsUnsigned(!PP.getLangOpts().CharIsSigned);

I think the check for UTF-8 should also be conditioned on 
`PP.getLangOpts().Char8`. When `char8_t` support is not enabled (as in C++17 or 
with `-fno-char8_t` in C++20), UTF-8 character literals still have type `char`.
  else if (!(Literal.isUTF8() && PP.getLangOpts().Char8) && 
!Literal.isUTF16() && !Literal.isUTF32())



Comment at: clang/test/Lexer/utf8-char-literal.cpp:4
 // RUN: %clang_cc1 -triple x86_64-apple-darwin -std=c2x -x c -fsyntax-only 
-verify %s
 // RUN: %clang_cc1 -triple x86_64-apple-darwin -std=c++1z -fsyntax-only 
-verify %s
 

I think we should drop testing for `-std=c++1z` and add testing of `-std=c++17` 
and `-std=c++20`. Ideally, the test would then validate the differences in 
behavior.



Comment at: clang/test/Lexer/utf8-char-literal.cpp:31-34
+#if __cplusplus > 201402L || __STDC_VERSION__ >= 202000L
+#if u8'\xff' != 0xff
+#error u8 char literal is not unsigned
+#endif

Prior to C++20 (unless `-fchar8_t` is passed), `u8'\xff'` should have the same 
behavior as `'\xff'`.


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

https://reviews.llvm.org/D124996

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

[PATCH] D125014: [Driver] Remove -fno-concept-satisfaction-caching

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov created this revision.
ilya-biryukov added a reviewer: kadircet.
Herald added a project: All.
ilya-biryukov requested review of this revision.
Herald added a project: clang.

The flag was added when the C++20 draft did not allow for concept
caching. The final C++20 standard permits the caching, so flag is
redundant. See http://wg21.link/p2104r0.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125014

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/Sema/SemaConcept.cpp
  clang/test/SemaTemplate/cxx2a-constraint-caching.cpp


Index: clang/test/SemaTemplate/cxx2a-constraint-caching.cpp
===
--- clang/test/SemaTemplate/cxx2a-constraint-caching.cpp
+++ clang/test/SemaTemplate/cxx2a-constraint-caching.cpp
@@ -1,5 +1,4 @@
 // RUN:  %clang_cc1 -std=c++2a -verify %s
-// RUN:  %clang_cc1 -std=c++2a -verify %s -fno-concept-satisfaction-caching 
-DNO_CACHE
 // expected-no-diagnostics
 
 template
@@ -25,10 +24,5 @@
   // because the constraint satisfaction results are cached.
   constexpr void f(A a, int = 2) {}
 }
-#ifdef NO_CACHE
-static_assert(!C);
-static_assert(!foo());
-#else
 static_assert(C);
 static_assert(foo());
-#endif
Index: clang/lib/Sema/SemaConcept.cpp
===
--- clang/lib/Sema/SemaConcept.cpp
+++ clang/lib/Sema/SemaConcept.cpp
@@ -317,10 +317,8 @@
 OutSatisfaction.IsSatisfied = true;
 return false;
   }
-
-  bool ShouldCache = LangOpts.ConceptSatisfactionCaching && Template;
-  if (!ShouldCache) {
-return ::CheckConstraintSatisfaction(*this, Template, ConstraintExprs,
+  if (!Template) {
+return ::CheckConstraintSatisfaction(*this, nullptr, ConstraintExprs,
  TemplateArgs, TemplateIDRange,
  OutSatisfaction);
   }
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -5656,10 +5656,6 @@
"The argument is parsed as blockname:major:minor:hashed:user info">;
 def fconcepts_ts : Flag<["-"], "fconcepts-ts">,
   HelpText<"Enable C++ Extensions for Concepts. (deprecated - use 
-std=c++2a)">;
-def fno_concept_satisfaction_caching : Flag<["-"],
-
"fno-concept-satisfaction-caching">,
-  HelpText<"Disable satisfaction caching for C++2a Concepts.">,
-  MarshallingInfoNegativeFlag>;
 
 defm recovery_ast : BoolOption<"f", "recovery-ast",
   LangOpts<"RecoveryAST">, DefaultTrue,
Index: clang/include/clang/Basic/LangOptions.def
===
--- clang/include/clang/Basic/LangOptions.def
+++ clang/include/clang/Basic/LangOptions.def
@@ -277,7 +277,6 @@
 LANGOPT(AlignedAllocation , 1, 0, "aligned allocation")
 LANGOPT(AlignedAllocationUnavailable, 1, 0, "aligned allocation functions are 
unavailable")
 LANGOPT(NewAlignOverride  , 32, 0, "maximum alignment guaranteed by 
'::operator new(size_t)'")
-LANGOPT(ConceptSatisfactionCaching , 1, 1, "enable satisfaction caching for 
C++20 Concepts")
 BENIGN_LANGOPT(ModulesCodegen , 1, 0, "Modules code generation")
 BENIGN_LANGOPT(ModulesDebugInfo , 1, 0, "Modules debug info")
 BENIGN_LANGOPT(ElideConstructors , 1, 1, "C++ copy constructor elision")
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -222,6 +222,10 @@
 
 Removed Compiler Flags
 -
+- Removed the ``-fno-concept-satisfaction-caching`` flag. The flag was added
+  at the time when the draft of C++20 standard did not permit caching of
+  atomic constraints. The final standard permits such caching, see
+  `WG21 P2104R0`_.
 
 New Pragmas in Clang
 


Index: clang/test/SemaTemplate/cxx2a-constraint-caching.cpp
===
--- clang/test/SemaTemplate/cxx2a-constraint-caching.cpp
+++ clang/test/SemaTemplate/cxx2a-constraint-caching.cpp
@@ -1,5 +1,4 @@
 // RUN:  %clang_cc1 -std=c++2a -verify %s
-// RUN:  %clang_cc1 -std=c++2a -verify %s -fno-concept-satisfaction-caching -DNO_CACHE
 // expected-no-diagnostics
 
 template
@@ -25,10 +24,5 @@
   // because the constraint satisfaction results are cached.
   constexpr void f(A a, int = 2) {}
 }
-#ifdef NO_CACHE
-static_assert(!C);
-static_assert(!foo());
-#else
 static_assert(C);
 static_assert(foo());
-#endif
Index: clang/lib/Sema/SemaConcept.cpp
===
--- clang/lib/Sema/SemaConcept.cpp
+++ clang/lib/Sema/SemaConcept.cpp
@@ -317,10 +317,8 @@
 OutSatisfaction.IsSatisfied = true;
 return false

[PATCH] D124774: [clang][ASTImporter][NFC]: Move clang::ImportError into own header.

[Shivam Rajput via Phabricator via cfe-commits]

phyBrackets updated this revision to Diff 427322.
phyBrackets added a comment.

Rebase several commits


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124774

Files:
  clang/include/clang/AST/ASTImportError.h
  clang/include/clang/AST/ASTImporter.h
  clang/include/clang/AST/ASTImporterSharedState.h

Index: clang/include/clang/AST/ASTImporterSharedState.h
===
--- clang/include/clang/AST/ASTImporterSharedState.h
+++ clang/include/clang/AST/ASTImporterSharedState.h
@@ -14,11 +14,10 @@
 #ifndef LLVM_CLANG_AST_ASTIMPORTERSHAREDSTATE_H
 #define LLVM_CLANG_AST_ASTIMPORTERSHAREDSTATE_H
 
+#include "clang/AST/ASTImportError.h"
 #include "clang/AST/ASTImporterLookupTable.h"
 #include "clang/AST/Decl.h"
 #include "llvm/ADT/DenseMap.h"
-// FIXME We need this because of ImportError.
-#include "clang/AST/ASTImporter.h"
 
 namespace clang {
 
Index: clang/include/clang/AST/ASTImporter.h
===
--- clang/include/clang/AST/ASTImporter.h
+++ clang/include/clang/AST/ASTImporter.h
@@ -14,7 +14,7 @@
 #ifndef LLVM_CLANG_AST_ASTIMPORTER_H
 #define LLVM_CLANG_AST_ASTIMPORTER_H
 
-#include "clang/AST/APValue.h"
+#include "clang/AST/ASTImportError.h"
 #include "clang/AST/DeclBase.h"
 #include "clang/AST/DeclarationName.h"
 #include "clang/AST/ExprCXX.h"
@@ -29,7 +29,6 @@
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/SmallVector.h"
-#include "llvm/Support/Error.h"
 #include 
 
 namespace clang {
@@ -49,33 +48,6 @@
 class TranslationUnitDecl;
 class TypeSourceInfo;
 
-  class ImportError : public llvm::ErrorInfo {
-  public:
-/// \brief Kind of error when importing an AST component.
-enum ErrorKind {
-NameConflict, /// Naming ambiguity (likely ODR violation).
-UnsupportedConstruct, /// Not supported node or case.
-Unknown /// Other error.
-};
-
-ErrorKind Error;
-
-static char ID;
-
-ImportError() : Error(Unknown) {}
-ImportError(const ImportError &Other) : Error(Other.Error) {}
-ImportError &operator=(const ImportError &Other) {
-  Error = Other.Error;
-  return *this;
-}
-ImportError(ErrorKind Error) : Error(Error) { }
-
-std::string toString() const;
-
-void log(raw_ostream &OS) const override;
-std::error_code convertToErrorCode() const override;
-  };
-
   // \brief Returns with a list of declarations started from the canonical decl
   // then followed by subsequent decls in the translation unit.
   // This gives a canonical list for each entry in the redecl chain.
Index: clang/include/clang/AST/ASTImportError.h
===
--- /dev/null
+++ clang/include/clang/AST/ASTImportError.h
@@ -0,0 +1,50 @@
+//===- ASTImportError.h - Define errors while importing AST -*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+//  This file defines the ASTImportError class which basically defines the kind
+//  of error while importing AST .
+//
+//===--===//
+
+#ifndef LLVM_CLANG_AST_ASTIMPORTERROR_H
+#define LLVM_CLANG_AST_ASTIMPORTERROR_H
+
+#include "llvm/Support/Error.h"
+
+namespace clang {
+
+class ImportError : public llvm::ErrorInfo {
+public:
+  /// \brief Kind of error when importing an AST component.
+  enum ErrorKind {
+NameConflict, /// Naming ambiguity (likely ODR violation).
+UnsupportedConstruct, /// Not supported node or case.
+Unknown   /// Other error.
+  };
+
+  ErrorKind Error;
+
+  static char ID;
+
+  ImportError() : Error(Unknown) {}
+  ImportError(const ImportError &Other) : Error(Other.Error) {}
+  ImportError &operator=(const ImportError &Other) {
+Error = Other.Error;
+return *this;
+  }
+  ImportError(ErrorKind Error) : Error(Error) {}
+
+  std::string toString() const;
+
+  void log(llvm::raw_ostream &OS) const override;
+  std::error_code convertToErrorCode() const override;
+};
+
+} // namespace clang
+
+#endif // LLVM_CLANG_AST_ASTIMPORTERROR_H
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D124850: [Sema][SVE2] Move/simplify Sema testing for SVE2 ACLE builtins

[Rosie Sumpter via Phabricator via cfe-commits]

RosieSumpter updated this revision to Diff 427320.
RosieSumpter added a comment.

- Corrected name `const_b16_ptr` to `const_bf16_ptr` in acle_sve2_bfloat.cpp


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

https://reviews.llvm.org/D124850

Files:
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_aba.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_abalb.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_abalt.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_abdlb.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_abdlt.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_adalp.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_adclb.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_adclt.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_addhnb.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_addhnt.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_addlb.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_addlbt.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_addlt.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_addp.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_addwb.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_addwt.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_aesd.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_aese.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_aesimc.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_aesmc.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_bcax.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_bdep.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_bext.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_bgrp.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_bsl.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_bsl1n.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_bsl2n.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_cadd.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_cdot.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_cmla.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_cvtlt.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_cvtnt.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_cvtx.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_cvtxnt.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_eor3.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_eorbt.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_eortb.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_hadd.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_histcnt.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_histseg.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_hsub.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_hsubr.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_ldnt1.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_ldnt1sb.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_ldnt1sh.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_ldnt1sw.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_ldnt1ub.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_ldnt1uh.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_ldnt1uw.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_logb.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_match.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_maxnmp.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_maxp.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_minnmp.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_minp.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_mla.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_mlalb.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_mlalt.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_mls.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_mlslb.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_mlslt.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_movlb.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_movlt.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_mul.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_mullb.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_mullt.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_nbsl.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_nmatch.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_pmul.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_pmullb.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_pmullb_128.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_pmullt.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_pmullt_128.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_qabs.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_qadd.c

[clang-tools-extra] 04b4190 - [Driver] Make "upgrade" of -include to include-pch optional; disable in clangd

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2022-05-05T16:47:17+02:00
New Revision: 04b419048955fc33718ba97e79f3558b6a27830e

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

LOG: [Driver] Make "upgrade" of -include to include-pch optional; disable in 
clangd

If clang is passed "-include foo.h", it will rewrite to "-include-pch foo.h.pch"
before passing it to cc1, if foo.h.pch exists.

Existence is checked, but validity is not. This is probably a reasonable
assumption for the compiler itself, but not for clang-based tools where the
actual compiler may be a different version of clang, or even GCC.
In the end, we lose our -include, we gain a -include-pch that can't be used,
and the file often fails to parse.

I would like to turn this off for all non-clang invocations (i.e.
createInvocationFromCommandLine), but we have explicit tests of this behavior
for libclang and I can't work out the implications of changing it.

Instead this patch:
 - makes it optional in the driver, default on (no change)
 - makes it optional in createInvocationFromCommandLine, default on (no change)
 - changes driver to do IO through the VFS so it can be tested
 - tests the option
 - turns the option off in clangd where the problem was reported

Subsequent patches should make libclang opt in explicitly and flip the default
for all other tools. It's probably also time to extract an options struct
for createInvocationFromCommandLine.

Fixes https://github.com/clangd/clangd/issues/856
Fixes https://github.com/clangd/vscode-clangd/issues/324

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

Added: 


Modified: 
clang-tools-extra/clangd/Compiler.cpp
clang/include/clang/Driver/Driver.h
clang/include/clang/Frontend/Utils.h
clang/lib/Driver/Driver.cpp
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Frontend/CreateInvocationFromCommandLine.cpp
clang/unittests/Frontend/UtilsTest.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/Compiler.cpp 
b/clang-tools-extra/clangd/Compiler.cpp
index a762b31f1dba1..2cf2eb8cda140 100644
--- a/clang-tools-extra/clangd/Compiler.cpp
+++ b/clang-tools-extra/clangd/Compiler.cpp
@@ -97,6 +97,7 @@ buildCompilerInvocation(const ParseInputs &Inputs, 
clang::DiagnosticConsumer &D,
   CIOpts.RecoverOnError = true;
   CIOpts.Diags =
   CompilerInstance::createDiagnostics(new DiagnosticOptions, &D, false);
+  CIOpts.ProbePrecompiled = false;
   std::unique_ptr CI = createInvocation(ArgStrs, CIOpts);
   if (!CI)
 return nullptr;

diff  --git a/clang/include/clang/Driver/Driver.h 
b/clang/include/clang/Driver/Driver.h
index f68f281f0e931..eb739ef31d57b 100644
--- a/clang/include/clang/Driver/Driver.h
+++ b/clang/include/clang/Driver/Driver.h
@@ -271,6 +271,9 @@ class Driver {
   /// Whether to check that input files exist when constructing compilation
   /// jobs.
   unsigned CheckInputsExist : 1;
+  /// Whether to probe for PCH files on disk, in order to upgrade
+  /// -include foo.h to -include-pch foo.h.pch.
+  unsigned ProbePrecompiled : 1;
 
 public:
   /// Force clang to emit reproducer for driver invocation. This is enabled
@@ -357,6 +360,9 @@ class Driver {
 
   void setCheckInputsExist(bool Value) { CheckInputsExist = Value; }
 
+  bool getProbePrecompiled() const { return ProbePrecompiled; }
+  void setProbePrecompiled(bool Value) { ProbePrecompiled = Value; }
+
   void setTargetAndMode(const ParsedClangName &TM) { ClangNameParts = TM; }
 
   const std::string &getTitle() { return DriverTitle; }

diff  --git a/clang/include/clang/Frontend/Utils.h 
b/clang/include/clang/Frontend/Utils.h
index 14116eed9e704..47f795b388f7a 100644
--- a/clang/include/clang/Frontend/Utils.h
+++ b/clang/include/clang/Frontend/Utils.h
@@ -202,6 +202,11 @@ struct CreateInvocationOptions {
   /// if any errors were encountered.
   /// By default, always return null on errors.
   bool RecoverOnError = false;
+  /// Allow the driver to probe the filesystem for PCH files.
+  /// This is used to replace -include with -include-pch in the cc1 args.
+  /// FIXME: ProbePrecompiled=true is a poor, historical default.
+  /// It misbehaves if the PCH file is from GCC, has the wrong version, etc.
+  bool ProbePrecompiled = true;
   /// If set, the target is populated with the cc1 args produced by the driver.
   /// This may be populated even if createInvocation returns nullptr.
   std::vector *CC1Args = nullptr;
@@ -236,7 +241,7 @@ std::unique_ptr 
createInvocationFromCommandLine(
 IntrusiveRefCntPtr(),
 IntrusiveRefCntPtr VFS = nullptr,
 bool ShouldRecoverOnErrors = false,
-std::vector *CC1Args = nullptr);
+std::vector *CC1Args = nullptr, bool ProbePrecompiled = true);
 
 } // namespace clang
 

diff  --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
in

[PATCH] D124970: [Driver] Make "upgrade" of -include to include-pch optional; disable in clangd

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
sammccall marked 2 inline comments as done.
Closed by commit rG04b419048955: [Driver] Make "upgrade" of -include 
to include-pch optional; disable in clangd (authored by sammccall).

Changed prior to commit:
  https://reviews.llvm.org/D124970?vs=427159&id=427323#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124970

Files:
  clang-tools-extra/clangd/Compiler.cpp
  clang/include/clang/Driver/Driver.h
  clang/include/clang/Frontend/Utils.h
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CreateInvocationFromCommandLine.cpp
  clang/unittests/Frontend/UtilsTest.cpp

Index: clang/unittests/Frontend/UtilsTest.cpp
===
--- clang/unittests/Frontend/UtilsTest.cpp
+++ clang/unittests/Frontend/UtilsTest.cpp
@@ -11,12 +11,15 @@
 #include "clang/Basic/TargetOptions.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/CompilerInvocation.h"
+#include "clang/Lex/PreprocessorOptions.h"
+#include "llvm/ADT/IntrusiveRefCntPtr.h"
 #include "llvm/Support/VirtualFileSystem.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
 namespace clang {
 namespace {
+using testing::ElementsAre;
 
 TEST(BuildCompilerInvocationTest, RecoverMultipleJobs) {
   // This generates multiple jobs and we recover by using the first.
@@ -33,5 +36,31 @@
   EXPECT_THAT(CI->TargetOpts->Triple, testing::StartsWith("i386-"));
 }
 
+// buildInvocationFromCommandLine should not translate -include to -include-pch,
+// even if the PCH file exists.
+TEST(BuildCompilerInvocationTest, ProbePrecompiled) {
+  std::vector Args = {"clang", "-include", "foo.h", "foo.cpp"};
+  auto FS = llvm::makeIntrusiveRefCnt();
+  FS->addFile("foo.h", 0, llvm::MemoryBuffer::getMemBuffer(""));
+  FS->addFile("foo.h.pch", 0, llvm::MemoryBuffer::getMemBuffer(""));
+
+  clang::IgnoringDiagConsumer D;
+  llvm::IntrusiveRefCntPtr CommandLineDiagsEngine =
+  clang::CompilerInstance::createDiagnostics(new DiagnosticOptions, &D,
+ false);
+  // Default: ProbePrecompiled is true.
+  std::unique_ptr CI = createInvocationFromCommandLine(
+  Args, CommandLineDiagsEngine, FS, false, nullptr);
+  ASSERT_TRUE(CI);
+  EXPECT_THAT(CI->getPreprocessorOpts().Includes, ElementsAre());
+  EXPECT_EQ(CI->getPreprocessorOpts().ImplicitPCHInclude, "foo.h.pch");
+
+  CI = createInvocationFromCommandLine(Args, CommandLineDiagsEngine, FS, false,
+   nullptr, /*ProbePrecompiled=*/false);
+  ASSERT_TRUE(CI);
+  EXPECT_THAT(CI->getPreprocessorOpts().Includes, ElementsAre("foo.h"));
+  EXPECT_EQ(CI->getPreprocessorOpts().ImplicitPCHInclude, "");
+}
+
 } // namespace
 } // namespace clang
Index: clang/lib/Frontend/CreateInvocationFromCommandLine.cpp
===
--- clang/lib/Frontend/CreateInvocationFromCommandLine.cpp
+++ clang/lib/Frontend/CreateInvocationFromCommandLine.cpp
@@ -48,6 +48,7 @@
 
   // Don't check that inputs exist, they may have been remapped.
   TheDriver.setCheckInputsExist(false);
+  TheDriver.setProbePrecompiled(Opts.ProbePrecompiled);
 
   std::unique_ptr C(TheDriver.BuildCompilation(Args));
   if (!C)
@@ -107,8 +108,8 @@
 std::unique_ptr clang::createInvocationFromCommandLine(
 ArrayRef Args, IntrusiveRefCntPtr Diags,
 IntrusiveRefCntPtr VFS, bool ShouldRecoverOnErrors,
-std::vector *CC1Args) {
+std::vector *CC1Args, bool ProbePrecompiled) {
   return createInvocation(
-  Args,
-  CreateInvocationOptions{Diags, VFS, ShouldRecoverOnErrors, CC1Args});
+  Args, CreateInvocationOptions{Diags, VFS, ShouldRecoverOnErrors,
+ProbePrecompiled, CC1Args});
 }
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -1364,7 +1364,8 @@
 
   bool RenderedImplicitInclude = false;
   for (const Arg *A : Args.filtered(options::OPT_clang_i_Group)) {
-if (A->getOption().matches(options::OPT_include)) {
+if (A->getOption().matches(options::OPT_include) &&
+D.getProbePrecompiled()) {
   // Handling of gcc-style gch precompiled headers.
   bool IsFirstImplicitInclude = !RenderedImplicitInclude;
   RenderedImplicitInclude = true;
@@ -1375,12 +1376,12 @@
   // so that replace_extension does the right thing.
   P += ".dummy";
   llvm::sys::path::replace_extension(P, "pch");
-  if (llvm::sys::fs::exists(P))
+  if (D.getVFS().exists(P))
 FoundPCH = true;
 
   if (!FoundPCH) {
 llvm::sys::path::replace_extension(P, "gch");
-if (llvm::sys::fs::exists(P)) {
+if (D.getVFS().exists(P)) {
   Fo

[PATCH] D104975: Implement P1949

[Tom Honermann via Phabricator via cfe-commits]

tahonermann added a comment.

> Since it is a C++23 feature, can we at least have it only if the user 
> requests C++23 standard compliance?

WG21 adopted P1949  as a defect report (DR) 
against prior standards with the intent that the changes be applied 
retroactively.

> Isn't it strange to force it upon users if they specify -std=c++20?

This is how DRs are intended to be handled.

> I checked the latest GCC 12. Despite P1949  
> support, I can use math symbols like 𝜕 and 𝛻 without any problem with any 
> combination of -std=c++xx, xx ≠ 98, xx ≠ 03.

It looks to me like gcc implements an extension that allows additional 
characters not permitted by P1949  to still be 
used in non-pedantic mode. Relevant gcc commits include:

- 
https://github.com/gcc-mirror/gcc/commit/c4d6dcacfca1b804504515496e6d9de176d7f51e
- 
https://github.com/gcc-mirror/gcc/commit/7abcc9ca20d4e17deabb308b5f483aaccc3dc02c
- 
https://github.com/gcc-mirror/gcc/commit/c264208e161830a5642ee3125871c23110508462

> Thanks, God, there are reasonable people in the GCC team.

Please refrain from such unhelpful comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104975

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

[PATCH] D124447: [clang-tidy] Add infrastructure support for running on project-level information

[Whisperity via Phabricator via cfe-commits]

whisperity added a comment.

In D124447#3493446 , @aaron.ballman 
wrote:

> precommit CI is showing a fair amount of failures that I believe are related 
> to your patch.

It was me who helped @bahramib to rebase and split a much larger and more 
convoluted development history into broken up patches for review, and I have no 
idea what is causing the CI issues, because all the CI issues are related to 
unit test libraries removing `const` from fixits and such(??) which this patch 
(or any in the current patch set) doesn't even touch, at all. I did run the 
test targets locally... it's likely that simply the rebase and the push 
happened against an unclean/breaking main branch...

We can investigate later, but I think the review can be done either way. If 
something is related, it has to be tangential, we strived to make the entire 
new infrastructure fully backwards compatible.

The only reason I can think of is that some `LangOpts` might not be forwarded 
correctly somewhere, will definitely look into this.

(Also, full disclosure, I am the B.Sc. thesis supervisor of @bahramib so I'm 
only here to keep an eye on (and fix up the "very LLVM-y" parts) of the check, 
I cannot impartially review and accept this as I was involved in the full 
creation process. 😇)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124447

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

[PATCH] D124983: [HLSL} add -fcgl option flag.

[Chris Bieneman via Phabricator via cfe-commits]

beanz added inline comments.



Comment at: clang/include/clang/Driver/Options.td:6765
  "as_6_5, as_6_6, as_6_7">;
+def fcgl : DXCFlag<"fcgl">,
+  HelpText<"Generate high-level code only. Without any dxil related passes.">;

aaron.ballman wrote:
> Any particular reason for this name? Naively, I have no idea what this does 
> or how it relates to code generation.
I have no idea what this name is short for… code gen language?… likeness?… 
lobotomy?…

We have this flag in DXC, and it is a useful shorthand.

Don’t know if @python3kgae or @pow2clk know what the actual abbreviation is.



Comment at: clang/include/clang/Driver/Options.td:6766
+def fcgl : DXCFlag<"fcgl">,
+  HelpText<"Generate high-level code only. Without any dxil related passes.">;

aaron.ballman wrote:
> 
This really has nothing to do with DXIL as a target, and will apply to any HLSL 
target out of clang. A more accurate description might be that it disables all 
optimization and code generation passes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124983

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

[PATCH] D124221: Reimplement `__builtin_dump_struct` in Sema.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D124221#3493792 , @erichkeane 
wrote:

> FWIW, I'm in favor of the patch as it sits.
>
> As a followup: So I was thinking about the "%s" specifier for string types.  
> Assuming char-ptr types are all strings is a LITTLE dangerous, but more so 
> the way we're doing it.  Its a shame we don't have some way of setting a 
> 'max' limit to the number of characters we have for 2 reasons:
>
> 1- For safety: If the char-ptr points to non-null-terminated memory, it'll 
> stop us from just arbitrarily printing into space by limiting at least the 
> NUMBER of characters we print into nonsense.
> 2- For readability: printing a 'long' string likely makes this output look 
> like nonsense and breaks everything up.  Limiting us to only a few characters 
> is likely a good idea.
> 3- : It might discourage SOME level of 
> attempts at using this for reflection, or at least make it a little harder.
>
> What I would love would be for something like a 10 char max:
>
>   struct S {
>  char *C;
>};
>S s { "The Rest of this string is cut off"};
>print as:
>struct U20A a = {
>  .c = 0x1234 "The Rest o"
>};
>
> Sadly, I don't see something like that in printf specifiers?  Unless someone 
> smarter than me can come up with some trickery.  PERHAPS have the max-limit 
> compile-time configurable, but I don't feel strongly.

The C Standard has this in the specification of the %s format specifier:

  If no l length modifier is present, the argument shall be a pointer to 
storage of character
  type. Characters from the storage are written up to (but not including) the 
terminating
  null character. If the precision is specified, no more than that many bytes 
are written. If
  the precision is not specified or is greater than the size of the storage, 
the storage shall
  contain a null character.

So you can use the precision modifier on %s to limit the length to a particular 
number of bytes. The only downside I can think of to picking a limit is, what 
happens when the user stores valid UTF-8 data in their string and prints it via 
`%.10s` (will we then potentially be splitting a codepoint in half and that 
does something bad?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124221

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

[PATCH] D124996: [clang][preprocessor] Fix unsigned-ness of utf8 char literals

[Tom Honermann via Phabricator via cfe-commits]

tahonermann added inline comments.



Comment at: clang/lib/Lex/PPExpressions.cpp:413
   Val.setIsUnsigned(!TargetInfo::isTypeSigned(TI.getWCharType()));
-else if (!Literal.isUTF16() && !Literal.isUTF32())
+else if (!Literal.isUTF8() && !Literal.isUTF16() && !Literal.isUTF32())
   Val.setIsUnsigned(!PP.getLangOpts().CharIsSigned);

tahonermann wrote:
> I think the check for UTF-8 should also be conditioned on 
> `PP.getLangOpts().Char8`. When `char8_t` support is not enabled (as in C++17 
> or with `-fno-char8_t` in C++20), UTF-8 character literals still have type 
> `char`.
>   else if (!(Literal.isUTF8() && PP.getLangOpts().Char8) && 
> !Literal.isUTF16() && !Literal.isUTF32())
My C++ bias may be showing here; `LangOptions.Char8` may not be relevant for C, 
so this may require additional qualification.


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

https://reviews.llvm.org/D124996

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

[PATCH] D124221: Reimplement `__builtin_dump_struct` in Sema.

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D124221#3494000 , @aaron.ballman 
wrote:

> In D124221#3493792 , @erichkeane 
> wrote:
>
>> FWIW, I'm in favor of the patch as it sits.
>>
>> As a followup: So I was thinking about the "%s" specifier for string types.  
>> Assuming char-ptr types are all strings is a LITTLE dangerous, but more so 
>> the way we're doing it.  Its a shame we don't have some way of setting a 
>> 'max' limit to the number of characters we have for 2 reasons:
>>
>> 1- For safety: If the char-ptr points to non-null-terminated memory, it'll 
>> stop us from just arbitrarily printing into space by limiting at least the 
>> NUMBER of characters we print into nonsense.
>> 2- For readability: printing a 'long' string likely makes this output look 
>> like nonsense and breaks everything up.  Limiting us to only a few 
>> characters is likely a good idea.
>> 3- : It might discourage SOME level of 
>> attempts at using this for reflection, or at least make it a little harder.
>>
>> What I would love would be for something like a 10 char max:
>>
>>   struct S {
>>  char *C;
>>};
>>S s { "The Rest of this string is cut off"};
>>print as:
>>struct U20A a = {
>>  .c = 0x1234 "The Rest o"
>>};
>>
>> Sadly, I don't see something like that in printf specifiers?  Unless someone 
>> smarter than me can come up with some trickery.  PERHAPS have the max-limit 
>> compile-time configurable, but I don't feel strongly.
>
> The C Standard has this in the specification of the %s format specifier:
>
>   If no l length modifier is present, the argument shall be a pointer to 
> storage of character
>   type. Characters from the storage are written up to (but not including) the 
> terminating
>   null character. If the precision is specified, no more than that many bytes 
> are written. If
>   the precision is not specified or is greater than the size of the storage, 
> the storage shall
>   contain a null character.
>
> So you can use the precision modifier on %s to limit the length to a 
> particular number of bytes. The only downside I can think of to picking a 
> limit is, what happens when the user stores valid UTF-8 data in their string 
> and prints it via `%.10s` (will we then potentially be splitting a codepoint 
> in half and that does something bad?

Ah! TIL!  That I think would be an excellent improvement to this builtin.  I 
suspect we could just choose a fixed value to start (maybe even in this patch 
@rsmith ?), then add a compile-time option in the future if folks care to 
extend it.  I would posit that 10-15 would be a good start?  Any shorter would 
be prohibitive I think?  I could be talked into a little longer...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124221

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

[clang] ad2263d - [Sema] Replace invalid FIXME about memory leak. NFC

[Ilya Biryukov via cfe-commits]

Author: Ilya Biryukov
Date: 2022-05-05T15:04:11Z
New Revision: ad2263de9f75119626f316e253fdc1de28d5c23a

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

LOG: [Sema] Replace invalid FIXME about memory leak. NFC

Added in my previous patch by mistake.

Added: 


Modified: 
clang/lib/Sema/SemaConcept.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaConcept.cpp b/clang/lib/Sema/SemaConcept.cpp
index 4ec37a94089b0..261f57e4d185b 100644
--- a/clang/lib/Sema/SemaConcept.cpp
+++ b/clang/lib/Sema/SemaConcept.cpp
@@ -341,7 +341,7 @@ bool Sema::CheckConstraintSatisfaction(
   OutSatisfaction = *Satisfaction;
   // We cannot use InsertPos here because CheckConstraintSatisfaction might 
have
   // invalidated it.
-  // FIXME: this leaks memory, we should allocate in the arena instead.
+  // Note that entries of SatisfactionCache are deleted in Sema's destructor.
   SatisfactionCache.InsertNode(Satisfaction.release());
   return false;
 }



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

[PATCH] D125011: [MSVC] Add support for pragma alloc_text

[Stephen Long via Phabricator via cfe-commits]

steplong updated this revision to Diff 427329.
steplong added reviewers: rnk, aaron.ballman, hans.
steplong added a comment.

Update patch to error out if a function is not C linkage. Not sure how to check 
if a function has been declared


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125011

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/ParsePragma.cpp
  clang/lib/Sema/SemaAttr.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/test/CodeGen/msvc_pragma_alloc_text.cpp

Index: clang/test/CodeGen/msvc_pragma_alloc_text.cpp
===
--- /dev/null
+++ clang/test/CodeGen/msvc_pragma_alloc_text.cpp
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -fms-extensions -S -emit-llvm -o - %s | FileCheck %s
+
+extern "C" void foo();
+extern "C" void foo1();
+extern "C" void foo2();
+extern "C" void foo3();
+
+#pragma alloc_text("abc", foo, foo1)
+#pragma alloc_text("def", foo2)
+#pragma alloc_text("def", foo3)
+
+// CHECK-LABEL: define{{.*}} void @foo() {{.*}} section "abc"
+extern "C" void foo() {}
+
+// CHECK-LABEL: define{{.*}} void @foo1() {{.*}} section "abc"
+extern "C" void foo1() {}
+
+// CHECK-LABEL: define{{.*}} void @foo2() {{.*}} section "def"
+extern "C" void foo2() {}
+
+// CHECK-LABEL: define{{.*}} void @foo3() {{.*}} section "def"
+extern "C" void foo3() {}
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -10220,8 +10220,10 @@
 
   // If this is a function definition, check if we have to apply optnone due to
   // a pragma.
-  if(D.isFunctionDefinition())
+  if(D.isFunctionDefinition()) {
 AddRangeBasedOptnone(NewFD);
+AddSectionMSAllocText(NewFD);
+  }
 
   // If this is the first declaration of an extern C variable, update
   // the map of such variables.
Index: clang/lib/Sema/SemaAttr.cpp
===
--- clang/lib/Sema/SemaAttr.cpp
+++ clang/lib/Sema/SemaAttr.cpp
@@ -741,6 +741,24 @@
   CurInitSegLoc = PragmaLocation;
 }
 
+void Sema::ActOnPragmaMSAllocText(
+SourceLocation PragmaLocation,
+StringRef Section,
+const SmallVector> &Functions) {
+  if (!CurContext->getRedeclContext()->isFileContext()) {
+Diag(PragmaLocation, diag::err_pragma_alloc_text_scope);
+return;
+  }
+
+  for (auto &Function : Functions) {
+StringRef Name;
+SourceLocation Loc;
+std::tie(Name, Loc) = Function;
+
+FunctionToSectionMap[Name] = std::make_tuple(Section, Loc);
+  }
+}
+
 void Sema::ActOnPragmaUnused(const Token &IdTok, Scope *curScope,
  SourceLocation PragmaLoc) {
 
@@ -1072,6 +1090,27 @@
 AddOptnoneAttributeIfNoConflicts(FD, OptimizeOffPragmaLocation);
 }
 
+void Sema::AddSectionMSAllocText(FunctionDecl *FD) {
+  if (!FD->getIdentifier())
+return;
+
+  StringRef Name = FD->getName();
+  auto It = FunctionToSectionMap.find(Name);
+  if (It != FunctionToSectionMap.end()) {
+StringRef Section;
+SourceLocation Loc;
+std::tie(Section, Loc) = It->second;
+
+if (!FD->isExternC()) {
+  Diag(Loc, diag::err_pragma_alloc_text_c_linkage);
+  return;
+}
+
+if (!FD->hasAttr())
+  FD->addAttr(SectionAttr::CreateImplicit(Context, Section));
+  }
+}
+
 void Sema::AddOptnoneAttributeIfNoConflicts(FunctionDecl *FD,
 SourceLocation Loc) {
   // Don't add a conflicting attribute. No diagnostic is needed.
Index: clang/lib/Parse/ParsePragma.cpp
===
--- clang/lib/Parse/ParsePragma.cpp
+++ clang/lib/Parse/ParsePragma.cpp
@@ -445,6 +445,8 @@
 PP.AddPragmaHandler(MSCodeSeg.get());
 MSSection = std::make_unique("section");
 PP.AddPragmaHandler(MSSection.get());
+MSAllocText = std::make_unique("alloc_text");
+PP.AddPragmaHandler(MSAllocText.get());
 MSRuntimeChecks = std::make_unique();
 PP.AddPragmaHandler(MSRuntimeChecks.get());
 MSIntrinsic = std::make_unique();
@@ -554,6 +556,8 @@
 MSCodeSeg.reset();
 PP.RemovePragmaHandler(MSSection.get());
 MSSection.reset();
+PP.RemovePragmaHandler(MSAllocText.get());
+MSAllocText.reset();
 PP.RemovePragmaHandler(MSRuntimeChecks.get());
 MSRuntimeChecks.reset();
 PP.RemovePragmaHandler(MSIntrinsic.get());
@@ -912,7 +916,8 @@
 .Case("const_seg", &Parser::HandlePragmaMSSegment)
 .Case("code_seg", &Parser::HandlePragmaMSSegment)
 .Case("section", &Parser::HandlePragmaMSSection)
-.Case("init_seg", &Parser::HandlePragmaMSInitSeg);
+.Case("init_seg", &Parser::HandlePragmaMSInitSeg)
+.Case("alloc_text", &Parser::HandlePragmaMSAllocText);
 
   if (!(this->*Handler)(PragmaName, PragmaLocation)) {
 // Pragma han