[PATCH] D129771: [clang-format] distinguish multiplication after brace-init from pointer

2022-07-15 Thread Owen Pan via Phabricator via cfe-commits
owenpan accepted this revision.
owenpan added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:2328
+if (PrevToken->is(tok::r_brace) && Tok.is(tok::star) &&
+PrevToken->MatchingParen == nullptr)
   return TT_PointerOrReference;




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129771

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


[PATCH] D129461: [PowerPC] Support x86 compatible intrinsics on AIX

2022-07-15 Thread ChenZheng via Phabricator via cfe-commits
shchenz added inline comments.



Comment at: clang/lib/Driver/ToolChains/AIX.cpp:232
+path::remove_filename(P);
+addSystemInclude(DriverArgs, CC1Args, P);
   }

qiucf wrote:
> shchenz wrote:
> > Can we use `path::parent_path(P)` directly in `addSystemInclude()`? 
> > `remove_filename()` sounds like `ppc_wrappers` is a file.
> It just wraps `parent_path`, and `parent_path` only accepts (and returns) 
> `StringRef`.
```
addSystemInclude(DriverArgs, CC1Args, path::parent_path(P.str()));
```

Above change works for me? I think it should be functionality right?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129461

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


[PATCH] D128048: Add a new clang option "-ftime-trace="

2022-07-15 Thread dongjunduo via Phabricator via cfe-commits
dongjunduo updated this revision to Diff 444898.
dongjunduo added a comment.

[Clang] restyle code


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128048

Files:
  clang/include/clang/Driver/Options.td
  clang/include/clang/Frontend/FrontendOptions.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/check-time-trace.cpp
  clang/tools/driver/cc1_main.cpp


Index: clang/tools/driver/cc1_main.cpp
===
--- clang/tools/driver/cc1_main.cpp
+++ clang/tools/driver/cc1_main.cpp
@@ -212,7 +212,9 @@
   bool Success = CompilerInvocation::CreateFromArgs(Clang->getInvocation(),
 Argv, Diags, Argv0);
 
-  if (Clang->getFrontendOpts().TimeTrace) {
+  if (Clang->getFrontendOpts().TimeTrace ||
+  !Clang->getFrontendOpts().TimeTracePath.empty()) {
+Clang->getFrontendOpts().TimeTrace = 1;
 llvm::timeTraceProfilerInitialize(
 Clang->getFrontendOpts().TimeTraceGranularity, Argv0);
   }
@@ -256,6 +258,13 @@
   if (llvm::timeTraceProfilerEnabled()) {
 SmallString<128> Path(Clang->getFrontendOpts().OutputFile);
 llvm::sys::path::replace_extension(Path, "json");
+if (!Clang->getFrontendOpts().TimeTracePath.empty()) {
+  // replace the suffix to '.json' directly
+  SmallString<128> TracePath(Clang->getFrontendOpts().TimeTracePath);
+  if (llvm::sys::fs::is_directory(TracePath))
+llvm::sys::path::append(TracePath, llvm::sys::path::filename(Path));
+  Path.assign(TracePath);
+}
 if (auto profilerOutput = Clang->createOutputFile(
 Path.str(), /*Binary=*/false, /*RemoveFileOnSignal=*/false,
 /*useTemporary=*/false)) {
Index: clang/test/Driver/check-time-trace.cpp
===
--- clang/test/Driver/check-time-trace.cpp
+++ clang/test/Driver/check-time-trace.cpp
@@ -2,6 +2,20 @@
 // RUN: cat %T/check-time-trace.json \
 // RUN:   | %python -c 'import json, sys; 
json.dump(json.loads(sys.stdin.read()), sys.stdout, sort_keys=True, indent=2)' \
 // RUN:   | FileCheck %s
+// RUN: %clangxx -S -ftime-trace=%T/new-name.json -ftime-trace-granularity=0 
-o %T/check-time-trace %s
+// RUN: cat %T/new-name.json \
+// RUN:   | %python -c 'import json, sys; 
json.dump(json.loads(sys.stdin.read()), sys.stdout, sort_keys=True, indent=2)' \
+// RUN:   | FileCheck %s
+// RUN: mkdir %T/output1
+// RUN: %clangxx -S -ftime-trace=%T/output1 -ftime-trace-granularity=0 -o 
%T/check-time-trace %s
+// RUN: cat %T/output1/check-time-trace.json \
+// RUN:   | %python -c 'import json, sys; 
json.dump(json.loads(sys.stdin.read()), sys.stdout, sort_keys=True, indent=2)' \
+// RUN:   | FileCheck %s
+// RUN: mkdir %T/output2
+// RUN: %clangxx -S -ftime-trace=%T/output2/ -ftime-trace-granularity=0 -o 
%T/check-time-trace %s
+// RUN: cat %T/output2/check-time-trace.json \
+// RUN:   | %python -c 'import json, sys; 
json.dump(json.loads(sys.stdin.read()), sys.stdout, sort_keys=True, indent=2)' \
+// RUN:   | FileCheck %s
 
 // CHECK:  "beginningOfTime": {{[0-9]{16},}}
 // CHECK-NEXT: "traceEvents": [
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -6183,6 +6183,7 @@
   Args.AddLastArg(CmdArgs, options::OPT_ftime_report_EQ);
   Args.AddLastArg(CmdArgs, options::OPT_ftime_trace);
   Args.AddLastArg(CmdArgs, options::OPT_ftime_trace_granularity_EQ);
+  Args.AddLastArg(CmdArgs, options::OPT_ftime_trace_EQ);
   Args.AddLastArg(CmdArgs, options::OPT_ftrapv);
   Args.AddLastArg(CmdArgs, options::OPT_malign_double);
   Args.AddLastArg(CmdArgs, options::OPT_fno_temp_file);
Index: clang/include/clang/Frontend/FrontendOptions.h
===
--- clang/include/clang/Frontend/FrontendOptions.h
+++ clang/include/clang/Frontend/FrontendOptions.h
@@ -499,6 +499,9 @@
   /// Minimum time granularity (in microseconds) traced by time profiler.
   unsigned TimeTraceGranularity;
 
+  /// Path which stores the output files for -ftime-trace
+  std::string TimeTracePath;
+
 public:
   FrontendOptions()
   : DisableFree(false), RelocatablePCH(false), ShowHelp(false),
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -2828,6 +2828,15 @@
   HelpText<"Minimum time granularity (in microseconds) traced by time 
profiler">,
   Flags<[CC1Option, CoreOption]>,
   MarshallingInfoInt, "500u">;
+def ftime_trace_EQ : Joined<["-"], "ftime-trace=">, Group,
+  HelpText<"Turn on time profiler. Generates JSON file based on output 
filename. "
+   "Specify the path which stores the tracing output file.">,

[PATCH] D129678: [analyzer][NFC] Tidy up handler-functions in SymbolicRangeInferrer

2022-07-15 Thread Gabor Marton via Phabricator via cfe-commits
martong accepted this revision.
martong added a comment.

In D129678#3652859 , @ASDenysPetrov 
wrote:

> Fixed a typo that caused `constraint_manager_negate.c` and `unary-sym-expr.c` 
> tests crashes.

Good. Still LGTM.


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

https://reviews.llvm.org/D129678

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


[PATCH] D129498: [analyzer] Add new function `clang_analyzer_value` to ExprInspectionChecker

2022-07-15 Thread 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


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

https://reviews.llvm.org/D129498

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


[PATCH] D102107: [OpenMP] Codegen aggregate for outlined function captures

2022-07-15 Thread Dhruva Chakrabarti via Phabricator via cfe-commits
dhruvachak added a comment.

In D102107#3648219 , @jdoerfert wrote:

> 



> Can you share the output of the AST dump tests and the new check lines, so 
> what run produces and the file we give to Filechec to verify it.

I looked at the AST test output and the CHECK lines more carefully. Turns out 
the full path was embedded in some of the CHECK lines causing the failures. I 
corrected those manually and those AST tests now pass. I will move on to the 
other failures.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102107

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


[PATCH] D129464: [Clang][CodeGen] Set FP options of builder at entry to compound statement

2022-07-15 Thread Serge Pavlov via Phabricator via cfe-commits
sepavloff added inline comments.



Comment at: clang/test/CodeGen/pragma-fenv_access.cpp:35
+}
+
+

aaron.ballman wrote:
> There are some extra test cases I'd like to see coverage for because there 
> are some interesting edge cases to consider.
> ```
> template 
> float func1(Ty) {
>   float f1 = 1.0f, f2 = 3.0f;
>   return f1 + f2 * 2.0f;
> }
> 
> #pragma float_control(precise, on, push)
> template float func1(int); 
> #pragma float_control(pop)
> 
> #pragma float_control(precise, on, push)
> template 
> float func2(Ty) {
>   float f1 = 1.0f, f2 = 3.0f;
>   return f1 + f2 * 2.0f;
> }
> #pragma float_control(pop)
> 
> template float func2(int);
> 
> void bar() {
> func1(1.1);
> func2(1.1);
> }
> ```
> This gets especially interesting when you think about delayed template 
> instantiation as happens by default on Windows targets. Consider this code 
> with the *driver level* `-ffast-math` flag enabled (not the cc1 option, which 
> is different).
> 
> I think that `func1` SHOULD be precise, because the explicit 
> instantiation is, while `func1` SHOULD NOT be precise, because the 
> definition is not. `func2` SHOULD NOT be precise, because the explicit 
> instantiation is not, while `func2` SHOULD be precise,  because the 
> definition is.
> 
> Partial specializations are a similar situation where the primary template 
> and its related code made have different options.
> 
> WDYT?
Standard FP pragmas are defined only in C standard, so interaction of them with 
C++ specific features is actually implementation-defined. The cases presented 
in your example are reasonable solutions with one exception: IMO `func2` 
should be precise, because its template is precise. It is equivalent to:
```
template 
float func2(Ty) {
#pragma float_control(precise, on)
  float f1 = 1.0f, f2 = 3.0f;
  return f1 + f2 * 2.0f;
}
``` 
so instantiation of it would produce function with precise operations.

Implementation of correct mechanism of the interaction requires substantial 
efforts and should be made in a separate patch, I think. In particular, we need 
to invent a way to associate a point of instantiation with the FPOptions in 
that point, so that delayed instantiation could be made with correct set of 
options.

In this patch the change in SemaTemplateInstantiateDecl.cpp prevents from 
compiler crash. Without it codegen tries  to create a call to constrained 
intrinsic in the function that do not have attribute StrictFP, because flag 
FEnvAccess is set at the end of translation unit in `pragma-fenv_access.cpp`.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129464

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


[clang] 2f1555f - [C++20] [Modules] Handle reachability for enum class

2022-07-15 Thread Chuanqi Xu via cfe-commits

Author: Chuanqi Xu
Date: 2022-07-15T15:57:04+08:00
New Revision: 2f1555fb11d74376347ee94eb3af258e047f68ce

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

LOG: [C++20] [Modules] Handle reachability for enum class

In previous reachability patch, we missed the case for enum class.
Trying to handle it in this patch and add the corresponding tests.

Added: 
clang/test/Modules/enum-class.cppm

Modified: 
clang/lib/Sema/SemaType.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp
index 3edce941c3817..3ab5d26a9a750 100644
--- a/clang/lib/Sema/SemaType.cpp
+++ b/clang/lib/Sema/SemaType.cpp
@@ -8669,12 +8669,13 @@ bool Sema::hasAcceptableDefinition(NamedDecl *D, 
NamedDecl **Suggested,
   // of it will do.
   *Suggested = nullptr;
   for (auto *Redecl : ED->redecls()) {
-if (isVisible(Redecl))
+if (isAcceptable(Redecl, Kind))
   return true;
 if (Redecl->isThisDeclarationADefinition() ||
 (Redecl->isCanonicalDecl() && !*Suggested))
   *Suggested = Redecl;
   }
+
   return false;
 }
 D = ED->getDefinition();

diff  --git a/clang/test/Modules/enum-class.cppm 
b/clang/test/Modules/enum-class.cppm
new file mode 100644
index 0..01ae8c0d8814d
--- /dev/null
+++ b/clang/test/Modules/enum-class.cppm
@@ -0,0 +1,26 @@
+// Checks for reachability for C++11 enum class properly
+//
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -std=c++20 %t/A.cppm -emit-module-interface -o %t/A.pcm
+// RUN: %clang_cc1 -std=c++20 -fprebuilt-module-path=%t %t/Use.cpp -verify 
-fsyntax-only
+
+//--- foo.h
+enum class foo {
+a, b, c
+};
+
+//--- A.cppm
+module;
+#include "foo.h"
+export module A;
+export foo func();
+
+//--- Use.cpp
+// expected-no-diagnostics
+import A;
+void bar() {
+auto f = func();
+}



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


[clang] 2a72137 - [IR] Don't use blockaddresses as callbr arguments

2022-07-15 Thread Nikita Popov via cfe-commits

Author: Nikita Popov
Date: 2022-07-15T10:18:17+02:00
New Revision: 2a721374aef326d4668f750d341c86d1aa1a0309

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

LOG: [IR] Don't use blockaddresses as callbr arguments

Following some recent discussions, this changes the representation
of callbrs in IR. The current blockaddress arguments are replaced
with `!` label constraints that refer directly to callbr indirect
destinations:

; Before:
%res = callbr i8* asm "", "=r,r,i"(i8* %x, i8* blockaddress(@test8, %foo))
to label %asm.fallthrough [label %foo]
; After:
%res = callbr i8* asm "", "=r,r,!i"(i8* %x)
to label %asm.fallthrough [label %foo]

The benefit of this is that we can easily update the successors of
a callbr, without having to worry about also updating blockaddress
references. This should allow us to remove some limitations:

* Allow unrolling/peeling/rotation of callbr, or any other
  clone-based optimizations
  (https://github.com/llvm/llvm-project/issues/41834)
* Allow duplicate successors
  (https://github.com/llvm/llvm-project/issues/45248)

This is just the IR representation change though, I will follow up
with patches to remove limtations in various transformation passes
that are no longer needed.

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

Added: 


Modified: 
clang/lib/CodeGen/CGStmt.cpp
clang/test/CodeGen/asm-goto.c
clang/test/CodeGen/asm.c
clang/test/Modules/asm-goto.c
llvm/docs/LangRef.rst
llvm/docs/ReleaseNotes.rst
llvm/include/llvm/IR/InlineAsm.h
llvm/include/llvm/IR/Instructions.h
llvm/lib/Bitcode/Reader/BitcodeReader.cpp
llvm/lib/CodeGen/GlobalISel/InlineAsmLowering.cpp
llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
llvm/lib/IR/InlineAsm.cpp
llvm/lib/IR/Instructions.cpp
llvm/lib/IR/Verifier.cpp
llvm/test/Analysis/BasicAA/pr52735.ll
llvm/test/Assembler/call-arg-is-callee.ll
llvm/test/Assembler/inline-asm-constraint-error.ll
llvm/test/Bitcode/callbr.ll
llvm/test/CodeGen/AArch64/callbr-asm-label.ll
llvm/test/CodeGen/AArch64/callbr-asm-obj-file.ll
llvm/test/CodeGen/AArch64/speculation-hardening-sls.ll
llvm/test/CodeGen/ARM/speculation-hardening-sls.ll
llvm/test/CodeGen/PowerPC/ppc64-inlineasm-clobber.ll
llvm/test/CodeGen/SystemZ/asm-20.ll
llvm/test/CodeGen/X86/callbr-asm-bb-exports.ll
llvm/test/CodeGen/X86/callbr-asm-blockplacement.ll
llvm/test/CodeGen/X86/callbr-asm-branch-folding.ll
llvm/test/CodeGen/X86/callbr-asm-destinations.ll
llvm/test/CodeGen/X86/callbr-asm-instr-scheduling.ll
llvm/test/CodeGen/X86/callbr-asm-kill.mir
llvm/test/CodeGen/X86/callbr-asm-label-addr.ll
llvm/test/CodeGen/X86/callbr-asm-obj-file.ll
llvm/test/CodeGen/X86/callbr-asm-outputs-pred-succ.ll
llvm/test/CodeGen/X86/callbr-asm-outputs.ll
llvm/test/CodeGen/X86/callbr-asm-phi-placement.ll
llvm/test/CodeGen/X86/callbr-asm-sink.ll
llvm/test/CodeGen/X86/callbr-asm.ll
llvm/test/CodeGen/X86/callbr-codegenprepare.ll
llvm/test/CodeGen/X86/inline-asm-pic.ll
llvm/test/CodeGen/X86/shrinkwrap-callbr.ll
llvm/test/CodeGen/X86/speculation-hardening-sls.ll
llvm/test/CodeGen/X86/tail-dup-asm-goto.ll
llvm/test/Instrumentation/MemorySanitizer/msan_asm_conservative.ll
llvm/test/Transforms/CallSiteSplitting/callsite-split-callbr.ll
llvm/test/Transforms/CodeExtractor/PartialInlinePGOMultiRegion.ll
llvm/test/Transforms/Coroutines/coro-debug.ll
llvm/test/Transforms/GVN/callbr-loadpre-critedge.ll
llvm/test/Transforms/GVN/callbr-scalarpre-critedge.ll
llvm/test/Transforms/GVN/critical-edge-split-failure.ll
llvm/test/Transforms/IROutliner/illegal-callbr.ll
llvm/test/Transforms/Inline/blockaddress.ll
llvm/test/Transforms/Inline/callbr.ll
llvm/test/Transforms/JumpThreading/callbr-edge-split.ll
llvm/test/Transforms/JumpThreading/pr46857-callbr.ll
llvm/test/Transforms/LICM/callbr-crash.ll
llvm/test/Transforms/LoopDeletion/two-predecessors.ll
llvm/test/Transforms/LoopRotate/callbr.ll
llvm/test/Transforms/LoopStrengthReduce/callbr-critical-edge-splitting.ll
llvm/test/Transforms/LoopStrengthReduce/callbr-critical-edge-splitting2.ll
llvm/test/Transforms/LoopStrengthReduce/remove_scev_indvars.ll
llvm/test/Transforms/LoopUnroll/callbr.ll
llvm/test/Transforms/PGOProfile/callbr.ll
llvm/test/Transforms/SimpleLoopUnswitch/not-safe-to-clone.ll
llvm/test/Transforms/SimplifyCFG/callbr-destinations.ll
llvm/test/Transforms/SimplifyCFG/jump-threading.ll
llvm/test/Verifier/callbr.ll
llvm/test/Verifier/dominates.ll
llvm/test/tools/llvm-diff/callbr.ll
llvm/test/tools/llvm-diff/phinode.ll

llvm/test/tools/llvm-reduce/remove-function-arguments-of-funcs-used

[PATCH] D129466: [clang-format][NFC] Replace most of std::vector with SmallVector

2022-07-15 Thread Owen Pan via Phabricator via cfe-commits
owenpan added inline comments.



Comment at: clang/lib/Format/ContinuationIndenter.cpp:40
 static unsigned getLengthToMatchingParen(const FormatToken &Tok,
- const std::vector &Stack) 
{
+ const SmallVector &Stack) 
{
   // Normally whether or not a break before T is possible is calculated and

curdeius wrote:
> Sorry for a late comment. I think that in a parameter list, we should use 
> `SmallVectorBase` to avoid changing the type if the inplace element count 
> changes. That's not important now though.
Hmm. I can't find an example of `SmallVectorBase` in llvm-project sources, 
though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129466

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


[PATCH] D129288: [IR] Don't use blockaddresses as callbr arguments

2022-07-15 Thread Nikita Popov via 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 rG2a721374aef3: [IR] Don't use blockaddresses as callbr 
arguments (authored by nikic).
Herald added a reviewer: bollu.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D129288?vs=443898&id=444908#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129288

Files:
  clang/lib/CodeGen/CGStmt.cpp
  clang/test/CodeGen/asm-goto.c
  clang/test/CodeGen/asm.c
  clang/test/Modules/asm-goto.c
  llvm/docs/LangRef.rst
  llvm/docs/ReleaseNotes.rst
  llvm/include/llvm/IR/InlineAsm.h
  llvm/include/llvm/IR/Instructions.h
  llvm/lib/Bitcode/Reader/BitcodeReader.cpp
  llvm/lib/CodeGen/GlobalISel/InlineAsmLowering.cpp
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
  llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
  llvm/lib/IR/InlineAsm.cpp
  llvm/lib/IR/Instructions.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/test/Analysis/BasicAA/pr52735.ll
  llvm/test/Assembler/call-arg-is-callee.ll
  llvm/test/Assembler/inline-asm-constraint-error.ll
  llvm/test/Bitcode/callbr.ll
  llvm/test/CodeGen/AArch64/callbr-asm-label.ll
  llvm/test/CodeGen/AArch64/callbr-asm-obj-file.ll
  llvm/test/CodeGen/AArch64/speculation-hardening-sls.ll
  llvm/test/CodeGen/ARM/speculation-hardening-sls.ll
  llvm/test/CodeGen/PowerPC/ppc64-inlineasm-clobber.ll
  llvm/test/CodeGen/SystemZ/asm-20.ll
  llvm/test/CodeGen/X86/callbr-asm-bb-exports.ll
  llvm/test/CodeGen/X86/callbr-asm-blockplacement.ll
  llvm/test/CodeGen/X86/callbr-asm-branch-folding.ll
  llvm/test/CodeGen/X86/callbr-asm-destinations.ll
  llvm/test/CodeGen/X86/callbr-asm-errors.ll
  llvm/test/CodeGen/X86/callbr-asm-instr-scheduling.ll
  llvm/test/CodeGen/X86/callbr-asm-kill.mir
  llvm/test/CodeGen/X86/callbr-asm-label-addr.ll
  llvm/test/CodeGen/X86/callbr-asm-obj-file.ll
  llvm/test/CodeGen/X86/callbr-asm-outputs-pred-succ.ll
  llvm/test/CodeGen/X86/callbr-asm-outputs.ll
  llvm/test/CodeGen/X86/callbr-asm-phi-placement.ll
  llvm/test/CodeGen/X86/callbr-asm-sink.ll
  llvm/test/CodeGen/X86/callbr-asm.ll
  llvm/test/CodeGen/X86/callbr-codegenprepare.ll
  llvm/test/CodeGen/X86/inline-asm-pic.ll
  llvm/test/CodeGen/X86/shrinkwrap-callbr.ll
  llvm/test/CodeGen/X86/speculation-hardening-sls.ll
  llvm/test/CodeGen/X86/tail-dup-asm-goto.ll
  llvm/test/Instrumentation/MemorySanitizer/msan_asm_conservative.ll
  llvm/test/Transforms/CallSiteSplitting/callsite-split-callbr.ll
  llvm/test/Transforms/CodeExtractor/PartialInlinePGOMultiRegion.ll
  llvm/test/Transforms/Coroutines/coro-debug.ll
  llvm/test/Transforms/GVN/callbr-loadpre-critedge.ll
  llvm/test/Transforms/GVN/callbr-scalarpre-critedge.ll
  llvm/test/Transforms/GVN/critical-edge-split-failure.ll
  llvm/test/Transforms/IROutliner/illegal-callbr.ll
  llvm/test/Transforms/Inline/blockaddress.ll
  llvm/test/Transforms/Inline/callbr.ll
  llvm/test/Transforms/JumpThreading/callbr-edge-split.ll
  llvm/test/Transforms/JumpThreading/pr46857-callbr.ll
  llvm/test/Transforms/LICM/callbr-crash.ll
  llvm/test/Transforms/LoopDeletion/two-predecessors.ll
  llvm/test/Transforms/LoopRotate/callbr.ll
  llvm/test/Transforms/LoopStrengthReduce/callbr-critical-edge-splitting.ll
  llvm/test/Transforms/LoopStrengthReduce/callbr-critical-edge-splitting2.ll
  llvm/test/Transforms/LoopStrengthReduce/remove_scev_indvars.ll
  llvm/test/Transforms/LoopUnroll/callbr.ll
  llvm/test/Transforms/PGOProfile/callbr.ll
  llvm/test/Transforms/SimpleLoopUnswitch/not-safe-to-clone.ll
  llvm/test/Transforms/SimplifyCFG/callbr-destinations.ll
  llvm/test/Transforms/SimplifyCFG/jump-threading.ll
  llvm/test/Verifier/callbr.ll
  llvm/test/Verifier/dominates.ll
  llvm/test/tools/llvm-diff/callbr.ll
  llvm/test/tools/llvm-diff/phinode.ll
  
llvm/test/tools/llvm-reduce/remove-function-arguments-of-funcs-used-in-blockaddress.ll
  llvm/unittests/IR/InstructionsTest.cpp
  polly/test/ScopDetect/callbr.ll

Index: polly/test/ScopDetect/callbr.ll
===
--- polly/test/ScopDetect/callbr.ll
+++ polly/test/ScopDetect/callbr.ll
@@ -12,7 +12,7 @@
 
 define void @func(i32 %n, double* noalias nonnull %A) {
 entry:
-  callbr void asm sideeffect "", "X,~{dirflag},~{fpsr},~{flags}"(i8* blockaddress(@func, %for)) #1
+  callbr void asm sideeffect "", "!i,~{dirflag},~{fpsr},~{flags}"() #1
   to label %fallthrough [label %for]
 
 fallthrough:
Index: llvm/unittests/IR/InstructionsTest.cpp
===
--- llvm/unittests/IR/InstructionsTest.cpp
+++ llvm/unittests/IR/InstructionsTest.cpp
@@ -1498,7 +1498,7 @@
   std::unique_ptr M = parseIR(Context, R"(
 define void @foo() {
 entry:
-  callbr void asm sideeffect "// XXX: ${0:l}", "X"(i8* blockaddress(@foo, %branch_test.exit))
+  callbr void a

[clang] 3c849d0 - Modernize Optional::{getValueOr,hasValue}

2022-07-15 Thread Fangrui Song via cfe-commits

Author: Fangrui Song
Date: 2022-07-15T01:20:39-07:00
New Revision: 3c849d0aefa1101cf38586449c2105a97797c414

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

LOG: Modernize Optional::{getValueOr,hasValue}

Added: 


Modified: 
clang-tools-extra/clangd/AST.cpp
clang/lib/Analysis/FlowSensitive/DebugSupport.cpp
clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
llvm/include/llvm/Support/Casting.h
llvm/lib/Target/LoongArch/LoongArchTargetMachine.cpp
mlir/include/mlir/IR/OpImplementation.h
mlir/lib/Dialect/Vector/Transforms/VectorTransforms.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/AST.cpp 
b/clang-tools-extra/clangd/AST.cpp
index 6ceff56115721..4df043d18cfb7 100644
--- a/clang-tools-extra/clangd/AST.cpp
+++ b/clang-tools-extra/clangd/AST.cpp
@@ -942,7 +942,7 @@ resolveForwardingParameters(const FunctionDecl *D, unsigned 
MaxDepth) {
   TailIt = std::copy(Info.Tail.rbegin(), Info.Tail.rend(), TailIt);
   // Prepare next recursion level
   Pack = Info.Pack;
-  CurrentFunction = Info.PackTarget.getValueOr(nullptr);
+  CurrentFunction = Info.PackTarget.value_or(nullptr);
   Depth++;
   // If we are recursing into a previously encountered function: Abort
   if (CurrentFunction) {

diff  --git a/clang/lib/Analysis/FlowSensitive/DebugSupport.cpp 
b/clang/lib/Analysis/FlowSensitive/DebugSupport.cpp
index 305d9d346089a..a2895d0197b1d 100644
--- a/clang/lib/Analysis/FlowSensitive/DebugSupport.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DebugSupport.cpp
@@ -103,8 +103,7 @@ Constraints
 
 auto StatusString = debugString(Result.getStatus());
 auto Solution = Result.getSolution();
-auto SolutionString =
-Solution.hasValue() ? "\n" + debugString(Solution.value()) : "";
+auto SolutionString = Solution ? "\n" + debugString(Solution.value()) : "";
 
 return formatv(
 Template,

diff  --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp 
b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
index 825b11adad405..08fac9fb2e698 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -504,7 +504,7 @@ void ExprEngine::handleConstructor(const Expr *E,
 
 unsigned Idx = 0;
 if (CE->getType()->isArrayType()) {
-  Idx = getIndexOfElementToConstruct(State, CE, LCtx).getValueOr(0u);
+  Idx = getIndexOfElementToConstruct(State, CE, LCtx).value_or(0u);
   State = setIndexOfElementToConstruct(State, CE, LCtx, Idx + 1);
 }
 

diff  --git a/lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp 
b/lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
index 2c44683ab1356..f89936445b2ba 100644
--- a/lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
+++ b/lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
@@ -124,7 +124,7 @@ Error IntelPTCollector::TraceStart(const 
TraceIntelPTStartRequest &request) {
 
   // We try to use cgroup filtering whenever possible
   Optional cgroup_fd;
-  if (!request.disable_cgroup_filtering.getValueOr(false))
+  if (!request.disable_cgroup_filtering.value_or(false))
 cgroup_fd = GetCGroupFileDescriptor(m_process.GetID());
 
   if (Expected trace =

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index cbc24b1550c7c..8ee709db9cdb5 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -3475,10 +3475,9 @@ VariableSP SymbolFileDWARF::ParseVariableDIE(const 
SymbolContext &sc,
 
   if (use_type_size_for_value && type_sp->GetType()) {
 DWARFExpression *location = location_list.GetMutableExpressionAtAddress();
-location->UpdateValue(
-const_value_form.Unsigned(),
-type_sp->GetType()->GetByteSize(nullptr).getValueOr(0),
-die.GetCU()->GetAddressByteSize());
+location->UpdateValue(const_value_form.Unsigned(),
+  type_sp->GetType()->GetByteSize(nullptr).value_or(0),
+  die.GetCU()->GetAddressByteSize());
   }
 
   return std::make_shared(

diff  --git a/llvm/include/llvm/Support/Casting.h 
b/llvm/include/llvm/Support/Casting.h
index 5444d777b7493..b6bbff8ada10b 100644
--- a/llvm/include/llvm/Support/Casting.h
+++ b/llvm/include/llvm/Support/Casting.h
@@ -265,7 +265,7 @@ struct CastIsPossible {
 template 
 struct CastIsPossible> {
   static inline bool isPossible(const Optional &f) {
-assert(f.hasValue() && "CastIsPossible::isPossible called on a nullopt!");
+assert(f && "CastIsPossible::isPossible called on

[clang] 263dcf4 - [syntax] Introduce a TokenManager interface.

2022-07-15 Thread Haojian Wu via cfe-commits

Author: Haojian Wu
Date: 2022-07-15T10:30:37+02:00
New Revision: 263dcf452fa04d98456df04d2c3a753ba6d916ab

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

LOG: [syntax] Introduce a TokenManager interface.

TokenManager defines Token interfaces for the clang syntax-tree. This is the 
level
of abstraction that the syntax-tree should use to operate on Tokens.

It decouples the syntax-tree from a particular token implementation (TokenBuffer
previously).  This enables us to use a different underlying token implementation
for the syntax Leaf node -- in clang pseudoparser, we want to produce a
syntax-tree with its own pseudo::Token rather than syntax::Token.

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

Added: 
clang/include/clang/Tooling/Syntax/TokenBufferTokenManager.h
clang/include/clang/Tooling/Syntax/TokenManager.h
clang/lib/Tooling/Syntax/TokenBufferTokenManager.cpp

Modified: 
clang-tools-extra/clangd/SemanticSelection.cpp
clang/include/clang/Tooling/Syntax/BuildTree.h
clang/include/clang/Tooling/Syntax/Mutations.h
clang/include/clang/Tooling/Syntax/Nodes.h
clang/include/clang/Tooling/Syntax/Tokens.h
clang/include/clang/Tooling/Syntax/Tree.h
clang/lib/Tooling/Syntax/BuildTree.cpp
clang/lib/Tooling/Syntax/CMakeLists.txt
clang/lib/Tooling/Syntax/ComputeReplacements.cpp
clang/lib/Tooling/Syntax/Mutations.cpp
clang/lib/Tooling/Syntax/Synthesis.cpp
clang/lib/Tooling/Syntax/Tree.cpp
clang/tools/clang-check/ClangCheck.cpp
clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
clang/unittests/Tooling/Syntax/MutationsTest.cpp
clang/unittests/Tooling/Syntax/SynthesisTest.cpp
clang/unittests/Tooling/Syntax/TreeTest.cpp
clang/unittests/Tooling/Syntax/TreeTestBase.cpp
clang/unittests/Tooling/Syntax/TreeTestBase.h

Removed: 




diff  --git a/clang-tools-extra/clangd/SemanticSelection.cpp 
b/clang-tools-extra/clangd/SemanticSelection.cpp
index 55f5816ed90b8..f118f3ec04b08 100644
--- a/clang-tools-extra/clangd/SemanticSelection.cpp
+++ b/clang-tools-extra/clangd/SemanticSelection.cpp
@@ -16,6 +16,7 @@
 #include "clang/Basic/SourceManager.h"
 #include "clang/Tooling/Syntax/BuildTree.h"
 #include "clang/Tooling/Syntax/Nodes.h"
+#include "clang/Tooling/Syntax/TokenBufferTokenManager.h"
 #include "clang/Tooling/Syntax/Tree.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/Support/Casting.h"
@@ -52,8 +53,9 @@ llvm::Optional toFoldingRange(SourceRange SR,
   return Range;
 }
 
-llvm::Optional extractFoldingRange(const syntax::Node *Node,
- const SourceManager &SM) {
+llvm::Optional
+extractFoldingRange(const syntax::Node *Node,
+const syntax::TokenBufferTokenManager &TM) {
   if (const auto *Stmt = dyn_cast(Node)) {
 const auto *LBrace = cast_or_null(
 Stmt->findChild(syntax::NodeRole::OpenParen));
@@ -65,9 +67,12 @@ llvm::Optional extractFoldingRange(const 
syntax::Node *Node,
 if (!LBrace || !RBrace)
   return llvm::None;
 // Fold the entire range within braces, including whitespace.
-const SourceLocation LBraceLocInfo = LBrace->getToken()->endLocation(),
- RBraceLocInfo = RBrace->getToken()->location();
-auto Range = toFoldingRange(SourceRange(LBraceLocInfo, RBraceLocInfo), SM);
+const SourceLocation LBraceLocInfo =
+ TM.getToken(LBrace->getTokenKey())->endLocation(),
+ RBraceLocInfo =
+ TM.getToken(RBrace->getTokenKey())->location();
+auto Range = toFoldingRange(SourceRange(LBraceLocInfo, RBraceLocInfo),
+TM.sourceManager());
 // Do not generate folding range for compound statements without any
 // nodes and newlines.
 if (Range && Range->startLine != Range->endLine)
@@ -77,15 +82,16 @@ llvm::Optional extractFoldingRange(const 
syntax::Node *Node,
 }
 
 // Traverse the tree and collect folding ranges along the way.
-std::vector collectFoldingRanges(const syntax::Node *Root,
-   const SourceManager &SM) {
+std::vector
+collectFoldingRanges(const syntax::Node *Root,
+ const syntax::TokenBufferTokenManager &TM) {
   std::queue Nodes;
   Nodes.push(Root);
   std::vector Result;
   while (!Nodes.empty()) {
 const syntax::Node *Node = Nodes.front();
 Nodes.pop();
-const auto Range = extractFoldingRange(Node, SM);
+const auto Range = extractFoldingRange(Node, TM);
 if (Range)
   Result.push_back(*Range);
 if (const auto *T = dyn_cast(Node))
@@ -157,9 +163,11 @@ llvm::Expected getSemanticRanges(ParsedAST 
&AST, Position Pos) {
 // control flow statement bodies).
 // Related 

[PATCH] D128411: [syntax] Introduce a TokenManager interface.

2022-07-15 Thread Haojian Wu via 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 rG263dcf452fa0: [syntax] Introduce a TokenManager interface. 
(authored by hokein).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128411

Files:
  clang-tools-extra/clangd/SemanticSelection.cpp
  clang/include/clang/Tooling/Syntax/BuildTree.h
  clang/include/clang/Tooling/Syntax/Mutations.h
  clang/include/clang/Tooling/Syntax/Nodes.h
  clang/include/clang/Tooling/Syntax/TokenBufferTokenManager.h
  clang/include/clang/Tooling/Syntax/TokenManager.h
  clang/include/clang/Tooling/Syntax/Tokens.h
  clang/include/clang/Tooling/Syntax/Tree.h
  clang/lib/Tooling/Syntax/BuildTree.cpp
  clang/lib/Tooling/Syntax/CMakeLists.txt
  clang/lib/Tooling/Syntax/ComputeReplacements.cpp
  clang/lib/Tooling/Syntax/Mutations.cpp
  clang/lib/Tooling/Syntax/Synthesis.cpp
  clang/lib/Tooling/Syntax/TokenBufferTokenManager.cpp
  clang/lib/Tooling/Syntax/Tree.cpp
  clang/tools/clang-check/ClangCheck.cpp
  clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
  clang/unittests/Tooling/Syntax/MutationsTest.cpp
  clang/unittests/Tooling/Syntax/SynthesisTest.cpp
  clang/unittests/Tooling/Syntax/TreeTest.cpp
  clang/unittests/Tooling/Syntax/TreeTestBase.cpp
  clang/unittests/Tooling/Syntax/TreeTestBase.h

Index: clang/unittests/Tooling/Syntax/TreeTestBase.h
===
--- clang/unittests/Tooling/Syntax/TreeTestBase.h
+++ clang/unittests/Tooling/Syntax/TreeTestBase.h
@@ -17,6 +17,7 @@
 #include "clang/Frontend/CompilerInvocation.h"
 #include "clang/Testing/TestClangConfig.h"
 #include "clang/Tooling/Syntax/Nodes.h"
+#include "clang/Tooling/Syntax/TokenBufferTokenManager.h"
 #include "clang/Tooling/Syntax/Tokens.h"
 #include "clang/Tooling/Syntax/Tree.h"
 #include "llvm/ADT/StringRef.h"
@@ -51,6 +52,7 @@
   std::shared_ptr Invocation;
   // Set after calling buildTree().
   std::unique_ptr TB;
+  std::unique_ptr TM;
   std::unique_ptr Arena;
 };
 
Index: clang/unittests/Tooling/Syntax/TreeTestBase.cpp
===
--- clang/unittests/Tooling/Syntax/TreeTestBase.cpp
+++ clang/unittests/Tooling/Syntax/TreeTestBase.cpp
@@ -35,13 +35,14 @@
 using namespace clang::syntax;
 
 namespace {
-ArrayRef tokens(syntax::Node *N) {
+ArrayRef tokens(syntax::Node *N,
+   const TokenBufferTokenManager &STM) {
   assert(N->isOriginal() && "tokens of modified nodes are not well-defined");
   if (auto *L = dyn_cast(N))
-return llvm::makeArrayRef(L->getToken(), 1);
+return llvm::makeArrayRef(STM.getToken(L->getTokenKey()), 1);
   auto *T = cast(N);
-  return llvm::makeArrayRef(T->findFirstLeaf()->getToken(),
-T->findLastLeaf()->getToken() + 1);
+  return llvm::makeArrayRef(STM.getToken(T->findFirstLeaf()->getTokenKey()),
+STM.getToken(T->findLastLeaf()->getTokenKey()) + 1);
 }
 } // namespace
 
@@ -70,23 +71,26 @@
   public:
 BuildSyntaxTree(syntax::TranslationUnit *&Root,
 std::unique_ptr &TB,
+std::unique_ptr &TM,
 std::unique_ptr &Arena,
 std::unique_ptr Tokens)
-: Root(Root), TB(TB), Arena(Arena), Tokens(std::move(Tokens)) {
+: Root(Root), TB(TB), TM(TM), Arena(Arena), Tokens(std::move(Tokens)) {
   assert(this->Tokens);
 }
 
 void HandleTranslationUnit(ASTContext &Ctx) override {
   TB = std::make_unique(std::move(*Tokens).consume());
   Tokens = nullptr; // make sure we fail if this gets called twice.
-  Arena = std::make_unique(Ctx.getSourceManager(),
-  Ctx.getLangOpts(), *TB);
-  Root = syntax::buildSyntaxTree(*Arena, Ctx);
+  TM = std::make_unique(
+  *TB, Ctx.getLangOpts(), Ctx.getSourceManager());
+  Arena = std::make_unique();
+  Root = syntax::buildSyntaxTree(*Arena, *TM, Ctx);
 }
 
   private:
 syntax::TranslationUnit *&Root;
 std::unique_ptr &TB;
+std::unique_ptr &TM;
 std::unique_ptr &Arena;
 std::unique_ptr Tokens;
   };
@@ -94,21 +98,23 @@
   class BuildSyntaxTreeAction : public ASTFrontendAction {
   public:
 BuildSyntaxTreeAction(syntax::TranslationUnit *&Root,
+  std::unique_ptr &TM,
   std::unique_ptr &TB,
   std::unique_ptr &Arena)
-: Root(Root), TB(TB), Arena(Arena) {}
+: Root(Root), TM(TM), TB(TB), Arena(Arena) {}
 
 std::unique_ptr CreateASTConsumer(CompilerInstance &CI,
StringRef InFile) override {
   // We start recording the tokens, ast consumer will take on the result.
   auto Tokens =
   std::make_unique(CI.getPreprocessor());
-

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

2022-07-15 Thread Manas Gupta via Phabricator via cfe-commits
manas updated this revision to Diff 444911.
manas added a comment.

Add concrete tests for same and different types


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112621

Files:
  clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
  clang/test/Analysis/constant-folding.c

Index: clang/test/Analysis/constant-folding.c
===
--- clang/test/Analysis/constant-folding.c
+++ clang/test/Analysis/constant-folding.c
@@ -281,3 +281,71 @@
 clang_analyzer_eval((b % a) < x + 10); // expected-warning{{TRUE}}
   }
 }
+
+void testDisequalityRules(unsigned int u1, unsigned int u2, unsigned int u3,
+  int s1, int s2, int s3) {
+
+  // Checks for concrete value with same type
+  if (u1 > 1 && u1 < 3 && u2 > 1 && u2 < 3) {
+// u1: [2, 2], u2: [2, 2]
+clang_analyzer_eval(u1 != u2); // expected-warning{{FALSE}}
+  }
+
+  // Check for concrete value with different types
+  if (u1 > 4 && u1 < 6 && s1 > 4 && s1 < 6) {
+// u1: [5, 5], s1: [5, 5]
+clang_analyzer_eval(u1 != s1); // expected-warning{{FALSE}}
+  }
+
+  // Checks when ranges are not overlapping
+  if (u1 <= 10 && u2 >= 20) {
+// u1: [0,10], u2: [20,UINT_MAX]
+clang_analyzer_eval(u1 != u2); // expected-warning{{TRUE}}
+  }
+
+  if (s1 <= INT_MIN + 10 && s2 >= INT_MAX - 10) {
+// s1: [INT_MIN,INT_MIN + 10], s2: [INT_MAX - 10,INT_MAX]
+clang_analyzer_eval(s1 != s2); // expected-warning{{TRUE}}
+  }
+
+  // Checks when ranges are completely overlapping and have more than one point
+  if (u1 >= 20 && u1 <= 50 && u2 >= 20 && u2 <= 50) {
+// u1: [20,50], u2: [20,50]
+clang_analyzer_eval(u1 != u2); // expected-warning{{UNKNOWN}}
+  }
+
+  if (s1 >= -20 && s1 <= 20 && s2 >= -20 && s2 <= 20) {
+// s1: [-20,20], s2: [-20,20]
+clang_analyzer_eval(s1 != s2); // expected-warning{{UNKNOWN}}
+  }
+
+  // Checks when ranges are partially overlapping
+  if (u1 >= 100 && u1 <= 200 && u2 >= 150 && u2 <= 300) {
+// u1: [100,200], u2: [150,300]
+clang_analyzer_eval(u1 != u2); // expected-warning{{UNKNOWN}}
+  }
+
+  if (s1 >= -80 && s1 <= -50 && s2 >= -100 && s2 <= -75) {
+// s1: [-80,-50], s2: [-100,-75]
+clang_analyzer_eval(s1 != s2); // expected-warning{{UNKNOWN}}
+  }
+
+  // Checks for ranges which are subset of one-another
+  if (u1 >= 500 && u1 <= 1000 && u2 >= 750 && u2 <= 1000) {
+// u1: [500,1000], u2: [750,1000]
+clang_analyzer_eval(u1 != u2); // expected-warning{{UNKNOWN}}
+  }
+
+  if (s1 >= -1000 && s1 <= -500 && s2 <= -500 && s2 >= -750) {
+// s1: [-1000,-500], s2: [-500,-750]
+clang_analyzer_eval(s1 != s2); // expected-warning{{UNKNOWN}}
+  }
+
+  // Checks for comparison between different types
+  // Using different variables as previous constraints may interfere in the
+  // reasoning.
+  if (u3 <= 255 && s3 < 0) {
+// u3: [0, 255], s3: [INT_MIN, -1]
+clang_analyzer_eval(u3 != s3); // expected-warning{{TRUE}}
+  }
+}
Index: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
===
--- clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
+++ clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
@@ -1326,18 +1326,7 @@
   }
 
   RangeSet VisitBinaryOperator(RangeSet LHS, BinaryOperator::Opcode Op,
-   RangeSet RHS, QualType T) {
-switch (Op) {
-case BO_Or:
-  return VisitBinaryOperator(LHS, RHS, T);
-case BO_And:
-  return VisitBinaryOperator(LHS, RHS, T);
-case BO_Rem:
-  return VisitBinaryOperator(LHS, RHS, T);
-default:
-  return infer(T);
-}
-  }
+   RangeSet RHS, QualType T);
 
   //===--===//
   // Ranges and operators
@@ -1607,6 +1596,49 @@
 //   Range-based reasoning about symbolic operations
 //===--===//
 
+template <>
+RangeSet SymbolicRangeInferrer::VisitBinaryOperator(RangeSet LHS,
+   RangeSet RHS,
+   QualType T) {
+
+  // We must check for empty RangeSets. This gets done via VisitBinaryOperator
+  // for other operators, but BO_NE is handled specifically.
+  if (LHS.isEmpty() || RHS.isEmpty()) {
+return RangeFactory.getEmptySet();
+  }
+
+  Range CoarseLHS = fillGaps(LHS);
+  Range CoarseRHS = fillGaps(RHS);
+
+  APSIntType ResultType = ValueFactory.getAPSIntType(T);
+
+  auto ConvertedCoarseLHS = convert(CoarseLHS, ResultType);
+  auto ConvertedCoarseRHS = convert(CoarseRHS, ResultType);
+
+  if (!ConvertedCoarseLHS || !ConvertedCoarseRHS) {
+return infer(T);
+  }
+
+  // When both the Ranges are non-overlapping then all possible pairs of (x

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

2022-07-15 Thread Manas Gupta via Phabricator via cfe-commits
manas added a comment.

The coverage showing unreachability of `VisitBinaryOperator` for 
concrete integer cases. F23801808: RangeConstraintManager.cpp.html 





Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1415-1416
+
+  Range CoarseLHS = fillGaps(LHS);
+  Range CoarseRHS = fillGaps(RHS);
+

ASDenysPetrov wrote:
> Avoid filling the gaps, because it's completely possible to compare all the 
> ranges.
> E.g. //LHS //`[1,2]U[8,9]`  and  //RHS //`[5,6]` are not equal.
> And you don't need to fiil the gap in LHS, because it will lead you to a 
> wrong answer, like `[1,9] != [5,6]` => **UNKNOWN** instead of **TRUE**.
If we don't fill gaps, then we will be making this method O(n) instead of O(1) 
as of now. As I see it, filling RHS (and not filling LHS), it can iterate over 
all ranges in LHS, and check each range's intersection with CoarseRHS.

Before, I do this, I just wanted to point out the unreachability of concrete 
cases to this method. I have tried to explain this below.



Comment at: clang/test/Analysis/constant-folding.c:339
+  }
+}

ASDenysPetrov wrote:
> I'd like to see the cases with concrete integers as well.
I have checked the coverage reports which show that concrete values are not 
reaching `VisitBinaryOperator`. This is due to them being trivially 
inferred in `RangedConstraintManager.cpp`. I attached the coverage for 
`RangeConstraintManager.cpp`.

I think that I should remove the handling of concrete APSInt all together from 
SymbolicRangeInferrer for above reason. What do you guys think?


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] D103096: [analyzer] Implement cast for ranges of symbolic integers

2022-07-15 Thread Gabor Marton via Phabricator via cfe-commits
martong added a comment.

Thanks Denys for your continued work on this. These are very good questions 
that must be answered, we need exactly such thinking to implement this 
properly. I believe we can advance gradually.

In D103096#3642681 , @ASDenysPetrov 
wrote:

> @martong Thank you for your patience. I started moving to a bit another 
> direction that we can improving it iteratively.
> Just spoiling, in my latest solution a single symbol will be associated to 
> many classes. Here are also tricky cases:
>
> ---
>
> Consider equalities
>
>   a32 == (int8)b32
>   b32 == c32
>
> Class-Symbol map is:
>
>   class1 { a32 , (int8)b32 }
>   class2 { b32 , c32 }
>
> Symbol-Class map is:
>
>   a32 { 32 : class1 }
>   b32 {  8 : class1, 32 : class2  }
>   c32 { 32 : class2 }
>
> If we know:
>
>   (int8)c32 == -1
>
> then what is:
>
>   (int8)a32 - ?
>
> Should we traverse like `a ->  32 -> class1 -> (int8)b32 -> b32 ->  class2 -> 
> c32 -> (int8)c32 ` ?

I think, we should have only `a ->  32 -> class1 -> (int8)b32`. The `(int8)b32 
-> b32` step would be incorrect according to the modulo logic.
With other words, we should check the equivalence class of the root symbol (and 
no other eq classes should be considered), in this case this is only `class1`. 
(More precisely we should check the `SymCastMap` of `class1`.)

> ---
>
> The `x8 == y32` we can treat as a range of `int8` ( {-128, 127} or  {0, 255} 
> ).

I am not sure what you mean here.
You mean, when we bifurcate on `x8 == y32` ? Then we have two branches, the 
false case `x8 == y32`: [0, 0] and `x8 == y32`: [[INT_MIN, -1], [1,INT_MAX]]

> ---
>
> For `(int8)x32 == (int16)x32` we can eliminate one of the symbols in the 
> class a s a redundant one.

Yes, but this is more like an optimization step. I'd handle this with low 
priority and with a FIXME comment.

> ---
>
> If `x32 == 0` then we can simplify next classes `(int16)x32 == y` and 
> `(int8)x32 == z` merging them into a single class {x32, y, z}.

Good point. This is an optimization for precision. I'd like to have this, but 
in a subsequent patch. Let's try to have the absolute simplest working version 
in this patch.
Also, this can extend to any concrete value that is meaningful in the smaller 
types. E.g. any single value of x32 in [0,127] could simplify (int8)x32.

> I believe there are more cases.

Yes. Consider liveness for example. We should remove the class from 
`SymCastMap` if the class itself becomes dead. This should be part of this 
patch.


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

https://reviews.llvm.org/D103096

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


[PATCH] D129466: [clang-format][NFC] Replace most of std::vector with SmallVector

2022-07-15 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added inline comments.



Comment at: clang/lib/Format/ContinuationIndenter.cpp:40
 static unsigned getLengthToMatchingParen(const FormatToken &Tok,
- const std::vector &Stack) 
{
+ const SmallVector &Stack) 
{
   // Normally whether or not a break before T is possible is calculated and

owenpan wrote:
> curdeius wrote:
> > Sorry for a late comment. I think that in a parameter list, we should use 
> > `SmallVectorBase` to avoid changing the type if the inplace element count 
> > changes. That's not important now though.
> Hmm. I can't find an example of `SmallVectorBase` in llvm-project sources, 
> though.
Typo, it's Impl: https://llvm.org/doxygen/classllvm_1_1SmallVectorImpl.html


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129466

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


[PATCH] D129824: [RISCV] Set triple based on -march flag which can be deduced in more generic way

2022-07-15 Thread Yueh-Ting (eop) Chen via Phabricator via cfe-commits
eopXD added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:660
+
+if (RVArch == llvm::Triple::riscv64 &&
+ArchName.startswith_insensitive("rv32"))

Do we need to throw error (or warning) when these two (`RVArch` and `ArchName`) 
don't match?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129824

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


[PATCH] D129832: [sanitizer] Add "mainsrc" prefix to sanitizer special case list

2022-07-15 Thread Vitaly Buka via Phabricator via cfe-commits
vitalybuka accepted this revision.
vitalybuka added a comment.
This revision is now accepted and ready to land.

The patch is LGTM, but please consider to keep an ignorelist limited.




Comment at: clang/docs/SanitizerSpecialCaseList.rst:110-112
+``mainsrc`` can be useful enabling a ubsan check for a large code base when
+finding the direct stack frame triggering the failure for every failure is
+difficult.

MaskRay wrote:
> vitalybuka wrote:
> > if this is transitionalt solution, would it be better just to use 
> > -fno-sanitize= on *.cpp file?
> This provides a more convenient way than using `-fno-sanitize=` for a 
> different set of files.
> 
> E.g. if a group wants to ignore a check for all ``[a-m]*` files, then can use 
> `mainsrc:[a-m]*` instead of patching the build system.
Convenience of -fno-sanitize= approach that you make it visible to maintainer 
of component. Then it's their responsibility.
Ignore list is toolchain team debt.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129832

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


[PATCH] D128411: [syntax] Introduce a TokenManager interface.

2022-07-15 Thread David Spickett via Phabricator via cfe-commits
DavidSpickett added a comment.

FYI, after this change I get:

  
/home/david.spickett/llvm-project/clang/include/clang/Tooling/Syntax/TokenBufferTokenManager.h:20:7:
 warning: 'clang::syntax::TokenBufferTokenManager' has virtual functions but 
non-virtual destructor [-Wnon-virtual-dtor]
  class TokenBufferTokenManager : public TokenManager {
^


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128411

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


[PATCH] D12669: [libcxxabi] Fix alignment of pointers returned by fallback_malloc

2022-07-15 Thread Simon Tatham via Phabricator via cfe-commits
simon_tatham added a comment.

I don't know if "silence implies no objection" is a reasonable assumption in 
this community, so for the sake of not treading on toes, I've put my revised 
version of the patch in a new location instead: D129842 
.


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

https://reviews.llvm.org/D12669

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


[PATCH] D129771: [clang-format] distinguish multiplication after brace-init from pointer

2022-07-15 Thread Krasimir Georgiev via Phabricator via cfe-commits
krasimir updated this revision to Diff 444921.
krasimir added a comment.

- address review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129771

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


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -111,6 +111,9 @@
 "} &&ptr = {};");
   EXPECT_EQ(Tokens.size(), 10u) << Tokens;
   EXPECT_TOKEN(Tokens[3], tok::ampamp, TT_PointerOrReference);
+  Tokens = annotate("int i = int{42} * 2;");
+  EXPECT_EQ(Tokens.size(), 11u) << Tokens;
+  EXPECT_TOKEN(Tokens[7], tok::star, TT_BinaryOperator);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsUsesOfPlusAndMinus) {
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -10458,6 +10458,9 @@
   verifyFormat("class {\n"
"}* ptr;",
Style);
+  // Don't confuse a multiplication after a brace-initialized expression with
+  // a class pointer.
+  verifyFormat("int i = int{42} * 34;", Style);
   verifyFormat("struct {\n"
"}&& ptr = {};",
Style);
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2317,7 +2317,15 @@
 // After right braces, star tokens are likely to be pointers to struct,
 // union, or class.
 //   struct {} *ptr;
-if (PrevToken->is(tok::r_brace) && Tok.is(tok::star))
+// This by itself is not sufficient to distinguish from multiplication
+// following a brace-initialized expression, as in:
+// int i = int{42} * 2;
+// In the struct case, the part of the struct declaration until the `{` and
+// the `}` are put on separate unwrapped lines; in the brace-initialized
+// case, the matching `{` is on the same unwrapped line, so check for the
+// presence of the matching brace to distinguish between those.
+if (PrevToken->is(tok::r_brace) && Tok.is(tok::star) &&
+!PrevToken->MatchingParen)
   return TT_PointerOrReference;
 
 // For "} &&"


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -111,6 +111,9 @@
 "} &&ptr = {};");
   EXPECT_EQ(Tokens.size(), 10u) << Tokens;
   EXPECT_TOKEN(Tokens[3], tok::ampamp, TT_PointerOrReference);
+  Tokens = annotate("int i = int{42} * 2;");
+  EXPECT_EQ(Tokens.size(), 11u) << Tokens;
+  EXPECT_TOKEN(Tokens[7], tok::star, TT_BinaryOperator);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsUsesOfPlusAndMinus) {
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -10458,6 +10458,9 @@
   verifyFormat("class {\n"
"}* ptr;",
Style);
+  // Don't confuse a multiplication after a brace-initialized expression with
+  // a class pointer.
+  verifyFormat("int i = int{42} * 34;", Style);
   verifyFormat("struct {\n"
"}&& ptr = {};",
Style);
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2317,7 +2317,15 @@
 // After right braces, star tokens are likely to be pointers to struct,
 // union, or class.
 //   struct {} *ptr;
-if (PrevToken->is(tok::r_brace) && Tok.is(tok::star))
+// This by itself is not sufficient to distinguish from multiplication
+// following a brace-initialized expression, as in:
+// int i = int{42} * 2;
+// In the struct case, the part of the struct declaration until the `{` and
+// the `}` are put on separate unwrapped lines; in the brace-initialized
+// case, the matching `{` is on the same unwrapped line, so check for the
+// presence of the matching brace to distinguish between those.
+if (PrevToken->is(tok::r_brace) && Tok.is(tok::star) &&
+!PrevToken->MatchingParen)
   return TT_PointerOrReference;
 
 // For "} &&"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D129771: [clang-format] distinguish multiplication after brace-init from pointer

2022-07-15 Thread Krasimir Georgiev via Phabricator via cfe-commits
krasimir marked an inline comment as done.
krasimir added a comment.

In D129771#3653830 , @jackhong12 
wrote:

> I think we can also add new test cases in 
> `clang/unittests/Format/TokenAnnotatorTest.cpp`.
>
>   Tokens = annotate("int i = int{42} * 34;");
>   ...

added a test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129771

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


[clang] 8dd2ef2 - [clang-format] distinguish multiplication after brace-init from pointer

2022-07-15 Thread Krasimir Georgiev via cfe-commits

Author: Krasimir Georgiev
Date: 2022-07-15T11:45:12+02:00
New Revision: 8dd2ef2130852e406dab6097b1feb248bf16ce91

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

LOG: [clang-format] distinguish multiplication after brace-init from pointer

After 
https://github.com/llvm/llvm-project/commit/b646f0955574c6ad4c156c9db522e46f597cfda9,
the added regression test started being formatted as-if the
multiplication `*` was a pointer. This adapts the heuristic to
distinguish between these two cases.

Reviewed By: jackhong12, curdeius, HazardyKnusperkeks, owenpan

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

Added: 


Modified: 
clang/lib/Format/TokenAnnotator.cpp
clang/unittests/Format/FormatTest.cpp
clang/unittests/Format/TokenAnnotatorTest.cpp

Removed: 




diff  --git a/clang/lib/Format/TokenAnnotator.cpp 
b/clang/lib/Format/TokenAnnotator.cpp
index 98c012994f45..e0dc7a7f05c9 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -2317,7 +2317,15 @@ class AnnotatingParser {
 // After right braces, star tokens are likely to be pointers to struct,
 // union, or class.
 //   struct {} *ptr;
-if (PrevToken->is(tok::r_brace) && Tok.is(tok::star))
+// This by itself is not sufficient to distinguish from multiplication
+// following a brace-initialized expression, as in:
+// int i = int{42} * 2;
+// In the struct case, the part of the struct declaration until the `{` and
+// the `}` are put on separate unwrapped lines; in the brace-initialized
+// case, the matching `{` is on the same unwrapped line, so check for the
+// presence of the matching brace to distinguish between those.
+if (PrevToken->is(tok::r_brace) && Tok.is(tok::star) &&
+!PrevToken->MatchingParen)
   return TT_PointerOrReference;
 
 // For "} &&"

diff  --git a/clang/unittests/Format/FormatTest.cpp 
b/clang/unittests/Format/FormatTest.cpp
index ba07584d661a..08cfdbe2cc7a 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -10458,6 +10458,9 @@ TEST_F(FormatTest, UnderstandsUsesOfStarAndAmp) {
   verifyFormat("class {\n"
"}* ptr;",
Style);
+  // Don't confuse a multiplication after a brace-initialized expression with
+  // a class pointer.
+  verifyFormat("int i = int{42} * 34;", Style);
   verifyFormat("struct {\n"
"}&& ptr = {};",
Style);

diff  --git a/clang/unittests/Format/TokenAnnotatorTest.cpp 
b/clang/unittests/Format/TokenAnnotatorTest.cpp
index df1b05226747..a2474d5c10f8 100644
--- a/clang/unittests/Format/TokenAnnotatorTest.cpp
+++ b/clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -111,6 +111,9 @@ TEST_F(TokenAnnotatorTest, UnderstandsUsesOfStarAndAmp) {
 "} &&ptr = {};");
   EXPECT_EQ(Tokens.size(), 10u) << Tokens;
   EXPECT_TOKEN(Tokens[3], tok::ampamp, TT_PointerOrReference);
+  Tokens = annotate("int i = int{42} * 2;");
+  EXPECT_EQ(Tokens.size(), 11u) << Tokens;
+  EXPECT_TOKEN(Tokens[7], tok::star, TT_BinaryOperator);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsUsesOfPlusAndMinus) {



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


[PATCH] D129771: [clang-format] distinguish multiplication after brace-init from pointer

2022-07-15 Thread Krasimir Georgiev via 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 rG8dd2ef213085: [clang-format] distinguish multiplication 
after brace-init from pointer (authored by krasimir).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129771

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


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -111,6 +111,9 @@
 "} &&ptr = {};");
   EXPECT_EQ(Tokens.size(), 10u) << Tokens;
   EXPECT_TOKEN(Tokens[3], tok::ampamp, TT_PointerOrReference);
+  Tokens = annotate("int i = int{42} * 2;");
+  EXPECT_EQ(Tokens.size(), 11u) << Tokens;
+  EXPECT_TOKEN(Tokens[7], tok::star, TT_BinaryOperator);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsUsesOfPlusAndMinus) {
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -10458,6 +10458,9 @@
   verifyFormat("class {\n"
"}* ptr;",
Style);
+  // Don't confuse a multiplication after a brace-initialized expression with
+  // a class pointer.
+  verifyFormat("int i = int{42} * 34;", Style);
   verifyFormat("struct {\n"
"}&& ptr = {};",
Style);
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2317,7 +2317,15 @@
 // After right braces, star tokens are likely to be pointers to struct,
 // union, or class.
 //   struct {} *ptr;
-if (PrevToken->is(tok::r_brace) && Tok.is(tok::star))
+// This by itself is not sufficient to distinguish from multiplication
+// following a brace-initialized expression, as in:
+// int i = int{42} * 2;
+// In the struct case, the part of the struct declaration until the `{` and
+// the `}` are put on separate unwrapped lines; in the brace-initialized
+// case, the matching `{` is on the same unwrapped line, so check for the
+// presence of the matching brace to distinguish between those.
+if (PrevToken->is(tok::r_brace) && Tok.is(tok::star) &&
+!PrevToken->MatchingParen)
   return TT_PointerOrReference;
 
 // For "} &&"


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -111,6 +111,9 @@
 "} &&ptr = {};");
   EXPECT_EQ(Tokens.size(), 10u) << Tokens;
   EXPECT_TOKEN(Tokens[3], tok::ampamp, TT_PointerOrReference);
+  Tokens = annotate("int i = int{42} * 2;");
+  EXPECT_EQ(Tokens.size(), 11u) << Tokens;
+  EXPECT_TOKEN(Tokens[7], tok::star, TT_BinaryOperator);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsUsesOfPlusAndMinus) {
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -10458,6 +10458,9 @@
   verifyFormat("class {\n"
"}* ptr;",
Style);
+  // Don't confuse a multiplication after a brace-initialized expression with
+  // a class pointer.
+  verifyFormat("int i = int{42} * 34;", Style);
   verifyFormat("struct {\n"
"}&& ptr = {};",
Style);
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2317,7 +2317,15 @@
 // After right braces, star tokens are likely to be pointers to struct,
 // union, or class.
 //   struct {} *ptr;
-if (PrevToken->is(tok::r_brace) && Tok.is(tok::star))
+// This by itself is not sufficient to distinguish from multiplication
+// following a brace-initialized expression, as in:
+// int i = int{42} * 2;
+// In the struct case, the part of the struct declaration until the `{` and
+// the `}` are put on separate unwrapped lines; in the brace-initialized
+// case, the matching `{` is on the same unwrapped line, so check for the
+// presence of the matching brace to distinguish between those.
+if (PrevToken->is(tok::r_brace) && Tok.is(tok::star) &&
+!PrevToken->MatchingParen)
   return TT_PointerOrReference;
 
 // For "} &&"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/ma

[clang] 30c2406 - [syntax] Add virtual destructor in TokenManager.

2022-07-15 Thread Haojian Wu via cfe-commits

Author: Haojian Wu
Date: 2022-07-15T11:51:13+02:00
New Revision: 30c2406e270cc5dab8da813ce5c54e4bb8c40e49

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

LOG: [syntax] Add virtual destructor in TokenManager.

Fix `-Wnon-virtual-dtor` warning.

Added: 


Modified: 
clang/include/clang/Tooling/Syntax/TokenManager.h

Removed: 




diff  --git a/clang/include/clang/Tooling/Syntax/TokenManager.h 
b/clang/include/clang/Tooling/Syntax/TokenManager.h
index 73c95b4b77a8..6f0d11ce0d6b 100644
--- a/clang/include/clang/Tooling/Syntax/TokenManager.h
+++ b/clang/include/clang/Tooling/Syntax/TokenManager.h
@@ -28,6 +28,8 @@ namespace syntax {
 /// Defines interfaces for operating "Token" in the clang syntax-tree.
 class TokenManager {
 public:
+  virtual ~TokenManager() = default;
+
   /// Describes what the exact class kind of the TokenManager is.
   virtual llvm::StringLiteral kind() const = 0;
 



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


[PATCH] D128411: [syntax] Introduce a TokenManager interface.

2022-07-15 Thread Haojian Wu via Phabricator via cfe-commits
hokein added a comment.

In D128411#3654452 , @DavidSpickett 
wrote:

> FYI, after this change I get:
>
>   
> /home/david.spickett/llvm-project/clang/include/clang/Tooling/Syntax/TokenBufferTokenManager.h:20:7:
>  warning: 'clang::syntax::TokenBufferTokenManager' has virtual functions but 
> non-virtual destructor [-Wnon-virtual-dtor]
>   class TokenBufferTokenManager : public TokenManager {
> ^

Sorry, it is fixed in 30c2406e270cc5dab8da813ce5c54e4bb8c40e49 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128411

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


[PATCH] D129797: [clang-tidy] Reduce the dependencies for the "make-confusable-table" tool

2022-07-15 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment.

This matches what I did for clang-pseudo-gen. It looks reasonable to me, but 
I'd love to get a third opinion...
(If none in a couple of days, happy to stamp it)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129797

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


[PATCH] D129648: Use pseudo parser for folding ranges

2022-07-15 Thread Haojian Wu via Phabricator via cfe-commits
hokein added inline comments.



Comment at: clang-tools-extra/clangd/SemanticSelection.cpp:175
+
+  llvm::Optional Preprocessed;
+  Preprocessed = DirectiveStructure.stripDirectives(OrigStream);

nit: no need to use llvm::Optional.



Comment at: clang-tools-extra/clangd/SemanticSelection.cpp:178
+
+  auto ParseableStream = clang::pseudo::stripComments(
+  cook(*Preprocessed, clang::pseudo::genericLangOpts()));

Do we want to strip comments? I think we can keep the comments.



Comment at: clang-tools-extra/clangd/SemanticSelection.cpp:185
+if (auto *Paired = Tok.pair()) {
+  if (Tok.Line < Paired->Line) {
+Position Start = offsetToPosition(

The `if` logic seems tricky, it is doing two different things:

1) avoid creating duplicate `FoldingRange` for a pair bracket
2) avoid creating a FoldingRange if the pair bracket is on the same line.

Are both intended?



Comment at: clang-tools-extra/clangd/SemanticSelection.cpp:187
+Position Start = offsetToPosition(
+Code, OrigStream.origToken(Tok).text().data() - Code.data());
+Position End = offsetToPosition(

the ` OrigStream.origToken` syntax seems weird from the caller side (it is 
counter-intuitive).

I think  `OrigStream.tokens()[T.OrigTokIdx]` is clearer.  



Comment at: clang-tools-extra/pseudo/include/clang-pseudo/Token.h:72
+  /// Index into the original token stream (of the original source file).
+  Index OrigTokIdx = 0;
   // Helpers to get/set Flags based on `enum class`.

I'd suggest initializing it to `Index::Invalid`.

it'd be nice to add some tests in the `pseudo/unittests/TokenTest.cpp`.





Comment at: clang-tools-extra/pseudo/include/clang-pseudo/Token.h:182
+  /// Extracts the token in original stream corresponding to Token T.
+  const Token &origToken(const Token &T) const {
+assert(isOriginal() && "stream is derived");

I'm not sure we should have this API, it seems error-prone to use, and it only 
makes sense for the original token stream.  (I'd just remove it, and use 
`OrigStream.tokens()[T.OrigTokIdx]` directly, see my other comment)



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129648

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


[PATCH] D129648: Use pseudo parser for folding ranges

2022-07-15 Thread Sam McCall via Phabricator via cfe-commits
sammccall added inline comments.



Comment at: clang-tools-extra/clangd/CMakeLists.txt:149
+target_include_directories(clangDaemon PUBLIC
+  ${CMAKE_CURRENT_SOURCE_DIR}/../pseudo/include
+)

It should rather be up to the clangPseudo target to specify its include 
directories, if it doesn't already

I think interface_include_directories() is the right function (though knowing 
llvm maybe it's wrapped somehow)

https://cmake.org/cmake/help/latest/prop_tgt/INTERFACE_INCLUDE_DIRECTORIES.html



Comment at: clang-tools-extra/clangd/SemanticSelection.cpp:158
 
 // FIXME(kirillbobyrev): Collect comments, PP conditional regions, includes and
 // other code regions (e.g. public/private/protected sections of classes,

we should transfer this fixme, either now or when we remove this code



Comment at: clang-tools-extra/clangd/SemanticSelection.cpp:176
+  llvm::Optional Preprocessed;
+  Preprocessed = DirectiveStructure.stripDirectives(OrigStream);
+

This means we won't offer folding ranges in disabled-PP regions.
This is a significant regression of the client-side fallback!
It's not trivial to address though. Add a FIXME?



Comment at: clang-tools-extra/clangd/SemanticSelection.cpp:178
+
+  auto ParseableStream = clang::pseudo::stripComments(
+  cook(*Preprocessed, clang::pseudo::genericLangOpts()));

hokein wrote:
> Do we want to strip comments? I think we can keep the comments.
Stripping comments doesn't seem necessary to me, maybe I'm missing something.



Comment at: clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp:272
 auto T = Annotations(Test);
-auto AST = TestTU::withCode(T.code()).build();
-EXPECT_THAT(gatherFoldingRanges(llvm::cantFail(getFoldingRanges(AST))),
-UnorderedElementsAreArray(T.ranges()))
+EXPECT_THAT(
+gatherFoldingRanges(llvm::cantFail(getFoldingRanges(T.code().str(,

You've removed the tests for the old implementation, but it's still used by 
ClangdServer.

Test both versions instead?



Comment at: clang-tools-extra/pseudo/include/clang-pseudo/Token.h:71
   uint8_t Flags = 0;
+  /// Index into the original token stream (of the original source file).
+  Index OrigTokIdx = 0;

nit: not clear what "original source file" is opposed to

maybe (as raw-lexed from the source code)



Comment at: clang-tools-extra/pseudo/include/clang-pseudo/Token.h:72
+  /// Index into the original token stream (of the original source file).
+  Index OrigTokIdx = 0;
   // Helpers to get/set Flags based on `enum class`.

hokein wrote:
> I'd suggest initializing it to `Index::Invalid`.
> 
> it'd be nice to add some tests in the `pseudo/unittests/TokenTest.cpp`.
> 
> 
OriginalIndex (avoid cryptic abbreviations, "token" is implied)



Comment at: clang-tools-extra/pseudo/include/clang-pseudo/Token.h:176
 
+  /// An original token stream corresponds to the original source file (Eg.
+  /// produced by lex()). It is not derived from another token stream.

I don't like the stream knowing about this attribute, there's no reasonable way 
to actually use it dynamically at runtime, we're complicating the API just so 
we can add some assertions.

What's wrong with OrigToken = OrigStream.tokens()[T.OrigIndex]?

(If we're going to teach the stream about the "derived stream" concept, it 
seems more natural to give it a pointer to its parent and allow mapping to any 
ancestor stream)



Comment at: clang-tools-extra/pseudo/lib/Lex.cpp:30
+  // Index into the token stream of original source code.
+  unsigned TokenIdx = 0;
   unsigned LastOffset = 0;

nit: unsigned -> Token::Index
Idx -> Index? (it's not too cryptic here but also only saving 2 chars)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129648

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


Re: [PATCH] D129311: [clang-format] Update return code

2022-07-15 Thread Sridhar Gopinath via cfe-commits
Could someone please review this change? Thanks!

— Sridhar

> On Jul 11, 2022, at 12:55 PM, Sridhar Gopinath via Phabricator 
>  wrote:
> 
> sridhar_gopinath updated this revision to Diff 443726.
> sridhar_gopinath added a comment.
> 
> Replaced subprocess.check_call with subprocess.call since the former crashes 
> when the return code is not zero.
> + formatting changes
> 
> 
> Repository:
>  rG LLVM Github Monorepo
> 
> CHANGES SINCE LAST ACTION
>  https://reviews.llvm.org/D129311/new/
> 
> https://reviews.llvm.org/D129311
> 
> Files:
>  clang/tools/clang-format/git-clang-format
> 
> 
> Index: clang/tools/clang-format/git-clang-format
> ===
> --- clang/tools/clang-format/git-clang-format
> +++ clang/tools/clang-format/git-clang-format
> @@ -198,16 +198,16 @@
> return 0
> 
>   if opts.diff:
> -print_diff(old_tree, new_tree)
> -  elif opts.diffstat:
> -print_diffstat(old_tree, new_tree)
> -  else:
> -changed_files = apply_changes(old_tree, new_tree, force=opts.force,
> -  patch_mode=opts.patch)
> -if (opts.verbose >= 0 and not opts.patch) or opts.verbose >= 1:
> -  print('changed files:')
> -  for filename in changed_files:
> -print('%s' % filename)
> +return print_diff(old_tree, new_tree)
> +  if opts.diffstat:
> +return print_diffstat(old_tree, new_tree)
> +
> +  changed_files = apply_changes(old_tree, new_tree, force=opts.force,
> +patch_mode=opts.patch)
> +  if (opts.verbose >= 0 and not opts.patch) or opts.verbose >= 1:
> +print('changed files:')
> +for filename in changed_files:
> +  print('%s' % filename)
> 
>   return 1
> 
> @@ -536,8 +536,8 @@
>   # We also only print modified files since `new_tree` only contains the files
>   # that were modified, so unmodified files would show as deleted without the
>   # filter.
> -  subprocess.check_call(['git', 'diff', '--diff-filter=M', old_tree, 
> new_tree,
> - '--'])
> +  return subprocess.call(['git', 'diff', '--diff-filter=M',
> +  old_tree, new_tree, '--exit-code', '--'])
> 
> def print_diffstat(old_tree, new_tree):
>   """Print the diffstat between the two trees to stdout."""
> @@ -548,8 +548,14 @@
>   # We also only print modified files since `new_tree` only contains the files
>   # that were modified, so unmodified files would show as deleted without the
>   # filter.
> -  subprocess.check_call(['git', 'diff', '--diff-filter=M', '--stat', 
> old_tree, new_tree,
> - '--'])
> +  return subprocess.call(['git',
> + 'diff',
> +  '--diff-filter=M',
> +  '--stat',
> +  old_tree,
> +  new_tree,
> +  '--exit-code',
> +  '--'])
> 
> def apply_changes(old_tree, new_tree, force=False, patch_mode=False):
>   """Apply the changes in `new_tree` to the working directory.
> 
> 
> 

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


[PATCH] D129845: Allow custom attributes in access specifiers

2022-07-15 Thread Danil Sidoruk via Phabricator via cfe-commits
eoanermine created this revision.
eoanermine added reviewers: owenpan, HazardyKnusperkeks, curdeius.
eoanermine added projects: clang, clang-format.
Herald added a project: All.
eoanermine requested review of this revision.
Herald added a subscriber: cfe-commits.

- Allow custom attributes in access specifiers
- Add tests for custom attributes in access specifiers
- Add more tests for correct handling of Qt's signals and slots


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D129845

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -3300,8 +3300,19 @@
"Q_SIGNALS:\n"
"  void g2();\n"
"};");
+  verifyFormat("class A {\n"
+   "public ATTR:\n"
+   "  void f1() {}\n"
+   "protected ATTR:\n"
+   "  void f2() {}\n"
+   "private ATTR:\n"
+   "  void f3() {}\n"
+   "};");
 
   // Don't interpret 'signals' the wrong way.
+  verifyFormat("struct Bitfields {\n"
+   "  unsigned signals : 1;\n"
+   "};");
   verifyFormat("signals.set();");
   verifyFormat("for (Signals signals : f()) {\n}");
   verifyFormat("{\n"
@@ -3311,6 +3322,21 @@
"label:\n"
"  signals.baz();\n"
"}");
+
+  // Don't interpret 'slots' the wrong way.
+  verifyFormat("struct Bitfields {\n"
+   "  unsigned slots : 1;\n"
+   "};");
+  verifyFormat("slots.set();");
+  verifyFormat("for (Signals slots : f()) {\n}");
+  verifyFormat("{\n"
+   "  slots.set(); // This needs indentation.\n"
+   "}");
+  verifyFormat("void f() {\n"
+   "label:\n"
+   "  slots.baz();\n"
+   "}");
+
   verifyFormat("private[1];");
   verifyFormat("testArray[public] = 1;");
   verifyFormat("public();");
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -3087,6 +3087,10 @@
   // Understand Qt's slots.
   if (FormatTok->isOneOf(Keywords.kw_slots, Keywords.kw_qslots))
 nextToken();
+  // Handle custom attributes.
+  if (FormatTok->is(tok::identifier) && 
Tokens->peekNextToken()->is(tok::colon))
+nextToken();
+
   // Otherwise, we don't know what it is, and we'd better keep the next token.
   if (FormatTok->is(tok::colon)) {
 nextToken();
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -113,13 +113,14 @@
   } else if (RootToken.isObjCAccessSpecifier()) {
 return true;
   }
-  // Handle Qt signals.
-  else if ((RootToken.isOneOf(Keywords.kw_signals, Keywords.kw_qsignals) &&
-RootToken.Next && RootToken.Next->is(tok::colon))) {
+  // Handle Qt signals and custom attributes.
+  else if (RootToken.isOneOf(Keywords.kw_signals, Keywords.kw_qsignals) &&
+   RootToken.Next && RootToken.Next->is(tok::colon)) {
 return true;
-  } else if (RootToken.Next &&
- RootToken.Next->isOneOf(Keywords.kw_slots,
- Keywords.kw_qslots) &&
+  } else if (RootToken.isAccessSpecifier(/*colonRequired=*/false) &&
+ RootToken.Next &&
+ RootToken.Next->isOneOf(Keywords.kw_slots, Keywords.kw_qslots,
+ tok::identifier) &&
  RootToken.Next->Next && RootToken.Next->Next->is(tok::colon)) 
{
 return true;
   }


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -3300,8 +3300,19 @@
"Q_SIGNALS:\n"
"  void g2();\n"
"};");
+  verifyFormat("class A {\n"
+   "public ATTR:\n"
+   "  void f1() {}\n"
+   "protected ATTR:\n"
+   "  void f2() {}\n"
+   "private ATTR:\n"
+   "  void f3() {}\n"
+   "};");
 
   // Don't interpret 'signals' the wrong way.
+  verifyFormat("struct Bitfields {\n"
+   "  unsigned signals : 1;\n"
+   "};");
   verifyFormat("signals.set();");
   verifyFormat("for (Signals signals : f()) {\n}");
   verifyFormat("{\n"
@@ -3311,6 +3322,21 @@
"label:\n"
"  signals.baz();\n"
"}");
+
+  // Don't interpret 'slots' the wrong 

[PATCH] D128048: Add a new clang option "-ftime-trace="

2022-07-15 Thread dongjunduo via Phabricator via cfe-commits
dongjunduo updated this revision to Diff 444948.
dongjunduo added a comment.

[Clang] fix mkdir error in ftime-trace= test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128048

Files:
  clang/include/clang/Driver/Options.td
  clang/include/clang/Frontend/FrontendOptions.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/check-time-trace.cpp
  clang/tools/driver/cc1_main.cpp


Index: clang/tools/driver/cc1_main.cpp
===
--- clang/tools/driver/cc1_main.cpp
+++ clang/tools/driver/cc1_main.cpp
@@ -212,7 +212,9 @@
   bool Success = CompilerInvocation::CreateFromArgs(Clang->getInvocation(),
 Argv, Diags, Argv0);
 
-  if (Clang->getFrontendOpts().TimeTrace) {
+  if (Clang->getFrontendOpts().TimeTrace ||
+  !Clang->getFrontendOpts().TimeTracePath.empty()) {
+Clang->getFrontendOpts().TimeTrace = 1;
 llvm::timeTraceProfilerInitialize(
 Clang->getFrontendOpts().TimeTraceGranularity, Argv0);
   }
@@ -256,6 +258,13 @@
   if (llvm::timeTraceProfilerEnabled()) {
 SmallString<128> Path(Clang->getFrontendOpts().OutputFile);
 llvm::sys::path::replace_extension(Path, "json");
+if (!Clang->getFrontendOpts().TimeTracePath.empty()) {
+  // replace the suffix to '.json' directly
+  SmallString<128> TracePath(Clang->getFrontendOpts().TimeTracePath);
+  if (llvm::sys::fs::is_directory(TracePath))
+llvm::sys::path::append(TracePath, llvm::sys::path::filename(Path));
+  Path.assign(TracePath);
+}
 if (auto profilerOutput = Clang->createOutputFile(
 Path.str(), /*Binary=*/false, /*RemoveFileOnSignal=*/false,
 /*useTemporary=*/false)) {
Index: clang/test/Driver/check-time-trace.cpp
===
--- clang/test/Driver/check-time-trace.cpp
+++ clang/test/Driver/check-time-trace.cpp
@@ -2,6 +2,20 @@
 // RUN: cat %T/check-time-trace.json \
 // RUN:   | %python -c 'import json, sys; 
json.dump(json.loads(sys.stdin.read()), sys.stdout, sort_keys=True, indent=2)' \
 // RUN:   | FileCheck %s
+// RUN: %clangxx -S -ftime-trace=%T/new-name.json -ftime-trace-granularity=0 
-o %T/check-time-trace %s
+// RUN: cat %T/new-name.json \
+// RUN:   | %python -c 'import json, sys; 
json.dump(json.loads(sys.stdin.read()), sys.stdout, sort_keys=True, indent=2)' \
+// RUN:   | FileCheck %s
+// RUN: rm -rf %T/output1 && mkdir %T/output1
+// RUN: %clangxx -S -ftime-trace=%T/output1 -ftime-trace-granularity=0 -o 
%T/check-time-trace %s
+// RUN: cat %T/output1/check-time-trace.json \
+// RUN:   | %python -c 'import json, sys; 
json.dump(json.loads(sys.stdin.read()), sys.stdout, sort_keys=True, indent=2)' \
+// RUN:   | FileCheck %s
+// RUN: rm -rf %T/output2 && mkdir %T/output2
+// RUN: %clangxx -S -ftime-trace=%T/output2/ -ftime-trace-granularity=0 -o 
%T/check-time-trace %s
+// RUN: cat %T/output2/check-time-trace.json \
+// RUN:   | %python -c 'import json, sys; 
json.dump(json.loads(sys.stdin.read()), sys.stdout, sort_keys=True, indent=2)' \
+// RUN:   | FileCheck %s
 
 // CHECK:  "beginningOfTime": {{[0-9]{16},}}
 // CHECK-NEXT: "traceEvents": [
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -6183,6 +6183,7 @@
   Args.AddLastArg(CmdArgs, options::OPT_ftime_report_EQ);
   Args.AddLastArg(CmdArgs, options::OPT_ftime_trace);
   Args.AddLastArg(CmdArgs, options::OPT_ftime_trace_granularity_EQ);
+  Args.AddLastArg(CmdArgs, options::OPT_ftime_trace_EQ);
   Args.AddLastArg(CmdArgs, options::OPT_ftrapv);
   Args.AddLastArg(CmdArgs, options::OPT_malign_double);
   Args.AddLastArg(CmdArgs, options::OPT_fno_temp_file);
Index: clang/include/clang/Frontend/FrontendOptions.h
===
--- clang/include/clang/Frontend/FrontendOptions.h
+++ clang/include/clang/Frontend/FrontendOptions.h
@@ -499,6 +499,9 @@
   /// Minimum time granularity (in microseconds) traced by time profiler.
   unsigned TimeTraceGranularity;
 
+  /// Path which stores the output files for -ftime-trace
+  std::string TimeTracePath;
+
 public:
   FrontendOptions()
   : DisableFree(false), RelocatablePCH(false), ShowHelp(false),
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -2828,6 +2828,15 @@
   HelpText<"Minimum time granularity (in microseconds) traced by time 
profiler">,
   Flags<[CC1Option, CoreOption]>,
   MarshallingInfoInt, "500u">;
+def ftime_trace_EQ : Joined<["-"], "ftime-trace=">, Group,
+  HelpText<"Turn on time profiler. Generates JSON file based on output 
filename. "
+

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

2022-07-15 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added inline comments.



Comment at: clang/lib/Sema/SemaDecl.cpp:17863-17869
+/// Assuming the suggested solution to CWG2595 gets accepted:
+/// [class.mem.special]p6:
+/// An eligible special member function is a special member function for which:
+/// - the function is not deleted,
+/// - the associated constraints, if any, are satisfied, and
+/// - no special member function of the same kind whose associated constraints,
+///   if any, are satisfied is more constrained.





Comment at: clang/lib/Sema/SemaDecl.cpp:17916
+
+  auto SetEligibleMethods = [&S, &Record,
+ AreSameKind](ArrayRef Methods,

That could be a separate (static) function



Comment at: clang/lib/Sema/SemaDecl.cpp:17937
+continue;
+  auto *MethodI = Methods[i];
+  const Expr *ConstraintsI = MethodI->getTrailingRequiresClause();

You should probably be explicit about the type here.



Comment at: clang/lib/Sema/SemaDecl.cpp:17960-17961
+}
+if (AnotherMethodIsMoreConstrained)
+  break;
+  }

Is that codepath ever taken?
I wonder if there is a way to do that that is not n^2. Maybe not.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128619

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


[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

2022-07-15 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment.

Thanks for working on that!
The patch looks very good overall


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128619

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


[clang] 5775c5d - Remove an unsued-variable warning, NFC.

2022-07-15 Thread Haojian Wu via cfe-commits

Author: Haojian Wu
Date: 2022-07-15T14:40:52+02:00
New Revision: 5775c5d05c91c82efef522043ddfb52c28e65b4d

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

LOG: Remove an unsued-variable warning, NFC.

Added: 


Modified: 
clang/lib/AST/DeclPrinter.cpp

Removed: 




diff  --git a/clang/lib/AST/DeclPrinter.cpp b/clang/lib/AST/DeclPrinter.cpp
index 3f04d9b4073e9..b041e2a67e95f 100644
--- a/clang/lib/AST/DeclPrinter.cpp
+++ b/clang/lib/AST/DeclPrinter.cpp
@@ -1007,10 +1007,10 @@ void DeclPrinter::VisitCXXRecordDecl(CXXRecordDecl *D) {
 }
   }
 
-  if (auto *Def = D->getDefinition()) {
-  if (D->hasAttr()) {
-  Out << " final";
-  }
+  if (D->hasDefinition()) {
+if (D->hasAttr()) {
+  Out << " final";
+}
   }
 
   if (D->isCompleteDefinition()) {



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


[PATCH] D129845: Allow custom attributes in access specifiers

2022-07-15 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:3304
+  verifyFormat("class A {\n"
+   "public ATTR:\n"
+   "  void f1() {}\n"

How about multiple macros?
E.g. `public ATTR1 ATTR2`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129845

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


[PATCH] D129845: [clang-format] Allow custom attributes in access specifiers

2022-07-15 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:3298
"  void f6() {}\n"
"signals:\n"
"  void g1();\n"

In the same vein, could you add test for `slots:` and `Q_SLOTS:` please?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129845

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


[PATCH] D129359: [pseudo] Generate an enum type for identifying grammar rules.

2022-07-15 Thread Haojian Wu via Phabricator via cfe-commits
hokein updated this revision to Diff 444956.
hokein marked 2 inline comments as done.
hokein added a comment.

update.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129359

Files:
  clang-tools-extra/pseudo/gen/Main.cpp
  clang-tools-extra/pseudo/include/clang-pseudo/cxx/CXX.h
  clang-tools-extra/pseudo/include/clang-pseudo/grammar/Grammar.h
  clang-tools-extra/pseudo/lib/grammar/Grammar.cpp
  clang-tools-extra/pseudo/unittests/GrammarTest.cpp

Index: clang-tools-extra/pseudo/unittests/GrammarTest.cpp
===
--- clang-tools-extra/pseudo/unittests/GrammarTest.cpp
+++ clang-tools-extra/pseudo/unittests/GrammarTest.cpp
@@ -114,6 +114,21 @@
   EXPECT_NE(G.lookupRule(ruleFor("x")).Guard, G.lookupRule(ruleFor("y")).Guard);
 }
 
+TEST_F(GrammarTest, MangleName) {
+  build(R"bnf(
+_ := declaration
+
+declaration := ptr-declarator ;
+ptr-declarator := * IDENTIFIER
+
+  )bnf");
+  ASSERT_TRUE(Diags.empty());
+  EXPECT_EQ(G.mangleRule(ruleFor("declaration")),
+"declaration_0ptr_declarator_1semi");
+  EXPECT_EQ(G.mangleRule(ruleFor("ptr-declarator")),
+"ptr_declarator_0star_1identifier");
+}
+
 TEST_F(GrammarTest, Diagnostics) {
   build(R"cpp(
 _ := ,_opt
Index: clang-tools-extra/pseudo/lib/grammar/Grammar.cpp
===
--- clang-tools-extra/pseudo/lib/grammar/Grammar.cpp
+++ clang-tools-extra/pseudo/lib/grammar/Grammar.cpp
@@ -45,6 +45,28 @@
   return T->Nonterminals[SID].Name;
 }
 
+std::string Grammar::mangleSymbol(SymbolID SID) const {
+  static const char *const TokNames[] = {
+#define TOK(X) #X,
+#define KEYWORD(X, Y) #X,
+#include "clang/Basic/TokenKinds.def"
+  nullptr};
+  if (clang::pseudo::isToken(SID))
+return TokNames[clang::pseudo::symbolToToken(SID)];
+  std::string Name = symbolName(SID).str();
+  // translation-unit -> translation_unit
+  std::replace(Name.begin(), Name.end(), '-', '_');
+  return Name;
+}
+
+std::string Grammar::mangleRule(RuleID RID) const {
+  const auto &R = lookupRule(RID);
+  std::string MangleName = mangleSymbol(R.Target);
+  for (size_t I = 0; I < R.seq().size(); ++I)
+MangleName += llvm::formatv("_{0}{1}", I, mangleSymbol(R.seq()[I]));
+  return MangleName;
+}
+
 llvm::Optional Grammar::findNonterminal(llvm::StringRef Name) const {
   auto It = llvm::partition_point(
   T->Nonterminals,
Index: clang-tools-extra/pseudo/include/clang-pseudo/grammar/Grammar.h
===
--- clang-tools-extra/pseudo/include/clang-pseudo/grammar/Grammar.h
+++ clang-tools-extra/pseudo/include/clang-pseudo/grammar/Grammar.h
@@ -165,6 +165,21 @@
   // Terminals have names like "," (kw_comma) or "OPERATOR" (kw_operator).
   llvm::StringRef symbolName(SymbolID) const;
 
+  // Gets the mangled name for a terminal/nonterminal.
+  // Compared to names in the grammar,
+  //   nonterminals `ptr-declartor` becomes `ptr_declarator`;
+  //   terminal `,` becomes `comma`;
+  //   terminal `IDENTIFIER` becomes `identifier`;
+  //   terminal `INT` becomes `int`;
+  // NOTE: for nonterminals, the mangled name is the same as the cxx::Symbol
+  // enum class; for terminals, we deliberately stripped the `kw_` prefix in
+  // favor of the simplicity.
+  std::string mangleSymbol(SymbolID) const;
+  // Gets the mangled name for the rule.
+  // E.g. for the grammar rule `ptr-declarator := ptr-operator ptr-declarator`,
+  // it is `ptr_declarator_0ptr_operator_1ptr_declarator`.
+  std::string mangleRule(RuleID) const;
+
   // Lookup the SymbolID of the nonterminal symbol by Name.
   llvm::Optional findNonterminal(llvm::StringRef Name) const;
 
Index: clang-tools-extra/pseudo/include/clang-pseudo/cxx/CXX.h
===
--- clang-tools-extra/pseudo/include/clang-pseudo/cxx/CXX.h
+++ clang-tools-extra/pseudo/include/clang-pseudo/cxx/CXX.h
@@ -37,6 +37,12 @@
 #undef NONTERMINAL
 };
 
+enum class Rule : RuleID {
+#define RULE(X, Y) X = Y,
+#include "CXXSymbols.inc"
+#undef RULE
+};
+
 enum class Extension : ExtensionID {
 #define EXTENSION(X, Y) X = Y,
 #include "CXXSymbols.inc"
Index: clang-tools-extra/pseudo/gen/Main.cpp
===
--- clang-tools-extra/pseudo/gen/Main.cpp
+++ clang-tools-extra/pseudo/gen/Main.cpp
@@ -83,17 +83,19 @@
 #ifndef NONTERMINAL
 #define NONTERMINAL(X, Y)
 #endif
+#ifndef RULE
+#define RULE(X, Y)
+#endif
 #ifndef EXTENSION
 #define EXTENSION(X, Y)
 #endif
-)cpp";
+)cpp";
 for (clang::pseudo::SymbolID ID = 0; ID < G.table().Nonterminals.size();
- ++ID) {
-  std::string Name = G.symbolName(ID).str();
-  // translation-unit -> translation_unit
-  std::replace(Name.begin(), Name.end(), '-', '_');
-  Out.os() << llvm::formatv("NONTERMINAL({0}, {1})

[PATCH] D129359: [pseudo] Generate an enum type for identifying grammar rules.

2022-07-15 Thread Haojian Wu via Phabricator via cfe-commits
hokein added inline comments.



Comment at: clang-tools-extra/pseudo/include/clang-pseudo/grammar/Grammar.h:172
+  //   terminal `,` becomes `comma`;
+  //   terminal `INT` becomes `int`;
+  std::string mangleSymbol(SymbolID) const;

sammccall wrote:
> I hope you mean kw_int? I'd like this to compile :-)
> 
> Can you also give the example for IDENTIFIER?
As discussed offline, we stick to the current version without `kw_` prefix. 
Added some comments for this decision.



Comment at: clang-tools-extra/pseudo/lib/grammar/Grammar.cpp:48
 
+std::string Grammar::mangleSymbol(SymbolID SID) const {
+  static const char *const TokNames[] = {

sammccall wrote:
> I'm not sure exposing these from `Grammar` is an improvement, i think even in 
> principle we'd only want to use these for codegen.
> 
> The need for testing is real, but i guess you can add a test that uses the 
> CXX generated grammar, assert the elements of Rule::whatever, and the name of 
> Symbol::whatever?
Yeah, these functions are only used in the codegen. I'm inlined to the put it 
into the `Grammar`(it also has a `symbolName` definition, I'd prefer to have 
all naming functions in a single place). 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129359

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


[clang-tools-extra] 2315358 - [pseudo] Generate an enum type for identifying grammar rules.

2022-07-15 Thread Haojian Wu via cfe-commits

Author: Haojian Wu
Date: 2022-07-15T15:09:31+02:00
New Revision: 2315358906078cd35e2eb64bdcb711b2ec35

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

LOG: [pseudo] Generate an enum type for identifying grammar rules.

The Rule enum type enables us to identify a grammar rule within C++'s
type system.

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

Added: 


Modified: 
clang-tools-extra/pseudo/gen/Main.cpp
clang-tools-extra/pseudo/include/clang-pseudo/cxx/CXX.h
clang-tools-extra/pseudo/include/clang-pseudo/grammar/Grammar.h
clang-tools-extra/pseudo/lib/grammar/Grammar.cpp
clang-tools-extra/pseudo/unittests/GrammarTest.cpp

Removed: 




diff  --git a/clang-tools-extra/pseudo/gen/Main.cpp 
b/clang-tools-extra/pseudo/gen/Main.cpp
index b74f3d46c3560..27857ca4b3e86 100644
--- a/clang-tools-extra/pseudo/gen/Main.cpp
+++ b/clang-tools-extra/pseudo/gen/Main.cpp
@@ -83,17 +83,19 @@ int main(int argc, char *argv[]) {
 #ifndef NONTERMINAL
 #define NONTERMINAL(X, Y)
 #endif
+#ifndef RULE
+#define RULE(X, Y)
+#endif
 #ifndef EXTENSION
 #define EXTENSION(X, Y)
 #endif
-)cpp";
+)cpp";
 for (clang::pseudo::SymbolID ID = 0; ID < G.table().Nonterminals.size();
- ++ID) {
-  std::string Name = G.symbolName(ID).str();
-  // translation-unit -> translation_unit
-  std::replace(Name.begin(), Name.end(), '-', '_');
-  Out.os() << llvm::formatv("NONTERMINAL({0}, {1})\n", Name, ID);
-}
+ ++ID)
+  Out.os() << llvm::formatv("NONTERMINAL({0}, {1})\n", G.mangleSymbol(ID),
+ID);
+for (clang::pseudo::RuleID RID = 0; RID < G.table().Rules.size(); ++RID)
+  Out.os() << llvm::formatv("RULE({0}, {1})\n", G.mangleRule(RID), RID);
 for (clang::pseudo::ExtensionID EID = 1 /*skip the sentinel 0 value*/;
  EID < G.table().AttributeValues.size(); ++EID) {
   llvm::StringRef Name = G.table().AttributeValues[EID];
@@ -102,8 +104,9 @@ int main(int argc, char *argv[]) {
 }
 Out.os() << R"cpp(
 #undef NONTERMINAL
+#undef RULE
 #undef EXTENSION
-)cpp";
+)cpp";
 break;
   case EmitGrammarContent:
 for (llvm::StringRef Line : llvm::split(GrammarText, '\n')) {

diff  --git a/clang-tools-extra/pseudo/include/clang-pseudo/cxx/CXX.h 
b/clang-tools-extra/pseudo/include/clang-pseudo/cxx/CXX.h
index d2509927337ed..a1426722262e1 100644
--- a/clang-tools-extra/pseudo/include/clang-pseudo/cxx/CXX.h
+++ b/clang-tools-extra/pseudo/include/clang-pseudo/cxx/CXX.h
@@ -37,6 +37,12 @@ enum class Symbol : SymbolID {
 #undef NONTERMINAL
 };
 
+enum class Rule : RuleID {
+#define RULE(X, Y) X = Y,
+#include "CXXSymbols.inc"
+#undef RULE
+};
+
 enum class Extension : ExtensionID {
 #define EXTENSION(X, Y) X = Y,
 #include "CXXSymbols.inc"

diff  --git a/clang-tools-extra/pseudo/include/clang-pseudo/grammar/Grammar.h 
b/clang-tools-extra/pseudo/include/clang-pseudo/grammar/Grammar.h
index ab11a84ebf295..ef22f71d801c0 100644
--- a/clang-tools-extra/pseudo/include/clang-pseudo/grammar/Grammar.h
+++ b/clang-tools-extra/pseudo/include/clang-pseudo/grammar/Grammar.h
@@ -165,6 +165,21 @@ class Grammar {
   // Terminals have names like "," (kw_comma) or "OPERATOR" (kw_operator).
   llvm::StringRef symbolName(SymbolID) const;
 
+  // Gets the mangled name for a terminal/nonterminal.
+  // Compared to names in the grammar,
+  //   nonterminals `ptr-declartor` becomes `ptr_declarator`;
+  //   terminal `,` becomes `comma`;
+  //   terminal `IDENTIFIER` becomes `identifier`;
+  //   terminal `INT` becomes `int`;
+  // NOTE: for nonterminals, the mangled name is the same as the cxx::Symbol
+  // enum class; for terminals, we deliberately stripped the `kw_` prefix in
+  // favor of the simplicity.
+  std::string mangleSymbol(SymbolID) const;
+  // Gets the mangled name for the rule.
+  // E.g. for the grammar rule `ptr-declarator := ptr-operator ptr-declarator`,
+  // it is `ptr_declarator_0ptr_operator_1ptr_declarator`.
+  std::string mangleRule(RuleID) const;
+
   // Lookup the SymbolID of the nonterminal symbol by Name.
   llvm::Optional findNonterminal(llvm::StringRef Name) const;
 

diff  --git a/clang-tools-extra/pseudo/lib/grammar/Grammar.cpp 
b/clang-tools-extra/pseudo/lib/grammar/Grammar.cpp
index 149d97f28a778..da4e2dfd7a542 100644
--- a/clang-tools-extra/pseudo/lib/grammar/Grammar.cpp
+++ b/clang-tools-extra/pseudo/lib/grammar/Grammar.cpp
@@ -45,6 +45,28 @@ llvm::StringRef Grammar::symbolName(SymbolID SID) const {
   return T->Nonterminals[SID].Name;
 }
 
+std::string Grammar::mangleSymbol(SymbolID SID) const {
+  static const char *const TokNames[] = {
+#define TOK(X) #X,
+#define KEYWORD(X, Y) #X,
+#include "clang/Basic/TokenKinds.def"
+  nullptr};
+  if (clang::pseudo::isToken(SID))
+re

[PATCH] D129359: [pseudo] Generate an enum type for identifying grammar rules.

2022-07-15 Thread Haojian Wu via 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 rG231535890655: [pseudo] Generate an enum type for identifying 
grammar rules. (authored by hokein).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129359

Files:
  clang-tools-extra/pseudo/gen/Main.cpp
  clang-tools-extra/pseudo/include/clang-pseudo/cxx/CXX.h
  clang-tools-extra/pseudo/include/clang-pseudo/grammar/Grammar.h
  clang-tools-extra/pseudo/lib/grammar/Grammar.cpp
  clang-tools-extra/pseudo/unittests/GrammarTest.cpp

Index: clang-tools-extra/pseudo/unittests/GrammarTest.cpp
===
--- clang-tools-extra/pseudo/unittests/GrammarTest.cpp
+++ clang-tools-extra/pseudo/unittests/GrammarTest.cpp
@@ -114,6 +114,21 @@
   EXPECT_NE(G.lookupRule(ruleFor("x")).Guard, G.lookupRule(ruleFor("y")).Guard);
 }
 
+TEST_F(GrammarTest, MangleName) {
+  build(R"bnf(
+_ := declaration
+
+declaration := ptr-declarator ;
+ptr-declarator := * IDENTIFIER
+
+  )bnf");
+  ASSERT_TRUE(Diags.empty());
+  EXPECT_EQ(G.mangleRule(ruleFor("declaration")),
+"declaration_0ptr_declarator_1semi");
+  EXPECT_EQ(G.mangleRule(ruleFor("ptr-declarator")),
+"ptr_declarator_0star_1identifier");
+}
+
 TEST_F(GrammarTest, Diagnostics) {
   build(R"cpp(
 _ := ,_opt
Index: clang-tools-extra/pseudo/lib/grammar/Grammar.cpp
===
--- clang-tools-extra/pseudo/lib/grammar/Grammar.cpp
+++ clang-tools-extra/pseudo/lib/grammar/Grammar.cpp
@@ -45,6 +45,28 @@
   return T->Nonterminals[SID].Name;
 }
 
+std::string Grammar::mangleSymbol(SymbolID SID) const {
+  static const char *const TokNames[] = {
+#define TOK(X) #X,
+#define KEYWORD(X, Y) #X,
+#include "clang/Basic/TokenKinds.def"
+  nullptr};
+  if (clang::pseudo::isToken(SID))
+return TokNames[clang::pseudo::symbolToToken(SID)];
+  std::string Name = symbolName(SID).str();
+  // translation-unit -> translation_unit
+  std::replace(Name.begin(), Name.end(), '-', '_');
+  return Name;
+}
+
+std::string Grammar::mangleRule(RuleID RID) const {
+  const auto &R = lookupRule(RID);
+  std::string MangleName = mangleSymbol(R.Target);
+  for (size_t I = 0; I < R.seq().size(); ++I)
+MangleName += llvm::formatv("_{0}{1}", I, mangleSymbol(R.seq()[I]));
+  return MangleName;
+}
+
 llvm::Optional Grammar::findNonterminal(llvm::StringRef Name) const {
   auto It = llvm::partition_point(
   T->Nonterminals,
Index: clang-tools-extra/pseudo/include/clang-pseudo/grammar/Grammar.h
===
--- clang-tools-extra/pseudo/include/clang-pseudo/grammar/Grammar.h
+++ clang-tools-extra/pseudo/include/clang-pseudo/grammar/Grammar.h
@@ -165,6 +165,21 @@
   // Terminals have names like "," (kw_comma) or "OPERATOR" (kw_operator).
   llvm::StringRef symbolName(SymbolID) const;
 
+  // Gets the mangled name for a terminal/nonterminal.
+  // Compared to names in the grammar,
+  //   nonterminals `ptr-declartor` becomes `ptr_declarator`;
+  //   terminal `,` becomes `comma`;
+  //   terminal `IDENTIFIER` becomes `identifier`;
+  //   terminal `INT` becomes `int`;
+  // NOTE: for nonterminals, the mangled name is the same as the cxx::Symbol
+  // enum class; for terminals, we deliberately stripped the `kw_` prefix in
+  // favor of the simplicity.
+  std::string mangleSymbol(SymbolID) const;
+  // Gets the mangled name for the rule.
+  // E.g. for the grammar rule `ptr-declarator := ptr-operator ptr-declarator`,
+  // it is `ptr_declarator_0ptr_operator_1ptr_declarator`.
+  std::string mangleRule(RuleID) const;
+
   // Lookup the SymbolID of the nonterminal symbol by Name.
   llvm::Optional findNonterminal(llvm::StringRef Name) const;
 
Index: clang-tools-extra/pseudo/include/clang-pseudo/cxx/CXX.h
===
--- clang-tools-extra/pseudo/include/clang-pseudo/cxx/CXX.h
+++ clang-tools-extra/pseudo/include/clang-pseudo/cxx/CXX.h
@@ -37,6 +37,12 @@
 #undef NONTERMINAL
 };
 
+enum class Rule : RuleID {
+#define RULE(X, Y) X = Y,
+#include "CXXSymbols.inc"
+#undef RULE
+};
+
 enum class Extension : ExtensionID {
 #define EXTENSION(X, Y) X = Y,
 #include "CXXSymbols.inc"
Index: clang-tools-extra/pseudo/gen/Main.cpp
===
--- clang-tools-extra/pseudo/gen/Main.cpp
+++ clang-tools-extra/pseudo/gen/Main.cpp
@@ -83,17 +83,19 @@
 #ifndef NONTERMINAL
 #define NONTERMINAL(X, Y)
 #endif
+#ifndef RULE
+#define RULE(X, Y)
+#endif
 #ifndef EXTENSION
 #define EXTENSION(X, Y)
 #endif
-)cpp";
+)cpp";
 for (clang::pseudo::SymbolID ID = 0; ID < G.table().Nonterminals.size();
- ++ID) {
-  std::string Name = G.symbolName(ID).str();
-  // translation-unit -> t

[PATCH] D109701: [clang] Emit SARIF Diagnostics: Create `clang::SarifDocumentWriter` interface

2022-07-15 Thread Vaibhav Yenamandra via Phabricator via cfe-commits
vaibhav.y updated this revision to Diff 444960.
vaibhav.y added a comment.

Undo test case renames


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109701

Files:
  clang/include/clang/Basic/Sarif.h
  clang/lib/Basic/CMakeLists.txt
  clang/lib/Basic/Sarif.cpp
  clang/unittests/Basic/CMakeLists.txt
  clang/unittests/Basic/SarifTest.cpp

Index: clang/unittests/Basic/SarifTest.cpp
===
--- /dev/null
+++ clang/unittests/Basic/SarifTest.cpp
@@ -0,0 +1,325 @@
+//===- unittests/Basic/SarifTest.cpp - Test writing SARIF documents ---===//
+//
+// 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
+//
+//===--===//
+
+#include "clang/Basic/Sarif.h"
+#include "clang/Basic/DiagnosticIDs.h"
+#include "clang/Basic/DiagnosticOptions.h"
+#include "clang/Basic/FileManager.h"
+#include "clang/Basic/FileSystemOptions.h"
+#include "clang/Basic/LangOptions.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/FormatVariadic.h"
+#include "llvm/Support/JSON.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/VirtualFileSystem.h"
+#include "llvm/Support/raw_ostream.h"
+#include "gmock/gmock-matchers.h"
+#include "gtest/gtest-death-test.h"
+#include "gtest/gtest-matchers.h"
+#include "gtest/gtest.h"
+
+#include 
+
+using namespace clang;
+
+namespace {
+
+using LineCol = std::pair;
+
+static std::string serializeSarifDocument(llvm::json::Object &&Doc) {
+  std::string Output;
+  llvm::json::Value value(std::move(Doc));
+  llvm::raw_string_ostream OS{Output};
+  OS << llvm::formatv("{0}", value);
+  OS.flush();
+  return Output;
+}
+
+class SarifDocumentWriterTest : public ::testing::Test {
+protected:
+  SarifDocumentWriterTest()
+  : InMemoryFileSystem(new llvm::vfs::InMemoryFileSystem),
+FileMgr(FileSystemOptions(), InMemoryFileSystem),
+DiagID(new DiagnosticIDs()), DiagOpts(new DiagnosticOptions()),
+Diags(DiagID, DiagOpts.get(), new IgnoringDiagConsumer()),
+SourceMgr(Diags, FileMgr) {}
+
+  IntrusiveRefCntPtr InMemoryFileSystem;
+  FileManager FileMgr;
+  IntrusiveRefCntPtr DiagID;
+  IntrusiveRefCntPtr DiagOpts;
+  DiagnosticsEngine Diags;
+  SourceManager SourceMgr;
+  LangOptions LangOpts;
+
+  FileID registerSource(llvm::StringRef Name, const char *SourceText,
+bool IsMainFile = false) {
+std::unique_ptr SourceBuf =
+llvm::MemoryBuffer::getMemBuffer(SourceText);
+const FileEntry *SourceFile =
+FileMgr.getVirtualFile(Name, SourceBuf->getBufferSize(), 0);
+SourceMgr.overrideFileContents(SourceFile, std::move(SourceBuf));
+FileID FID = SourceMgr.getOrCreateFileID(SourceFile, SrcMgr::C_User);
+if (IsMainFile)
+  SourceMgr.setMainFileID(FID);
+return FID;
+  }
+
+  CharSourceRange getFakeCharSourceRange(FileID FID, LineCol Begin,
+ LineCol End) {
+auto BeginLoc = SourceMgr.translateLineCol(FID, Begin.first, Begin.second);
+auto EndLoc = SourceMgr.translateLineCol(FID, End.first, End.second);
+return CharSourceRange{SourceRange{BeginLoc, EndLoc}, /* ITR = */ false};
+  }
+};
+
+TEST_F(SarifDocumentWriterTest, canCreateEmptyDocument) {
+  // GIVEN:
+  SarifDocumentWriter Writer{SourceMgr};
+
+  // WHEN:
+  const llvm::json::Object &EmptyDoc = Writer.createDocument();
+  std::vector Keys(EmptyDoc.size());
+  std::transform(EmptyDoc.begin(), EmptyDoc.end(), Keys.begin(),
+ [](auto item) { return item.getFirst(); });
+
+  // THEN:
+  ASSERT_THAT(Keys, testing::UnorderedElementsAre("$schema", "version"));
+}
+
+// Test that a newly inserted run will associate correct tool names
+TEST_F(SarifDocumentWriterTest, canCreateDocumentWithOneRun) {
+  // GIVEN:
+  SarifDocumentWriter Writer{SourceMgr};
+  const char *ShortName = "sariftest";
+  const char *LongName = "sarif writer test";
+
+  // WHEN:
+  Writer.createRun(ShortName, LongName);
+  Writer.endRun();
+  const llvm::json::Object &Doc = Writer.createDocument();
+  const llvm::json::Array *Runs = Doc.getArray("runs");
+
+  // THEN:
+  // A run was created
+  ASSERT_THAT(Runs, testing::NotNull());
+
+  // It is the only run
+  ASSERT_EQ(Runs->size(), 1UL);
+
+  // The tool associated with the run was the tool
+  const llvm::json::Object *driver =
+  Runs->begin()->getAsObject()->getObject("tool")->getObject("driver");
+  ASSERT_THAT(driver, testing::NotNull());
+
+  ASSERT_TRUE(driver->getString("name").hasValue());
+  ASSERT_TRUE(driver->getString("fullName").hasValue());
+  ASSERT_TRUE(driver->getString("language").hasValue());
+
+  EXPECT_EQ(dri

[clang-tools-extra] 76910d4 - [pseudo] Share the underly payload when stripping comments for a token stream

2022-07-15 Thread Haojian Wu via cfe-commits

Author: Haojian Wu
Date: 2022-07-15T15:20:48+02:00
New Revision: 76910d4a56c8dba000f198bba13e71cf0492c8cb

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

LOG: [pseudo] Share the underly payload when stripping comments for a token 
stream

`stripComments(cook(...))` is a common pattern being written.
Without this patch, this has a use-after-free issue (cook returns a temporary
TokenStream object which has its own payload, but the payload is not
shared with the one returned by stripComments).

Reviewed By: sammccall

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

Added: 


Modified: 
clang-tools-extra/pseudo/include/clang-pseudo/Token.h
clang-tools-extra/pseudo/lib/Lex.cpp
clang-tools-extra/pseudo/lib/Token.cpp

Removed: 




diff  --git a/clang-tools-extra/pseudo/include/clang-pseudo/Token.h 
b/clang-tools-extra/pseudo/include/clang-pseudo/Token.h
index b558891f0a86..36e5221a0d30 100644
--- a/clang-tools-extra/pseudo/include/clang-pseudo/Token.h
+++ b/clang-tools-extra/pseudo/include/clang-pseudo/Token.h
@@ -170,6 +170,18 @@ class TokenStream {
 return Storage[1];
   }
 
+  /// Returns the shared payload.
+  std::shared_ptr getPayload() const { return Payload; }
+  /// Adds the given payload to the stream.
+  void addPayload(std::shared_ptr P) {
+if (!Payload)
+  Payload = std::move(P);
+else
+  Payload = std::make_shared<
+  std::pair, std::shared_ptr>>(
+  std::move(P), std::move(Payload));
+  }
+
   /// Print the tokens in this stream to the output stream.
   ///
   /// The presence of newlines/spaces is preserved, but not the quantity.

diff  --git a/clang-tools-extra/pseudo/lib/Lex.cpp 
b/clang-tools-extra/pseudo/lib/Lex.cpp
index 6a5a10f1b979..c96e2f27cba9 100644
--- a/clang-tools-extra/pseudo/lib/Lex.cpp
+++ b/clang-tools-extra/pseudo/lib/Lex.cpp
@@ -77,7 +77,7 @@ TokenStream cook(const TokenStream &Code, const LangOptions 
&LangOpts) {
   auto CleanedStorage = std::make_shared();
   clang::IdentifierTable Identifiers(LangOpts);
   TokenStream Result(CleanedStorage);
-
+  Result.addPayload(Code.getPayload());
   for (auto Tok : Code.tokens()) {
 if (Tok.flag(LexFlags::NeedsCleaning)) {
   // Remove escaped newlines and trigraphs.

diff  --git a/clang-tools-extra/pseudo/lib/Token.cpp 
b/clang-tools-extra/pseudo/lib/Token.cpp
index b58c8e4a862e..5b07a62f37fb 100644
--- a/clang-tools-extra/pseudo/lib/Token.cpp
+++ b/clang-tools-extra/pseudo/lib/Token.cpp
@@ -116,7 +116,7 @@ clang::LangOptions genericLangOpts(clang::Language Lang,
 }
 
 TokenStream stripComments(const TokenStream &Input) {
-  TokenStream Out;
+  TokenStream Out(Input.getPayload());
   for (const Token &T : Input.tokens()) {
 if (T.Kind == tok::comment)
   continue;



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


[PATCH] D125311: [pseudo] Share the underly payload when stripping comments for a token stream

2022-07-15 Thread Haojian Wu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG76910d4a56c8: [pseudo] Share the underly payload when 
stripping comments for a token stream (authored by hokein).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125311

Files:
  clang-tools-extra/pseudo/include/clang-pseudo/Token.h
  clang-tools-extra/pseudo/lib/Lex.cpp
  clang-tools-extra/pseudo/lib/Token.cpp


Index: clang-tools-extra/pseudo/lib/Token.cpp
===
--- clang-tools-extra/pseudo/lib/Token.cpp
+++ clang-tools-extra/pseudo/lib/Token.cpp
@@ -116,7 +116,7 @@
 }
 
 TokenStream stripComments(const TokenStream &Input) {
-  TokenStream Out;
+  TokenStream Out(Input.getPayload());
   for (const Token &T : Input.tokens()) {
 if (T.Kind == tok::comment)
   continue;
Index: clang-tools-extra/pseudo/lib/Lex.cpp
===
--- clang-tools-extra/pseudo/lib/Lex.cpp
+++ clang-tools-extra/pseudo/lib/Lex.cpp
@@ -77,7 +77,7 @@
   auto CleanedStorage = std::make_shared();
   clang::IdentifierTable Identifiers(LangOpts);
   TokenStream Result(CleanedStorage);
-
+  Result.addPayload(Code.getPayload());
   for (auto Tok : Code.tokens()) {
 if (Tok.flag(LexFlags::NeedsCleaning)) {
   // Remove escaped newlines and trigraphs.
Index: clang-tools-extra/pseudo/include/clang-pseudo/Token.h
===
--- clang-tools-extra/pseudo/include/clang-pseudo/Token.h
+++ clang-tools-extra/pseudo/include/clang-pseudo/Token.h
@@ -170,6 +170,18 @@
 return Storage[1];
   }
 
+  /// Returns the shared payload.
+  std::shared_ptr getPayload() const { return Payload; }
+  /// Adds the given payload to the stream.
+  void addPayload(std::shared_ptr P) {
+if (!Payload)
+  Payload = std::move(P);
+else
+  Payload = std::make_shared<
+  std::pair, std::shared_ptr>>(
+  std::move(P), std::move(Payload));
+  }
+
   /// Print the tokens in this stream to the output stream.
   ///
   /// The presence of newlines/spaces is preserved, but not the quantity.


Index: clang-tools-extra/pseudo/lib/Token.cpp
===
--- clang-tools-extra/pseudo/lib/Token.cpp
+++ clang-tools-extra/pseudo/lib/Token.cpp
@@ -116,7 +116,7 @@
 }
 
 TokenStream stripComments(const TokenStream &Input) {
-  TokenStream Out;
+  TokenStream Out(Input.getPayload());
   for (const Token &T : Input.tokens()) {
 if (T.Kind == tok::comment)
   continue;
Index: clang-tools-extra/pseudo/lib/Lex.cpp
===
--- clang-tools-extra/pseudo/lib/Lex.cpp
+++ clang-tools-extra/pseudo/lib/Lex.cpp
@@ -77,7 +77,7 @@
   auto CleanedStorage = std::make_shared();
   clang::IdentifierTable Identifiers(LangOpts);
   TokenStream Result(CleanedStorage);
-
+  Result.addPayload(Code.getPayload());
   for (auto Tok : Code.tokens()) {
 if (Tok.flag(LexFlags::NeedsCleaning)) {
   // Remove escaped newlines and trigraphs.
Index: clang-tools-extra/pseudo/include/clang-pseudo/Token.h
===
--- clang-tools-extra/pseudo/include/clang-pseudo/Token.h
+++ clang-tools-extra/pseudo/include/clang-pseudo/Token.h
@@ -170,6 +170,18 @@
 return Storage[1];
   }
 
+  /// Returns the shared payload.
+  std::shared_ptr getPayload() const { return Payload; }
+  /// Adds the given payload to the stream.
+  void addPayload(std::shared_ptr P) {
+if (!Payload)
+  Payload = std::move(P);
+else
+  Payload = std::make_shared<
+  std::pair, std::shared_ptr>>(
+  std::move(P), std::move(Payload));
+  }
+
   /// Print the tokens in this stream to the output stream.
   ///
   /// The presence of newlines/spaces is preserved, but not the quantity.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D129748: [Modules] Disable preferred_name attribute in C++20 Modules

2022-07-15 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment.

In D129748#3653897 , @ChuanqiXu wrote:

> In D129748#3651771 , @erichkeane 
> wrote:
>
>> I guess I don't have a good idea why this attribute would cause ODR issues?  
>> It would seem that if it appeared in 2 different TUs that we could just 
>> 'pick' whichever we wanted, right?
>
> If the compiler finds it appeared in 2 different TUs with different 
> definition (although the judgement is wrong), the compiler would emit an 
> error. So it would block the uses of C++20 Modules with `preferred_name`.
>
>> The description in the bug report of the problem isn't clear to me what the 
>> actual issue is.
>
> Sorry. My bad. Let me try to clarify it. When we write the attribute 
> `preferred_name(foo)` in ASTWriter, the compiler would try to write the type 
> for the argument `foo`. Then when the compiler write the type for `foo`, the 
> compiler find the type for `foo` is a TypeDef type. So the compiler would 
> write the corresponding type `foo_templ`. The key point here is that 
> the AST for `foo_templ` is complete now. Since the AST for 
> `foo_templ` is constructed in Sema.
>
> But problem comes when we read it. When we read the attribute 
> `preferred_name(foo)`, we would read the type for the argument `foo` and then 
> we would try to read the type `foo_templ` later. However, the key 
> problem here is that when we read `foo_templ`, its AST is not 
> constructed yet! So we get a different type with the writer writes. So here 
> is the ODR violation.
>
> The problem is fundamental and I've spent 2 weeks on it. But I don't find any 
> fixes for it. Then I found that, once I disabled `preferred_name`, we could 
> go much further. So I am wondering if it would be an option to skip 
> `preferred_name` if C++ modules is enabled. The idea may not be good in 
> general. But I feel it might be an option in this specific case given it is 
> hard to solve and `preferred_name` is primarily for printers.

Hmm... interesting.  I wish I had a better understanding of the ASTWriter to 
help with this, but given:

1- Getting modules compiled is important
2- this attribute is 'ignore-able', and is for diagnostics only
3- the ASTWriter/ASTReader seems to be messing this up.

I think there _IS_ perhaps an acceptability to ignoring this attribute when it 
would cause a problem.  However, I think doing it as you're doing it now is 
wrong.  IF we are going to solve the symptom here, I think we should use a much 
more precise cut, and make either ASTReader not emit the attribute, or 
ASTWriter just 'ignore' it when reading.  WDYT?  @aaron.ballman as well...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129748

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


[PATCH] D129748: [Modules] Disable preferred_name attribute in C++20 Modules

2022-07-15 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D129748#3654909 , @erichkeane 
wrote:

> I think there _IS_ perhaps an acceptability to ignoring this attribute when 
> it would cause a problem.  However, I think doing it as you're doing it now 
> is wrong.  IF we are going to solve the symptom here, I think we should use a 
> much more precise cut, and make either ASTReader not emit the attribute, or 
> ASTWriter just 'ignore' it when reading.  WDYT?  @aaron.ballman as well...

I'm less convinced that's reasonable, because that root cause seems like it 
will impact several other attributes as well. For example, I would be 
`IBOutletCollection`, `OwnerAttr`, `PointerAttr`, and `TypeTagForDatatypeAttr` 
all behave the same way as they all take a type argument. I don't think we want 
to selectively disable so many attributes (for example, disallowing owner and 
pointer attributes means we lose out on C++ Core Guideline features that people 
will likely expect to be able to use with modules. However, I agree that being 
able to modularize the STL is an important use case.

I'd like to understand better what the root cause is. The attribute requires a 
resolved type name, which means we must have seen the type before the attribute 
when writing the AST out. When reading the AST back in, why is the type not 
visible before the attribute? That sounds like we're writing the AST out in the 
wrong order somehow, or something odd like that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129748

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


[PATCH] D129280: [analyzer] PlacementNewChecker, properly handle array overhead (cookie)

2022-07-15 Thread Gabor Marton via Phabricator via cfe-commits
martong added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129280

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


[PATCH] D109701: [clang] Emit SARIF Diagnostics: Create `clang::SarifDocumentWriter` interface

2022-07-15 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision.
aaron.ballman added a comment.

LGTM assuming precommit CI comes back happy with it, thank you! I'll land it 
once I see things are green.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109701

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


[PATCH] D129045: [C++20][Modules] Update handling of implicit inlines [P1779R3]

2022-07-15 Thread Iain Sandoe via Phabricator via cfe-commits
iains reopened this revision.
iains added a comment.
This revision is now accepted and ready to land.

reopening to post the patch I plan to  re-land.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129045

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


[PATCH] D129045: [C++20][Modules] Update handling of implicit inlines [P1779R3]

2022-07-15 Thread Iain Sandoe via Phabricator via cfe-commits
iains updated this revision to Diff 444965.
iains added a comment.

rebased, fixed interaction with clang modules.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129045

Files:
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/test/AST/ast-dump-constant-expr.cpp
  clang/test/AST/ast-dump-lambda.cpp
  clang/test/CXX/class/class.friend/p7-cxx20.cpp
  clang/test/CXX/class/class.mfct/p1-cxx20.cpp

Index: clang/test/CXX/class/class.mfct/p1-cxx20.cpp
===
--- /dev/null
+++ clang/test/CXX/class/class.mfct/p1-cxx20.cpp
@@ -0,0 +1,57 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+// RUN: cd %t
+//
+// RUN: %clang_cc1 -std=c++20 no-modules.cpp -fsyntax-only -ast-dump | \
+// RUN: FileCheck --match-full-lines --check-prefix=CHECK-NM %s
+// RUN: %clang_cc1 -std=c++20 -xc++-user-header header-unit.h -ast-dump | \
+// RUN: FileCheck --match-full-lines --check-prefix=CHECK-HU %s
+// RUN: %clang_cc1 -std=c++20 module.cpp -ast-dump | \
+// RUN: FileCheck --match-full-lines --check-prefix=CHECK-MOD %s
+
+//--- no-modules.cpp
+
+class X {
+  void x(){};
+};
+
+// CHECK-NM: `-CXXRecordDecl {{.*}}  line:2:7 class X definition
+// CHECK-NM:   |-CXXRecordDecl {{.*}}  col:7 implicit class X
+// CHECK-NM-NEXT: `-CXXMethodDecl {{.*}}  col:8 x 'void ()' implicit-inline
+
+// A header unit header
+//--- header-unit.h
+
+class Y {
+  void y(){};
+};
+
+// CHECK-HU: `-CXXRecordDecl {{.*}} <./header-unit.h:2:1, line:4:1> line:2:7 class Y definition
+// CHECK-HU: |-CXXRecordDecl {{.*}}  col:7 implicit class Y
+// CHECK-HU-NEXT: `-CXXMethodDecl {{.*}}  col:8 y 'void ()' implicit-inline
+
+// A textually-included header
+//--- header.h
+
+class A {
+  void a(){};
+};
+
+//--- module.cpp
+module;
+#include "header.h"
+
+export module M;
+
+class Z {
+  void z(){};
+};
+
+// CHECK-MOD: |-CXXRecordDecl {{.*}} <./header.h:2:1, line:4:1> line:2:7 in M. hidden class A definition
+// CHECK-MOD: | |-CXXRecordDecl {{.*}}  col:7 in M. hidden implicit class A
+// CHECK-MOD-NEXT: | `-CXXMethodDecl {{.*}}  col:8 in M. hidden a 'void ()' implicit-inline
+
+// CHECK-MOD: `-CXXRecordDecl {{.*}}  line:6:7 in M hidden class Z{{( ReachableWhenImported)?}} definition
+// CHECK-MOD: |-CXXRecordDecl {{.*}}  col:7 in M hidden implicit class Z{{( ReachableWhenImported)?}}
+// CHECK-MOD-NEXT: `-CXXMethodDecl {{.*}}  col:8 in M hidden z 'void ()'{{( ReachableWhenImported)?}}
Index: clang/test/CXX/class/class.friend/p7-cxx20.cpp
===
--- /dev/null
+++ clang/test/CXX/class/class.friend/p7-cxx20.cpp
@@ -0,0 +1,59 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+// RUN: cd %t
+//
+// RUN: %clang_cc1 -std=c++20 no-modules.cpp -fsyntax-only -ast-dump | \
+// RUN: FileCheck --match-full-lines --check-prefix=CHECK-NM %s
+// RUN: %clang_cc1 -std=c++20 -xc++-user-header header-unit.h -ast-dump | \
+// RUN: FileCheck --match-full-lines --check-prefix=CHECK-HU %s
+// RUN: %clang_cc1 -std=c++20 module.cpp -ast-dump | \
+// RUN: FileCheck --match-full-lines --check-prefix=CHECK-MOD %s
+
+//--- no-modules.cpp
+
+class X {
+  friend void x(){};
+};
+
+// CHECK-NM: `-CXXRecordDecl {{.*}}  line:2:7 class X definition
+// CHECK-NM:   |-CXXRecordDecl {{.*}}  col:7 implicit class X
+// CHECK-NM-NEXT: `-FriendDecl {{.*}}  col:15
+// CHECK-NM-NEXT: `-FunctionDecl {{.*}} parent {{.*}}  col:15 x 'void ()' implicit-inline
+
+//--- header-unit.h
+
+class Y {
+  friend void y(){};
+};
+
+// CHECK-HU: `-CXXRecordDecl {{.*}} <./header-unit.h:2:1, line:4:1> line:2:7 class Y definition
+// CHECK-HU: |-CXXRecordDecl {{.*}}  col:7 implicit class Y
+// CHECK-HU-NEXT: `-FriendDecl {{.*}}  col:15
+// CHECK-HU-NEXT: `-FunctionDecl {{.*}} parent {{.*}}  col:15 y 'void ()' implicit-inline
+
+// A textually-included header
+//--- header.h
+
+class A {
+  friend void a(){};
+};
+
+//--- module.cpp
+module;
+#include "header.h"
+
+export module M;
+
+class Z {
+  friend void z(){};
+};
+// CHECK-MOD: |-CXXRecordDecl {{.*}} <./header.h:2:1, line:4:1> line:2:7 in M. hidden class A definition
+// CHECK-MOD: | |-CXXRecordDecl {{.*}}  col:7 in M. hidden implicit class A
+// CHECK-MOD-NEXT: | `-FriendDecl {{.*}}  col:15 in M.
+// CHECK-MOD-NEXT: |   `-FunctionDecl {{.*}} parent {{.*}}  col:15 in M. hidden a 'void ()' implicit-inline
+
+// CHECK-MOD: `-CXXRecordDecl {{.*}}  line:6:7 in M hidden class Z{{( ReachableWhenImported)?}} definition
+// CHECK-MOD: |-CXXRecordDecl {{.*}}  col:7 in M hidden implicit class Z{{( ReachableWhenImported)?}}
+// CHECK-MOD-NEXT: `-FriendDecl {{.*}}  col:15 in M{{( ReachableWhenImported)?}}
+// CHECK-MOD-NEXT: `-FunctionDecl {{.*}} parent {{.*}}  col:15 in M hidden z 'void ()'{{( ReachableWhenImported)?}}
Index: clang/test/AST/ast-dump-lambda.cpp
=

[PATCH] D129802: [DRAFT] Implementing new atomic orderings in LLVM and generate barriers for legacy __sync builtins. Support corresponding memory model in outline atomics as well.

2022-07-15 Thread Wilco Dijkstra via Phabricator via cfe-commits
Wilco1 added a comment.

The general requirement is that inline and outline atomics have identical 
behaviour, and that GCC and LLVM emit the same sequences. I agree __sync is 
badly documented, so it's hard to figure whether an extra DMB barrier could 
actually make a difference, but it's best to be conservative with atomics. Also 
it was trivial to add (GCC just adds an extra flag for __sync which then emits 
the extra barrier if the flag is set, so you don't need to introduce new atomic 
models).

However if __sync primitives are hardly used in the real world then perhaps it 
is about time to deprecate them with annoying warnings, and completely remove 
support next year. Does that sound reasonable?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129802

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


[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

2022-07-15 Thread Barry Revzin via Phabricator via cfe-commits
BRevzin added inline comments.



Comment at: clang/test/AST/conditionally-trivial-smfs.cpp:39
+
+template struct DefaultConstructorCheck<2>;
+// CHECK: "kind": "ClassTemplateSpecializationDecl",

It's possible that I just don't understand what these tests actually mean 
but... where is the test for `DefaultConstructorCheck<2>` having a deleted 
default constructor, or `DefaultConstructorCheck<3>` having a non-defaulted one?

It'd also be worthwhile to have at least one test with constaints that subsume 
each other instead of being mutually exclusive. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128619

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


[PATCH] D129359: [pseudo] Generate an enum type for identifying grammar rules.

2022-07-15 Thread Sam McCall via Phabricator via cfe-commits
sammccall added inline comments.



Comment at: clang-tools-extra/pseudo/lib/grammar/Grammar.cpp:48
 
+std::string Grammar::mangleSymbol(SymbolID SID) const {
+  static const char *const TokNames[] = {

hokein wrote:
> sammccall wrote:
> > I'm not sure exposing these from `Grammar` is an improvement, i think even 
> > in principle we'd only want to use these for codegen.
> > 
> > The need for testing is real, but i guess you can add a test that uses the 
> > CXX generated grammar, assert the elements of Rule::whatever, and the name 
> > of Symbol::whatever?
> Yeah, these functions are only used in the codegen. I'm inlined to the put it 
> into the `Grammar`(it also has a `symbolName` definition, I'd prefer to have 
> all naming functions in a single place). 
That's exactly my point though: that symbolname is general purpose and should 
be used everywhere, which is why it belongs on grammar.

Vs the mangled name that literally nobody should be using for anything.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129359

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


[PATCH] D129855: [clang][PowerPC] Set lld as clang's default linker for PowerPC Linux

2022-07-15 Thread Quinn Pham via Phabricator via cfe-commits
quinnp created this revision.
Herald added subscribers: steven.zhang, shchenz, kbarton, nemanjai.
Herald added a project: All.
quinnp requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay.
Herald added a project: clang.

This patch changes the default linker for `clang` on PowerPC Linux to `lld`.
Here is a summary of the expected behaviour before and after this patch:

To use `lld` as the linker before this patch:

- build with `lld` in `LLVM_ENABLE_PROJECTS`
- build with `-DCLANG_DEFAULT_LINKER=lld`

To use `lld` as the linker after this patch:

- build with `lld` in `LLVM_ENABLE_PROJECTS`

To use `ld` as the linker before this patch:

- default behaviour

To use `ld` as the linker after this patch:

- build with `-DCLANG_DEFAULT_LINKER=`

Note: After this patch, if you build `clang` for PowerPC Linux and `lld` is not
included in `LLVM_ENABLE_PROJECTS`, the built compiler will report an error
during linking on PowerPC Linux. Therefore, anyone using the default behaviour
before this patch will need to modify their build configuration to either:

- include `-DCLANG_DEFAULT_LINKER=` to continue using `ld` or
- `lld` in `LLVM_ENABLE_PROJECTS` to switch to `lld`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D129855

Files:
  clang/lib/Driver/ToolChains/PPCLinux.h


Index: clang/lib/Driver/ToolChains/PPCLinux.h
===
--- clang/lib/Driver/ToolChains/PPCLinux.h
+++ clang/lib/Driver/ToolChains/PPCLinux.h
@@ -24,6 +24,8 @@
   AddClangSystemIncludeArgs(const llvm::opt::ArgList &DriverArgs,
 llvm::opt::ArgStringList &CC1Args) const override;
 
+  const char *getDefaultLinker() const override { return "ld.lld"; }
+
 private:
   bool SupportIEEEFloat128(const Driver &D, const llvm::Triple &Triple,
const llvm::opt::ArgList &Args) const;


Index: clang/lib/Driver/ToolChains/PPCLinux.h
===
--- clang/lib/Driver/ToolChains/PPCLinux.h
+++ clang/lib/Driver/ToolChains/PPCLinux.h
@@ -24,6 +24,8 @@
   AddClangSystemIncludeArgs(const llvm::opt::ArgList &DriverArgs,
 llvm::opt::ArgStringList &CC1Args) const override;
 
+  const char *getDefaultLinker() const override { return "ld.lld"; }
+
 private:
   bool SupportIEEEFloat128(const Driver &D, const llvm::Triple &Triple,
const llvm::opt::ArgList &Args) const;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

2022-07-15 Thread Chris Hamilton via Phabricator via cfe-commits
chrish_ericsson_atx added a comment.

In D112374#3653967 , @JDevlieghere 
wrote:

> I don't. I think reverting your change was well within the guidelines 
> outlined by LLVM's patch reversion policy: 
> https://llvm.org/docs/DeveloperPolicy.html#patch-reversion-policy
>
> Additionally, I think you could've given me a little bit more time to catch 
> up on the discussion here. The code review policy and practices 
> (https://llvm.org/docs/CodeReview.html#code-reviews-speed-and-reciprocity) 
> recommend pinging every few days to once per week depending on how urgent the 
> patch is.
>
> By relanding, you broke the bots again 
> (https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/45354/#showFailuresLink)
>  and I'm forced to revert this change a second time. Please refrain from 
> landing this again until we've settled on a way forward.

I agree with @JDevlieghere here. In general, it's reasonable that the patch was 
reverted when it was found to break other things in the LLVM project, and 
reasonable to expect the original author's cooperation in resolving that 
breakage before the patch gets reapplied--especially when the patch is as large 
and far-reaching as this one is.

As an aside, this also patch breaks one of our internal tidy checkers. Likely 
the fix we'll need for that checker is simple, but it'll take a bit of research 
for me to understand what needs to happen in our internal code. Thanks for your 
patience and willingness to help your colleagues understand and adapt to the 
change you've authored.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112374

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


[PATCH] D129648: Use pseudo parser for folding ranges

2022-07-15 Thread Utkarsh Saxena via Phabricator via cfe-commits
usaxena95 updated this revision to Diff 444975.
usaxena95 marked 15 inline comments as done.
usaxena95 added a comment.

Addressed comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129648

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/SemanticSelection.cpp
  clang-tools-extra/clangd/SemanticSelection.h
  clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp
  clang-tools-extra/pseudo/include/clang-pseudo/Token.h
  clang-tools-extra/pseudo/lib/CMakeLists.txt
  clang-tools-extra/pseudo/lib/Lex.cpp
  clang-tools-extra/pseudo/unittests/TokenTest.cpp

Index: clang-tools-extra/pseudo/unittests/TokenTest.cpp
===
--- clang-tools-extra/pseudo/unittests/TokenTest.cpp
+++ clang-tools-extra/pseudo/unittests/TokenTest.cpp
@@ -31,6 +31,10 @@
   return arg.Line == (unsigned)Line && arg.Indent == (unsigned)Indent;
 }
 
+MATCHER_P(originalIndex, index, "") {
+  return arg.OriginalIndex == (Token::Index)index;
+}
+
 TEST(TokenTest, Lex) {
   LangOptions Opts;
   std::string Code = R"cpp(
@@ -105,20 +109,23 @@
   Raw.tokens(),
   ElementsAre(AllOf(token("one_\\\ntoken", tok::raw_identifier),
 hasFlag(LexFlags::StartsPPLine),
-hasFlag(LexFlags::NeedsCleaning), lineIndent(1, 0)),
+hasFlag(LexFlags::NeedsCleaning), lineIndent(1, 0),
+originalIndex(0)),
   AllOf(token("two", tok::raw_identifier),
 hasFlag(LexFlags::StartsPPLine),
-Not(hasFlag(LexFlags::NeedsCleaning))),
+Not(hasFlag(LexFlags::NeedsCleaning)),
+originalIndex(1)),
   AllOf(token("\\\ntokens", tok::raw_identifier),
 Not(hasFlag(LexFlags::StartsPPLine)),
-hasFlag(LexFlags::NeedsCleaning;
+hasFlag(LexFlags::NeedsCleaning), originalIndex(2;
 
   TokenStream Cooked = cook(Raw, Opts);
   EXPECT_THAT(
   Cooked.tokens(),
-  ElementsAre(AllOf(token("one_token", tok::identifier), lineIndent(1, 0)),
-  token("two", tok::identifier),
-  token("tokens", tok::identifier)));
+  ElementsAre(AllOf(token("one_token", tok::identifier), lineIndent(1, 0),
+originalIndex(0)),
+  AllOf(token("two", tok::identifier), originalIndex(1)),
+  AllOf(token("tokens", tok::identifier), originalIndex(2;
 }
 
 TEST(TokenTest, EncodedCharacters) {
@@ -182,13 +189,14 @@
 )cpp";
   TokenStream Cook = cook(lex(Code, Opts), Opts);
   TokenStream Split = stripComments(Cook);
-  EXPECT_THAT(Split.tokens(), ElementsAreArray({
-  token(">", tok::greater),
-  token(">", tok::greater),
-  token(">", tok::greater),
-  token(">", tok::greater),
-  token(">>=", tok::greatergreaterequal),
-  }));
+  EXPECT_THAT(Split.tokens(),
+  ElementsAre(AllOf(token(">", tok::greater), originalIndex(0)),
+  AllOf(token(">", tok::greater), originalIndex(0)),
+  // Token and 1 and 2 are comments.
+  AllOf(token(">", tok::greater), originalIndex(3)),
+  AllOf(token(">", tok::greater), originalIndex(3)),
+  AllOf(token(">>=", tok::greatergreaterequal),
+originalIndex(4;
 }
 
 TEST(TokenTest, DropComments) {
@@ -199,13 +207,16 @@
 )cpp";
   TokenStream Raw = cook(lex(Code, Opts), Opts);
   TokenStream Stripped = stripComments(Raw);
-  EXPECT_THAT(Raw.tokens(),
-  ElementsAreArray(
-  {token("// comment", tok::comment), token("int", tok::kw_int),
-   token("/*abc*/", tok::comment), token(";", tok::semi)}));
-
-  EXPECT_THAT(Stripped.tokens(), ElementsAreArray({token("int", tok::kw_int),
-   token(";", tok::semi)}));
+  EXPECT_THAT(
+  Raw.tokens(),
+  ElementsAre(AllOf(token("// comment", tok::comment), originalIndex(0)),
+  AllOf(token("int", tok::kw_int), originalIndex(1)),
+  AllOf(token("/*abc*/", tok::comment), originalIndex(2)),
+  AllOf(token(";", tok::semi), originalIndex(3;
+
+  EXPECT_THAT(Stripped.tokens(),
+  ElementsAre(AllOf(token("int", tok::kw_int), originalIndex(1)),
+  AllOf(token(";", tok::semi), originalIndex(3;
 }
 
 } // namespace
Index: clang-tools-extra/pseudo/lib/Lex.cpp
===
--- clang-tools-extra/pseudo/l

[PATCH] D129648: Use pseudo parser for folding ranges

2022-07-15 Thread Utkarsh Saxena via Phabricator via cfe-commits
usaxena95 added inline comments.



Comment at: clang-tools-extra/clangd/SemanticSelection.cpp:185
+if (auto *Paired = Tok.pair()) {
+  if (Tok.Line < Paired->Line) {
+Position Start = offsetToPosition(

hokein wrote:
> The `if` logic seems tricky, it is doing two different things:
> 
> 1) avoid creating duplicate `FoldingRange` for a pair bracket
> 2) avoid creating a FoldingRange if the pair bracket is on the same line.
> 
> Are both intended?
Yes. Both of these are intentional. Added a comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129648

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


[PATCH] D129222: [pseudo] Implement a guard to determine function declarator.

2022-07-15 Thread Haojian Wu via Phabricator via cfe-commits
hokein updated this revision to Diff 444976.
hokein added a comment.

rebase and address the main comment -- encoding the function-declarator into 
the grammar.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129222

Files:
  clang-tools-extra/pseudo/lib/cxx/CXX.cpp
  clang-tools-extra/pseudo/lib/cxx/cxx.bnf
  clang-tools-extra/pseudo/test/cxx/declarator-function.cpp
  clang-tools-extra/pseudo/test/cxx/declarator-var.cpp
  clang-tools-extra/pseudo/test/cxx/recovery-init-list.cpp
  clang-tools-extra/pseudo/test/glr.cpp

Index: clang-tools-extra/pseudo/test/glr.cpp
===
--- clang-tools-extra/pseudo/test/glr.cpp
+++ clang-tools-extra/pseudo/test/glr.cpp
@@ -1,4 +1,4 @@
-// RUN: clang-pseudo -grammar=%cxx-bnf-file -source=%s --print-forest -print-statistics | FileCheck %s
+// RUN: clang-pseudo -grammar=cxx -source=%s --print-forest -print-statistics | FileCheck %s
 
 void foo() {
   T* a; // a multiply expression or a pointer declaration?
Index: clang-tools-extra/pseudo/test/cxx/recovery-init-list.cpp
===
--- clang-tools-extra/pseudo/test/cxx/recovery-init-list.cpp
+++ clang-tools-extra/pseudo/test/cxx/recovery-init-list.cpp
@@ -3,7 +3,7 @@
 // CHECK:  translation-unit~simple-declaration
 // CHECK-NEXT: ├─decl-specifier-seq~AUTO := tok[0]
 // CHECK-NEXT: ├─init-declarator-list~init-declarator
-// CHECK-NEXT: │ ├─declarator~IDENTIFIER := tok[1]
+// CHECK-NEXT: │ ├─non-function-declarator~IDENTIFIER := tok[1]
 // CHECK-NEXT: │ └─initializer~brace-or-equal-initializer
 // CHECK-NEXT: │   ├─= := tok[2]
 // CHECK-NEXT: │   └─initializer-clause~braced-init-list
Index: clang-tools-extra/pseudo/test/cxx/declarator-var.cpp
===
--- clang-tools-extra/pseudo/test/cxx/declarator-var.cpp
+++ clang-tools-extra/pseudo/test/cxx/declarator-var.cpp
@@ -1,11 +1,9 @@
 // The standard grammar allows an function-body to use any declarator, including
 // a non-function declarator. This creates an ambiguity where a
 // simple-declaration is misparsed as a function-definition.
-// FIXME: eliminate this false parse.
-// XFAIL: *
 
 // RUN: clang-pseudo -grammar=cxx -source=%s --print-forest | FileCheck %s
 void (*s)(){};
 // CHECK-NOT:  function-definition
-// CHECK:  init-declarator := declarator initializer
+// CHECK:  init-declarator := non-function-declarator initializer
 // CHECK-NOT:  function-definition
Index: clang-tools-extra/pseudo/test/cxx/declarator-function.cpp
===
--- clang-tools-extra/pseudo/test/cxx/declarator-function.cpp
+++ clang-tools-extra/pseudo/test/cxx/declarator-function.cpp
@@ -1,11 +1,28 @@
 // The standard grammar allows an init-list with any declarator, including
 // a function declarator. This creates an ambiguity where a function-definition
 // is misparsed as a simple-declaration.
-// FIXME: eliminate this false parse.
-// XFAIL: *
 
 // RUN: clang-pseudo -grammar=cxx -source=%s --print-forest | FileCheck %s
-void s(){};
-// CHECK-NOT:  simple-declaration
-// CHECK:  function-definition := decl-specifier-seq declarator
-// function-body CHECK-NOT:  simple-declaration
+void s() {};
+// CHECK:  ├─declaration-seq~function-definition := decl-specifier-seq function-declarator function-body
+// CHECK-NEXT: │ ├─decl-specifier-seq~VOID := tok[0]
+// CHECK-NEXT: │ ├─function-declarator~noptr-declarator := noptr-declarator parameters-and-qualifiers
+// CHECK-NEXT: │ │ ├─noptr-declarator~IDENTIFIER := tok[1]
+// CHECK-NEXT: │ │ └─parameters-and-qualifiers := ( )
+// CHECK-NEXT: │ │   ├─( := tok[2]
+// CHECK-NEXT: │ │   └─) := tok[3]
+// CHECK-NEXT: │ └─function-body~compound-statement := { }
+// CHECK-NEXT: │   ├─{ := tok[4]
+// CHECK-NEXT: │   └─} := tok[5]
+// CHECK-NEXT: └─declaration~; := tok[6]
+
+// Should not parse as a function-definition.
+int s2 {};
+// CHECK-NEXT:  declaration~simple-declaration := decl-specifier-seq init-declarator-list ;
+// CHECK-NEXT:  ├─decl-specifier-seq~INT := tok[7]
+// CHECK-NEXT:  ├─init-declarator-list~init-declarator := non-function-declarator initializer
+// CHECK-NEXT:  │ ├─non-function-declarator~IDENTIFIER := tok[8]
+// CHECK-NEXT:  │ └─initializer~braced-init-list := { }
+// CHECK-NEXT:  │   ├─{ := tok[9]
+// CHECK-NEXT:  │   └─} := tok[10]
+// CHECK-NEXT:  └─; := tok[11]
Index: clang-tools-extra/pseudo/lib/cxx/cxx.bnf
===
--- clang-tools-extra/pseudo/lib/cxx/cxx.bnf
+++ clang-tools-extra/pseudo/lib/cxx/cxx.bnf
@@ -402,8 +402,10 @@
 placeholder-type-specifier := type-constraint_opt DECL

[PATCH] D128745: [c++] implements DR692, DR1395 and tentatively DR1432, about partial ordering of variadic template partial specialization or function template

2022-07-15 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision.
aaron.ballman added a comment.

LGTM!




Comment at: clang/lib/Sema/SemaTemplateDeduction.cpp:5444
+  if (Better1 && Better2) {
+bool ClangABICompat14 = S.Context.getLangOpts().getClangABICompat() <=
+LangOptions::ClangABI::Ver14;

ychen wrote:
> aaron.ballman wrote:
> > Can you add CodeGen test coverage for both ABI modes (new RUN line with 
> > `-fclang-abi-compat=14`)
> Some test cases could reach the CodeGen phase *with* 
> `-fclang-abi-compat=14`(i.e without this patch), some test cases could reach 
> the CodeGen phase *without* `-fclang-abi-compat=14`(i.e with this patch). 
> Please let me know if the added CodeGen tests are what you expected.
Yes, that test is what I was looking for, thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128745

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


[PATCH] D126189: [C++20][Modules] Build module static initializers per P1874R1.

2022-07-15 Thread Iain Sandoe via Phabricator via cfe-commits
iains reopened this revision.
iains added a comment.
This revision is now accepted and ready to land.

reopening to post the patch I intend to land.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126189

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


[PATCH] D126189: [C++20][Modules] Build module static initializers per P1874R1.

2022-07-15 Thread Iain Sandoe via Phabricator via cfe-commits
iains updated this revision to Diff 444989.
iains added a comment.

rebased, fixed the interaction with clang module map modules.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126189

Files:
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/Basic/Module.h
  clang/include/clang/Sema/Sema.h
  clang/lib/CodeGen/CGDeclCXX.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/Parse/ParseAST.cpp
  clang/lib/Sema/SemaModule.cpp
  clang/test/CodeGen/module-intializer-pmf.cpp
  clang/test/CodeGen/module-intializer.cpp

Index: clang/test/CodeGen/module-intializer.cpp
===
--- /dev/null
+++ clang/test/CodeGen/module-intializer.cpp
@@ -0,0 +1,186 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: cd %t
+
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 N.cpp \
+// RUN:-emit-module-interface -o N.pcm
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 N.pcm -S -emit-llvm \
+// RUN:  -o - | FileCheck %s --check-prefix=CHECK-N
+
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 O.cpp \
+// RUN:-emit-module-interface -o O.pcm
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 O.pcm -S -emit-llvm \
+// RUN:  -o - | FileCheck %s --check-prefix=CHECK-O
+
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 M-part.cpp \
+// RUN:-emit-module-interface -o M-part.pcm
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 M-part.pcm -S \
+// RUN: -emit-llvm -o - | FileCheck %s --check-prefix=CHECK-P
+
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 M.cpp \
+// RUN: -fmodule-file=N.pcm -fmodule-file=O.pcm -fmodule-file=M-part.pcm \
+// RUN:-emit-module-interface -o M.pcm
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 M.pcm -S -emit-llvm \
+// RUN:  -o - | FileCheck %s --check-prefix=CHECK-M
+
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 useM.cpp \
+// RUN: -fmodule-file=M.pcm -S -emit-llvm  -o - \
+// RUN: | FileCheck %s --check-prefix=CHECK-USE
+
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 M-impl.cpp \
+// RUN: -fmodule-file=M.pcm -S -emit-llvm  -o - \
+// RUN: | FileCheck %s --check-prefix=CHECK-IMPL
+
+//--- N-h.h
+
+struct Oink {
+  Oink(){};
+};
+
+Oink Hog;
+
+//--- N.cpp
+
+module;
+#include "N-h.h"
+
+export module N;
+
+export struct Quack {
+  Quack(){};
+};
+
+export Quack Duck;
+
+// CHECK-N: define internal void @__cxx_global_var_init
+// CHECK-N: call {{.*}} @_ZN4OinkC1Ev
+// CHECK-N: define internal void @__cxx_global_var_init
+// CHECK-N: call {{.*}} @_ZNW1N5QuackC1Ev
+// CHECK-N: define void @_ZGIW1N
+// CHECK-N: store i8 1, ptr @_ZGIW1N__in_chrg
+// CHECK-N: call void @__cxx_global_var_init
+// CHECK-N: call void @__cxx_global_var_init
+
+//--- O-h.h
+
+struct Meow {
+  Meow(){};
+};
+
+Meow Cat;
+
+//--- O.cpp
+
+module;
+#include "O-h.h"
+
+export module O;
+
+export struct Bark {
+  Bark(){};
+};
+
+export Bark Dog;
+
+// CHECK-O: define internal void @__cxx_global_var_init
+// CHECK-O: call {{.*}} @_ZN4MeowC2Ev
+// CHECK-O: define internal void @__cxx_global_var_init
+// CHECK-O: call {{.*}} @_ZNW1O4BarkC1Ev
+// CHECK-O: define void @_ZGIW1O
+// CHECK-O: store i8 1, ptr @_ZGIW1O__in_chrg
+// CHECK-O: call void @__cxx_global_var_init
+// CHECK-O: call void @__cxx_global_var_init
+
+//--- P-h.h
+
+struct Croak {
+  Croak(){};
+};
+
+Croak Frog;
+
+//--- M-part.cpp
+
+module;
+#include "P-h.h"
+
+module M:Part;
+
+struct Squawk {
+  Squawk(){};
+};
+
+Squawk parrot;
+
+// CHECK-P: define internal void @__cxx_global_var_init
+// CHECK-P: call {{.*}} @_ZN5CroakC1Ev
+// CHECK-P: define internal void @__cxx_global_var_init
+// CHECK-P: call {{.*}} @_ZNW1M6SquawkC1Ev
+// CHECK-P: define void @_ZGIW1MWP4Part
+// CHECK-P: store i8 1, ptr @_ZGIW1MWP4Part__in_chrg
+// CHECK-P: call void @__cxx_global_var_init
+// CHECK-P: call void @__cxx_global_var_init
+
+//--- M-h.h
+
+struct Moo {
+  Moo(){};
+};
+
+Moo Cow;
+
+//--- M.cpp
+
+module;
+#include "M-h.h"
+
+export module M;
+import N;
+export import O;
+import :Part;
+
+export struct Baa {
+  int x;
+  Baa(){};
+  Baa(int x) : x(x) {}
+  int getX() { return x; }
+};
+
+export Baa Sheep(10);
+
+// CHECK-M: define internal void @__cxx_global_var_init
+// CHECK-M: call {{.*}} @_ZN3MooC1Ev
+// CHECK-M: define internal void @__cxx_global_var_init
+// CHECK-M: call {{.*}} @_ZNW1M3BaaC1Ei
+// CHECK-M: declare void @_ZGIW1O()
+// CHECK-M: declare void @_ZGIW1N()
+// CHECK-M: declare void @_ZGIW1MWP4Part()
+// CHECK-M: define void @_ZGIW1M
+// CHECK-M: store i8 1, ptr @_ZGIW1M__in_chrg
+// CHECK-M: call void @_ZGIW1O()
+// CHECK-M: call void @_ZGIW1N()
+// CHECK-M: call void @_ZGIW1MWP4Part()
+// CHECK-M: call void @__cxx_global_var_init
+// CHECK-M: call void @__cxx_global_var_init
+
+//--- useM.cpp
+
+import M;
+
+int main() {
+  return Sheep.getX();
+}
+
+// CHECK-U

[PATCH] D121141: [Clang] Add `-funstable` flag to enable unstable and experimental features: follow-up fixes

2022-07-15 Thread Mark de Wever via Phabricator via cfe-commits
Mordante accepted this revision.
Mordante added a comment.

LGTM, but please update the name of the flag in the title before landing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121141

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


[PATCH] D129864: [Flang] Generate documentation for compiler flags

2022-07-15 Thread Dylan Fleming via Phabricator via cfe-commits
DylanFleming-arm created this revision.
Herald added subscribers: arphaman, mgorny.
Herald added a reviewer: sscalpone.
Herald added projects: Flang, All.
DylanFleming-arm requested review of this revision.
Herald added subscribers: cfe-commits, jdoerfert.
Herald added a project: clang.

This patch aims to create a webpage to document
Flang's command line options on https://flang.llvm.org/docs/
in a similar way to Clang's
https://clang.llvm.org/docs/ClangCommandLineReference.html

This is done by using clang tablegen to generate an .rst
file from options.td (which is current shared with clang)
For this to work, ClangOptionDocEmitter.cpp was updated
to allow specific flang flags to be included,
rather than bulk excluding clang flags.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D129864

Files:
  clang/utils/TableGen/ClangOptionDocEmitter.cpp
  flang/docs/CMakeLists.txt
  flang/docs/index.md
  flang/include/flang/FlangOptionsDocs.td

Index: flang/include/flang/FlangOptionsDocs.td
===
--- /dev/null
+++ flang/include/flang/FlangOptionsDocs.td
@@ -0,0 +1,35 @@
+//==--- FlangOptionDocs.td - Option documentation -===//
+//
+// 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
+//
+//===--===//
+
+def GlobalDocumentation {
+  code Intro =[{..
+  ---
+  NOTE: This file is automatically generated by running clang-tblgen
+  -gen-opt-docs. Do not edit this file by hand!!
+  ---
+
+=
+Flang command line argument reference
+=
+.. contents::
+   :local:
+
+Introduction
+
+
+}];
+
+  string Program = "flang";
+
+  list ExcludedFlags = [""];
+  list IncludedFlags = ["FlangOption"];
+
+}
+
+
+include "../../../clang/include/clang/Driver/Options.td"
Index: flang/docs/index.md
===
--- flang/docs/index.md
+++ flang/docs/index.md
@@ -45,6 +45,7 @@
DoConcurrent
Extensions
FIRLangRef
+   FlangCommandLineReference
FlangDriver
FortranIR
FortranLLVMTestSuite
Index: flang/docs/CMakeLists.txt
===
--- flang/docs/CMakeLists.txt
+++ flang/docs/CMakeLists.txt
@@ -91,6 +91,16 @@
 endif()
 endif()
 
+function (gen_rst_file_from_td output_file td_option source docs_target)
+  if (NOT EXISTS "${CMAKE_CURRENT_SOURCE_DIR}/${source}")
+message(FATAL_ERROR "Cannot find source file: ${source} in ${CMAKE_CURRENT_SOURCE_DIR}")
+  endif()
+  get_filename_component(TABLEGEN_INCLUDE_DIR "${CMAKE_CURRENT_SOURCE_DIR}/${source}" DIRECTORY)
+  list(APPEND LLVM_TABLEGEN_FLAGS "-I${TABLEGEN_INCLUDE_DIR}")
+  clang_tablegen(Source/${output_file} ${td_option} SOURCE ${source} TARGET "gen-${output_file}")
+  add_dependencies(${docs_target} "gen-${output_file}")
+endfunction()
+
 if (LLVM_ENABLE_SPHINX)
   include(AddSphinxTarget)
   if (SPHINX_FOUND)
@@ -114,6 +124,7 @@
   COMMAND "${Python3_EXECUTABLE}"
   ARGS ${CMAKE_CURRENT_BINARY_DIR}/Source/FIR/CreateFIRLangRef.py)
 
+  gen_rst_file_from_td(FlangCommandLineReference.rst -gen-opt-docs ../include/flang/FlangOptionsDocs.td docs-flang-html)
 endif()
 if (${SPHINX_OUTPUT_MAN})
   add_sphinx_target(man flang)
Index: clang/utils/TableGen/ClangOptionDocEmitter.cpp
===
--- clang/utils/TableGen/ClangOptionDocEmitter.cpp
+++ clang/utils/TableGen/ClangOptionDocEmitter.cpp
@@ -168,6 +168,30 @@
   return false;
 }
 
+bool isIncluded(const Record *OptionOrGroup, const Record *DocInfo) {
+  if (!DocInfo->getValue("IncludedFlags"))
+return true;
+  for (StringRef Inclusion : DocInfo->getValueAsListOfStrings("IncludedFlags"))
+if (hasFlag(OptionOrGroup, Inclusion))
+  return true;
+  return false;
+}
+
+bool isGroupIncluded(const DocumentedGroup &Group, const Record *DocInfo) {
+  if (isIncluded(Group.Group, DocInfo))
+return true;
+  for (auto &O : Group.Options)
+if (isIncluded(O.Option, DocInfo))
+  return true;
+  for (auto &G : Group.Groups) {
+if (isIncluded(G.Group, DocInfo))
+  return true;
+if (isGroupIncluded(G, DocInfo))
+  return true;
+  }
+  return false;
+}
+
 bool isExcluded(const Record *OptionOrGroup, const Record *DocInfo) {
   // FIXME: Provide a flag to specify the set of exclusions.
   for (StringRef Exclusion : DocInfo->getValueAsListOfStrings("ExcludedFlags"))
@@ -302,7 +326,7 @@
 
 void emitOption(const DocumentedOption &Option, const Record *DocInfo,
 raw_ostream &OS

[PATCH] D121141: [Clang] Add `-funstable` flag to enable unstable and experimental features: follow-up fixes

2022-07-15 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added inline comments.



Comment at: clang/include/clang/Driver/Options.td:1186
 
-defm unstable : BoolFOption<"unstable",
-  LangOpts<"Unstable">, DefaultFalse,
-  PosFlag,
+defm experimental_library : BoolFOption<"experimental-library",
+  LangOpts<"ExperimentalLibrary">, DefaultFalse,

MaskRay wrote:
> This can be simplified with `OptInCC1FFlag` (both driver/CC1 for the pos 
> form, but driver-only for the neg form).
> You'll need to set CoreOption to make the option available to clang-cl.
I was looking for documentation on `OptInCC1FFlag`, and I found:

```
// A boolean option which is opt-in in CC1. The positive option exists in CC1 
and
// Args.hasArg(OPT_ffoo) can be used to check that the flag is enabled.
// This is useful if the option is usually disabled.
// Use this only when the option cannot be declared via BoolFOption.
multiclass OptInCC1FFlag There may be an archive ordering problem. 
I'm not sure I follow -- what problem are you concerned about?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121141

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


[PATCH] D129648: Use pseudo parser for folding ranges

2022-07-15 Thread Utkarsh Saxena via Phabricator via cfe-commits
usaxena95 updated this revision to Diff 445002.
usaxena95 added a comment.

Removed unused headers and fix spellings.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129648

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/SemanticSelection.cpp
  clang-tools-extra/clangd/SemanticSelection.h
  clang-tools-extra/clangd/unittests/SemanticSelectionTests.cpp
  clang-tools-extra/pseudo/include/clang-pseudo/Token.h
  clang-tools-extra/pseudo/lib/CMakeLists.txt
  clang-tools-extra/pseudo/lib/Lex.cpp
  clang-tools-extra/pseudo/unittests/TokenTest.cpp

Index: clang-tools-extra/pseudo/unittests/TokenTest.cpp
===
--- clang-tools-extra/pseudo/unittests/TokenTest.cpp
+++ clang-tools-extra/pseudo/unittests/TokenTest.cpp
@@ -31,6 +31,10 @@
   return arg.Line == (unsigned)Line && arg.Indent == (unsigned)Indent;
 }
 
+MATCHER_P(originalIndex, index, "") {
+  return arg.OriginalIndex == (Token::Index)index;
+}
+
 TEST(TokenTest, Lex) {
   LangOptions Opts;
   std::string Code = R"cpp(
@@ -105,20 +109,23 @@
   Raw.tokens(),
   ElementsAre(AllOf(token("one_\\\ntoken", tok::raw_identifier),
 hasFlag(LexFlags::StartsPPLine),
-hasFlag(LexFlags::NeedsCleaning), lineIndent(1, 0)),
+hasFlag(LexFlags::NeedsCleaning), lineIndent(1, 0),
+originalIndex(0)),
   AllOf(token("two", tok::raw_identifier),
 hasFlag(LexFlags::StartsPPLine),
-Not(hasFlag(LexFlags::NeedsCleaning))),
+Not(hasFlag(LexFlags::NeedsCleaning)),
+originalIndex(1)),
   AllOf(token("\\\ntokens", tok::raw_identifier),
 Not(hasFlag(LexFlags::StartsPPLine)),
-hasFlag(LexFlags::NeedsCleaning;
+hasFlag(LexFlags::NeedsCleaning), originalIndex(2;
 
   TokenStream Cooked = cook(Raw, Opts);
   EXPECT_THAT(
   Cooked.tokens(),
-  ElementsAre(AllOf(token("one_token", tok::identifier), lineIndent(1, 0)),
-  token("two", tok::identifier),
-  token("tokens", tok::identifier)));
+  ElementsAre(AllOf(token("one_token", tok::identifier), lineIndent(1, 0),
+originalIndex(0)),
+  AllOf(token("two", tok::identifier), originalIndex(1)),
+  AllOf(token("tokens", tok::identifier), originalIndex(2;
 }
 
 TEST(TokenTest, EncodedCharacters) {
@@ -182,13 +189,14 @@
 )cpp";
   TokenStream Cook = cook(lex(Code, Opts), Opts);
   TokenStream Split = stripComments(Cook);
-  EXPECT_THAT(Split.tokens(), ElementsAreArray({
-  token(">", tok::greater),
-  token(">", tok::greater),
-  token(">", tok::greater),
-  token(">", tok::greater),
-  token(">>=", tok::greatergreaterequal),
-  }));
+  EXPECT_THAT(Split.tokens(),
+  ElementsAre(AllOf(token(">", tok::greater), originalIndex(0)),
+  AllOf(token(">", tok::greater), originalIndex(0)),
+  // Token 1 and 2 are comments.
+  AllOf(token(">", tok::greater), originalIndex(3)),
+  AllOf(token(">", tok::greater), originalIndex(3)),
+  AllOf(token(">>=", tok::greatergreaterequal),
+originalIndex(4;
 }
 
 TEST(TokenTest, DropComments) {
@@ -199,13 +207,16 @@
 )cpp";
   TokenStream Raw = cook(lex(Code, Opts), Opts);
   TokenStream Stripped = stripComments(Raw);
-  EXPECT_THAT(Raw.tokens(),
-  ElementsAreArray(
-  {token("// comment", tok::comment), token("int", tok::kw_int),
-   token("/*abc*/", tok::comment), token(";", tok::semi)}));
-
-  EXPECT_THAT(Stripped.tokens(), ElementsAreArray({token("int", tok::kw_int),
-   token(";", tok::semi)}));
+  EXPECT_THAT(
+  Raw.tokens(),
+  ElementsAre(AllOf(token("// comment", tok::comment), originalIndex(0)),
+  AllOf(token("int", tok::kw_int), originalIndex(1)),
+  AllOf(token("/*abc*/", tok::comment), originalIndex(2)),
+  AllOf(token(";", tok::semi), originalIndex(3;
+
+  EXPECT_THAT(Stripped.tokens(),
+  ElementsAre(AllOf(token("int", tok::kw_int), originalIndex(1)),
+  AllOf(token(";", tok::semi), originalIndex(3;
 }
 
 } // namespace
Index: clang-tools-extra/pseudo/lib/Lex.cpp
===
--- clang-tools-extra/pseudo/lib/Lex.cpp
+++ clang-tools-

[PATCH] D121141: [Clang] Add `-fexperimental-library` flag to enable unstable and experimental features: follow-up fixes

2022-07-15 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added a subscriber: mstorsjo.
ldionne added a comment.

The `experimental-library-flag` test is currently failing on Windows because on 
Windows, `-stdlib=libc++` seems to be ignored and we don't add `-lc++` or 
`-lc++experimental`. Does someone understand how things are supposed to work 
when using libc++ on Windows? @mstorsjo maybe?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121141

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


[clang] f5d9de8 - [Clang] Add a new clang option "-ftime-trace="

2022-07-15 Thread Junduo Dong via cfe-commits

Author: dongjunduo
Date: 2022-07-15T08:55:17-07:00
New Revision: f5d9de8cc33014923fbd1c5682758557887e85ba

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

LOG: [Clang] Add a new clang option "-ftime-trace="

The time profiler traces the stages during the clang compile
process. Each compiling stage of a single source file
corresponds to a separately .json file which holds its
time tracing data. However, the .json files are stored in the
same path/directory as its corresponding stage's '-o' option.
For example, if we compile the "demo.cc" to "demo.o" with option
"-o /tmp/demo.o", the time trace data file path is "/tmp/demo.json".

A typical c++ project can contain multiple source files in different
path, but all the json files' paths can be a mess.

The option "-ftime-trace=" allows you to specify where the json
files should be stored. This allows the users to place the time trace
data files of interest in the desired location for further data analysis.

Usage:
- clang/clang++ -ftime-trace ...
- clang/clang++ -ftime-trace=the-directory-you-want ...
- clang/clang++ -ftime-trace=the-directory-you-want/ ...
- clang/clang++ -ftime-trace=the-full-file-path-you-want ...

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

Added: 


Modified: 
clang/include/clang/Driver/Options.td
clang/include/clang/Frontend/FrontendOptions.h
clang/lib/Driver/ToolChains/Clang.cpp
clang/test/Driver/check-time-trace.cpp
clang/tools/driver/cc1_main.cpp

Removed: 




diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 404effb4e1de4..6d0736c02bdda 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -2857,6 +2857,15 @@ def ftime_trace_granularity_EQ : Joined<["-"], 
"ftime-trace-granularity=">, Grou
   HelpText<"Minimum time granularity (in microseconds) traced by time 
profiler">,
   Flags<[CC1Option, CoreOption]>,
   MarshallingInfoInt, "500u">;
+def ftime_trace_EQ : Joined<["-"], "ftime-trace=">, Group,
+  HelpText<"Turn on time profiler. Generates JSON file based on output 
filename. "
+   "Specify the path which stores the tracing output file.">,
+  DocBrief<[{
+  Turn on time profiler. Generates JSON file based on output filename. Results
+  can be analyzed with chrome://tracing or `Speedscope App
+  `_ for flamegraph visualization.}]>,
+  Flags<[CC1Option, CoreOption]>,
+  MarshallingInfoString>;
 def fproc_stat_report : Joined<["-"], "fproc-stat-report">, Group,
   HelpText<"Print subprocess statistics">;
 def fproc_stat_report_EQ : Joined<["-"], "fproc-stat-report=">, Group,

diff  --git a/clang/include/clang/Frontend/FrontendOptions.h 
b/clang/include/clang/Frontend/FrontendOptions.h
index ff5a9c5c77f4d..b0e719ffcacf5 100644
--- a/clang/include/clang/Frontend/FrontendOptions.h
+++ b/clang/include/clang/Frontend/FrontendOptions.h
@@ -499,6 +499,9 @@ class FrontendOptions {
   /// Minimum time granularity (in microseconds) traced by time profiler.
   unsigned TimeTraceGranularity;
 
+  /// Path which stores the output files for -ftime-trace
+  std::string TimeTracePath;
+
 public:
   FrontendOptions()
   : DisableFree(false), RelocatablePCH(false), ShowHelp(false),

diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index 3e36d4def2f33..72a1250f89568 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -6230,6 +6230,7 @@ void Clang::ConstructJob(Compilation &C, const JobAction 
&JA,
   Args.AddLastArg(CmdArgs, options::OPT_ftime_report_EQ);
   Args.AddLastArg(CmdArgs, options::OPT_ftime_trace);
   Args.AddLastArg(CmdArgs, options::OPT_ftime_trace_granularity_EQ);
+  Args.AddLastArg(CmdArgs, options::OPT_ftime_trace_EQ);
   Args.AddLastArg(CmdArgs, options::OPT_ftrapv);
   Args.AddLastArg(CmdArgs, options::OPT_malign_double);
   Args.AddLastArg(CmdArgs, options::OPT_fno_temp_file);

diff  --git a/clang/test/Driver/check-time-trace.cpp 
b/clang/test/Driver/check-time-trace.cpp
index 1d80748a52331..52b3e71394fba 100644
--- a/clang/test/Driver/check-time-trace.cpp
+++ b/clang/test/Driver/check-time-trace.cpp
@@ -2,6 +2,20 @@
 // RUN: cat %T/check-time-trace.json \
 // RUN:   | %python -c 'import json, sys; 
json.dump(json.loads(sys.stdin.read()), sys.stdout, sort_keys=True, indent=2)' \
 // RUN:   | FileCheck %s
+// RUN: %clangxx -S -ftime-trace=%T/new-name.json -ftime-trace-granularity=0 
-o %T/check-time-trace %s
+// RUN: cat %T/new-name.json \
+// RUN:   | %python -c 'import json, sys; 
json.dump(json.loads(sys.stdin.read()), sys.stdout, sort_keys=True, indent=2)' \
+// RUN:   | FileCheck %s
+// RUN: rm -rf %T/output1 && mkdir %T/output1
+// RUN: 

[PATCH] D128048: Add a new clang option "-ftime-trace="

2022-07-15 Thread dongjunduo via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGf5d9de8cc330: [Clang] Add a new clang option 
"-ftime-trace=" (authored by dongjunduo).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128048

Files:
  clang/include/clang/Driver/Options.td
  clang/include/clang/Frontend/FrontendOptions.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/check-time-trace.cpp
  clang/tools/driver/cc1_main.cpp


Index: clang/tools/driver/cc1_main.cpp
===
--- clang/tools/driver/cc1_main.cpp
+++ clang/tools/driver/cc1_main.cpp
@@ -212,7 +212,9 @@
   bool Success = CompilerInvocation::CreateFromArgs(Clang->getInvocation(),
 Argv, Diags, Argv0);
 
-  if (Clang->getFrontendOpts().TimeTrace) {
+  if (Clang->getFrontendOpts().TimeTrace ||
+  !Clang->getFrontendOpts().TimeTracePath.empty()) {
+Clang->getFrontendOpts().TimeTrace = 1;
 llvm::timeTraceProfilerInitialize(
 Clang->getFrontendOpts().TimeTraceGranularity, Argv0);
   }
@@ -256,6 +258,13 @@
   if (llvm::timeTraceProfilerEnabled()) {
 SmallString<128> Path(Clang->getFrontendOpts().OutputFile);
 llvm::sys::path::replace_extension(Path, "json");
+if (!Clang->getFrontendOpts().TimeTracePath.empty()) {
+  // replace the suffix to '.json' directly
+  SmallString<128> TracePath(Clang->getFrontendOpts().TimeTracePath);
+  if (llvm::sys::fs::is_directory(TracePath))
+llvm::sys::path::append(TracePath, llvm::sys::path::filename(Path));
+  Path.assign(TracePath);
+}
 if (auto profilerOutput = Clang->createOutputFile(
 Path.str(), /*Binary=*/false, /*RemoveFileOnSignal=*/false,
 /*useTemporary=*/false)) {
Index: clang/test/Driver/check-time-trace.cpp
===
--- clang/test/Driver/check-time-trace.cpp
+++ clang/test/Driver/check-time-trace.cpp
@@ -2,6 +2,20 @@
 // RUN: cat %T/check-time-trace.json \
 // RUN:   | %python -c 'import json, sys; 
json.dump(json.loads(sys.stdin.read()), sys.stdout, sort_keys=True, indent=2)' \
 // RUN:   | FileCheck %s
+// RUN: %clangxx -S -ftime-trace=%T/new-name.json -ftime-trace-granularity=0 
-o %T/check-time-trace %s
+// RUN: cat %T/new-name.json \
+// RUN:   | %python -c 'import json, sys; 
json.dump(json.loads(sys.stdin.read()), sys.stdout, sort_keys=True, indent=2)' \
+// RUN:   | FileCheck %s
+// RUN: rm -rf %T/output1 && mkdir %T/output1
+// RUN: %clangxx -S -ftime-trace=%T/output1 -ftime-trace-granularity=0 -o 
%T/check-time-trace %s
+// RUN: cat %T/output1/check-time-trace.json \
+// RUN:   | %python -c 'import json, sys; 
json.dump(json.loads(sys.stdin.read()), sys.stdout, sort_keys=True, indent=2)' \
+// RUN:   | FileCheck %s
+// RUN: rm -rf %T/output2 && mkdir %T/output2
+// RUN: %clangxx -S -ftime-trace=%T/output2/ -ftime-trace-granularity=0 -o 
%T/check-time-trace %s
+// RUN: cat %T/output2/check-time-trace.json \
+// RUN:   | %python -c 'import json, sys; 
json.dump(json.loads(sys.stdin.read()), sys.stdout, sort_keys=True, indent=2)' \
+// RUN:   | FileCheck %s
 
 // CHECK:  "beginningOfTime": {{[0-9]{16},}}
 // CHECK-NEXT: "traceEvents": [
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -6230,6 +6230,7 @@
   Args.AddLastArg(CmdArgs, options::OPT_ftime_report_EQ);
   Args.AddLastArg(CmdArgs, options::OPT_ftime_trace);
   Args.AddLastArg(CmdArgs, options::OPT_ftime_trace_granularity_EQ);
+  Args.AddLastArg(CmdArgs, options::OPT_ftime_trace_EQ);
   Args.AddLastArg(CmdArgs, options::OPT_ftrapv);
   Args.AddLastArg(CmdArgs, options::OPT_malign_double);
   Args.AddLastArg(CmdArgs, options::OPT_fno_temp_file);
Index: clang/include/clang/Frontend/FrontendOptions.h
===
--- clang/include/clang/Frontend/FrontendOptions.h
+++ clang/include/clang/Frontend/FrontendOptions.h
@@ -499,6 +499,9 @@
   /// Minimum time granularity (in microseconds) traced by time profiler.
   unsigned TimeTraceGranularity;
 
+  /// Path which stores the output files for -ftime-trace
+  std::string TimeTracePath;
+
 public:
   FrontendOptions()
   : DisableFree(false), RelocatablePCH(false), ShowHelp(false),
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -2857,6 +2857,15 @@
   HelpText<"Minimum time granularity (in microseconds) traced by time 
profiler">,
   Flags<[CC1Option, CoreOption]>,
   MarshallingInfoInt, "500u">;
+def ftime_trace_EQ : Joined<["-"], "ftime-trace=">, Group,
+  HelpText<"Turn on time 

[PATCH] D129832: [sanitizer] Add "mainsrc" prefix to sanitizer special case list

2022-07-15 Thread Kirill Stoimenov via Phabricator via cfe-commits
kstoimenov accepted this revision.
kstoimenov added a comment.

I think the name 'mainsrc' is slightly misleading because of the association 
with the 'main' function. Maybe something like primarysrc would be better to 
avoid this confusion?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129832

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


[PATCH] D129872: [clang][OpenMP] Fix runtime crash in the call to __kmp_alloc.

2022-07-15 Thread Jennifer Yu via Phabricator via cfe-commits
jyu2 created this revision.
jyu2 added reviewers: ddpagan, ABataev, mikerice.
jyu2 added projects: clang, OpenMP.
Herald added subscribers: guansong, yaxunl.
Herald added a project: All.
jyu2 requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added subscribers: openmp-commits, cfe-commits, sstefan1.

The problem is that "alloctor" from omp_init_allocator gets truncated before 
passing to __kmp_alloc.

To fix this, when generate AllocrVal, call to IgnoreImpCasts to skip 'trunc'


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D129872

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/test/OpenMP/omp_init_allocator.c
  openmp/runtime/test/api/omp_init_allocator.c

Index: openmp/runtime/test/api/omp_init_allocator.c
===
--- /dev/null
+++ openmp/runtime/test/api/omp_init_allocator.c
@@ -0,0 +1,20 @@
+// RUN: %libomp-compile-and-run
+
+#include 
+#include 
+
+void test_allocate_allocator() {
+  omp_alloctrait_t x_traits[1] = {{omp_atk_alignment, 64}};
+  omp_allocator_handle_t x_alloc =
+  omp_init_allocator(omp_default_mem_space, 1, x_traits);
+  {
+int x;
+#pragma omp allocate(x) allocator(x_alloc)
+  }
+  omp_destroy_allocator(x_alloc);
+}
+
+int main() {
+  test_allocate_allocator();
+  printf("passed\n");
+}
Index: clang/test/OpenMP/omp_init_allocator.c
===
--- /dev/null
+++ clang/test/OpenMP/omp_init_allocator.c
@@ -0,0 +1,61 @@
+// RUN: %clang_cc1 -isystem %S/Inputs -emit-llvm -o - -fopenmp \
+// RUN: -fopenmp-version=50 -triple x86_64-unknown-linux-gnu %s | FileCheck %s
+
+typedef enum {
+  omp_atk_sync_hint = 1,
+  omp_atk_alignment = 2
+} omp_alloctrait_key_t;
+typedef unsigned long int uintptr_t;
+typedef uintptr_t omp_uintptr_t;
+typedef struct {
+  omp_alloctrait_key_t key;
+  omp_uintptr_t value;
+} omp_alloctrait_t;
+typedef enum omp_allocator_handle_t {
+  omp_null_allocator = 0,
+  omp_default_mem_alloc = 1,
+  omp_large_cap_mem_alloc = 2,
+  omp_const_mem_alloc = 3,
+  omp_high_bw_mem_alloc = 4,
+  omp_low_lat_mem_alloc = 5,
+  omp_cgroup_mem_alloc = 6,
+  omp_pteam_mem_alloc = 7,
+  omp_thread_mem_alloc = 8,
+
+  omp_target_host_mem_alloc = 100,
+  omp_target_shared_mem_alloc = 101,
+  omp_target_device_mem_alloc = 102,
+  KMP_ALLOCATOR_MAX_HANDLE = (18446744073709551615UL)
+} omp_allocator_handle_t;
+typedef enum omp_memspace_handle_t {
+  omp_default_mem_space = 0,
+  omp_large_cap_mem_space = 1,
+  omp_const_mem_space = 2,
+  omp_high_bw_mem_space = 3,
+  omp_low_lat_mem_space = 4,
+
+  omp_target_host_mem_space = 100,
+  omp_target_shared_mem_space = 101,
+  omp_target_device_mem_space = 102,
+  KMP_MEMSPACE_MAX_HANDLE = (18446744073709551615UL)
+} omp_memspace_handle_t;
+
+extern omp_allocator_handle_t omp_init_allocator(omp_memspace_handle_t m,
+ int ntraits,
+ omp_alloctrait_t traits[]);
+//CHECK-LABEL: test_allocate_allocator
+void test_allocate_allocator() {
+  omp_alloctrait_t x_traits[1] = {{omp_atk_alignment, 64}};
+  omp_allocator_handle_t x_alloc =
+  omp_init_allocator(omp_default_mem_space, 1, x_traits);
+  {
+int x;
+// CHECK: [[RET:%.+]] = call i64 @omp_init_allocator
+// CHECK-NEXT: store i64 [[RET]], ptr [[X_ALLOC:%x_alloc]]
+// CHECK: [[TMP:%.+]] = load i64, ptr [[X_ALLOC]]
+// CHECK-NEXT: [[CONV:%conv]] = inttoptr i64 [[TMP]] to ptr
+// CHECK-NEXT: call ptr @__kmpc_alloc(i32 %0, i64 4, ptr [[CONV]])
+// CHECK-NOT: trunc
+#pragma omp allocate(x) allocator(x_alloc)
+  }
+}
Index: clang/lib/CodeGen/CGOpenMPRuntime.cpp
===
--- clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -12114,7 +12114,7 @@
 const Expr *Allocator) {
   llvm::Value *AllocVal;
   if (Allocator) {
-AllocVal = CGF.EmitScalarExpr(Allocator);
+AllocVal = CGF.EmitScalarExpr(Allocator->IgnoreImpCasts());
 // According to the standard, the original allocator type is a enum
 // (integer). Convert to pointer type, if required.
 AllocVal = CGF.EmitScalarConversion(AllocVal, Allocator->getType(),
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D129873: [clang-offload-bundler] Library-ize ClangOffloadBundler

2022-07-15 Thread Jacob Lambert via Phabricator via cfe-commits
lamb-j created this revision.
Herald added a subscriber: mgorny.
Herald added a reviewer: alexander-shaposhnikov.
Herald added a project: All.
lamb-j requested review of this revision.
Herald added subscribers: cfe-commits, sstefan1, MaskRay.
Herald added a reviewer: jdoerfert.
Herald added a project: clang.

Lifting the core functionalities of the clang-offload-bundler into a
user-facing library/API. This will allow online and JIT compilers to
bundle and unbundle files without spawning a new process.

This patch lifts the classes and functions used to implement 
the clang-offload-bundler into a separate OffloadBundler.cpp,
and defines three top-level API functions in OfflaodBundler.h.

  BundleFiles()
  UnbundleFiles()
  UnbundleArchives()

This patch also introduces a Config class that locally stores the
previously global cl::opt options and arrays to allow users to call
the APIs in a multi-threaded context, and introduces an
OffloadBundler class to encapsulate the top-level API functions.

We also  lift the BundlerExecutable variable, which is specific
to the clang-offload-bundler tool, from the API, and replace
its use with an ObjcopyPath variable. This variable must be set
in order to internally call llvm-objcopy.

Finally, we move the API files from
clang/tools/clang-offload-bundler into clang/lib/Driver and
clang/include/clang/Driver.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D129873

Files:
  clang/include/clang/Driver/OffloadBundler.h
  clang/lib/Driver/CMakeLists.txt
  clang/lib/Driver/OffloadBundler.cpp
  clang/tools/clang-offload-bundler/CMakeLists.txt
  clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp

Index: clang/tools/clang-offload-bundler/CMakeLists.txt
===
--- clang/tools/clang-offload-bundler/CMakeLists.txt
+++ clang/tools/clang-offload-bundler/CMakeLists.txt
@@ -2,15 +2,16 @@
 
 add_clang_tool(clang-offload-bundler
   ClangOffloadBundler.cpp
-  
+
   DEPENDS
   intrinsics_gen
   )
 
 set(CLANG_OFFLOAD_BUNDLER_LIB_DEPS
   clangBasic
+  clangDriver
   )
-  
+
 add_dependencies(clang clang-offload-bundler)
 
 clang_target_link_libraries(clang-offload-bundler
Index: clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
===
--- clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
+++ clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
@@ -7,15 +7,14 @@
 //===--===//
 ///
 /// \file
-/// This file implements a clang-offload-bundler that bundles different
-/// files that relate with the same source code but different targets into a
-/// single one. Also the implements the opposite functionality, i.e. unbundle
-/// files previous created by this tool.
+/// This file implements a stand-alone clang-offload-bundler tool using the
+/// OffloadBundler API.
 ///
 //===--===//
 
 #include "clang/Basic/Cuda.h"
 #include "clang/Basic/Version.h"
+#include "clang/Driver/OffloadBundler.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/SmallVector.h"
@@ -56,37 +55,43 @@
 using namespace llvm;
 using namespace llvm::object;
 
-static cl::opt Help("h", cl::desc("Alias for -help"), cl::Hidden);
+static void PrintVersion(raw_ostream &OS) {
+  OS << clang::getClangToolFullVersion("clang-offload-bundler") << '\n';
+}
+
+int main(int argc, const char **argv) {
+
+  cl::opt Help("h", cl::desc("Alias for -help"), cl::Hidden);
 
-// Mark all our options with this category, everything else (except for -version
-// and -help) will be hidden.
-static cl::OptionCategory
+  // Mark all our options with this category, everything else (except for
+  // -version and -help) will be hidden.
+  cl::OptionCategory
 ClangOffloadBundlerCategory("clang-offload-bundler options");
-static cl::list
-InputFileNames("input",
+  cl::list
+InputFileNames("input", cl::ZeroOrMore,
cl::desc("Input file."
 " Can be specified multiple times "
 "for multiple input files."),
cl::cat(ClangOffloadBundlerCategory));
-static cl::list
-InputFileNamesDeprecatedOpt("inputs", cl::CommaSeparated,
+  cl::list
+InputFileNamesDeprecatedOpt("inputs", cl::CommaSeparated, cl::ZeroOrMore,
 cl::desc("[,...] (deprecated)"),
 cl::cat(ClangOffloadBundlerCategory));
-static cl::list
-OutputFileNames("output",
+  cl::list
+OutputFileNames("output", cl::ZeroOrMore,
 cl::desc("Output file."
  " Can be specified multiple times "
  "for multiple output files."),
 cl::cat(ClangOffloadBundlerCategory));
-static cl::list
-OutputFileNamesDeprecate

[PATCH] D129301: [clang-offload-bundler][NFC] Library-ize ClangOffloadBundler (1/4)

2022-07-15 Thread Jacob Lambert via Phabricator via cfe-commits
lamb-j abandoned this revision.
lamb-j added a comment.

Abandoned in favor of combined patch: https://reviews.llvm.org/D129873


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129301

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


[PATCH] D129303: [clang-offload-bundler] Library-ize ClangOffloadBundler (2/4)

2022-07-15 Thread Jacob Lambert via Phabricator via cfe-commits
lamb-j abandoned this revision.
lamb-j added a comment.

Abandoned in favor of combined patch: https://reviews.llvm.org/D129873


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129303

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


[PATCH] D129304: [clang-offload-bundler] Library-ize ClangOffloadBundler (3/4)

2022-07-15 Thread Jacob Lambert via Phabricator via cfe-commits
lamb-j abandoned this revision.
lamb-j added a comment.

Abandoned in favor of combined patch: https://reviews.llvm.org/D129873


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129304

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


[PATCH] D129305: [clang-offload-bundler][NFC] Library-ize ClangOffloadBundler (4/4)

2022-07-15 Thread Jacob Lambert via Phabricator via cfe-commits
lamb-j abandoned this revision.
lamb-j added a comment.

Abandoned in favor of combined patch: https://reviews.llvm.org/D129873


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129305

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


[PATCH] D129835: [clang] adds a discardable attribute

2022-07-15 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

> When we have a function that can have its value discarded, we can use 
> [[clang::discardable]] to indicate that the `[[nodiscard]]` attribute should 
> be ignored.

Alternatively, you can scope the pragma and declarations appropriately and then 
we don't need to add a new attribute. Is there a reason that wouldn't work for 
you?

(This attribute would be interesting to consider if we added a feature flag 
like `-fdiagnose-discarded-results` where the default is as-if everything was 
declared `nodiscard` and then you could mark the APIs with 
`[[clang::discardable]]` for the few whose results don't matter. However, 
that's also a much bigger feature because system headers are a challenge.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129835

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


[clang] 82f76c0 - [analyzer][NFC] Tidy up handler-functions in SymbolicRangeInferrer

2022-07-15 Thread Denys Petrov via cfe-commits

Author: Denys Petrov
Date: 2022-07-15T19:24:57+03:00
New Revision: 82f76c04774fbeb2313b84932afac478f010c8d0

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

LOG: [analyzer][NFC] Tidy up handler-functions in SymbolicRangeInferrer

Summary: Sorted some handler-functions into more appropriate visitor functions 
of the SymbolicRangeInferrer.
- Spread `getRangeForNegatedSub` body over several visitor functions: 
`VisitSymExpr`, `VisitSymIntExpr`, `VisitSymSymExpr`.
- Moved `getRangeForComparisonSymbol` from `infer` to `VisitSymSymExpr`.

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

Added: 


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

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp 
b/clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
index e788a7a608302..8f1e3aa039759 100644
--- a/clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
+++ b/clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
@@ -1213,13 +1213,21 @@ class SymbolicRangeInferrer
   }
 
   RangeSet VisitSymExpr(SymbolRef Sym) {
-// If we got to this function, the actual type of the symbolic
+if (Optional RS = getRangeForNegatedSym(Sym))
+  return *RS;
+// If we've reached this line, the actual type of the symbolic
 // expression is not supported for advanced inference.
 // In this case, we simply backoff to the default "let's simply
 // infer the range from the expression's type".
 return infer(Sym->getType());
   }
 
+  RangeSet VisitUnarySymExpr(const UnarySymExpr *USE) {
+if (Optional RS = getRangeForNegatedUnarySym(USE))
+  return *RS;
+return infer(USE->getType());
+  }
+
   RangeSet VisitSymIntExpr(const SymIntExpr *Sym) {
 return VisitBinaryOperator(Sym);
   }
@@ -1228,14 +1236,25 @@ class SymbolicRangeInferrer
 return VisitBinaryOperator(Sym);
   }
 
-  RangeSet VisitSymSymExpr(const SymSymExpr *Sym) {
+  RangeSet VisitSymSymExpr(const SymSymExpr *SSE) {
 return intersect(
 RangeFactory,
+// If Sym is a 
diff erence of symbols A - B, then maybe we have range
+// set stored for B - A.
+//
+// If we have range set stored for both A - B and B - A then
+// calculate the effective range set by intersecting the range set
+// for A - B and the negated range set of B - A.
+getRangeForNegatedSymSym(SSE),
+// If Sym is a comparison expression (except <=>),
+// find any other comparisons with the same operands.
+// See function description.
+getRangeForComparisonSymbol(SSE),
 // If Sym is (dis)equality, we might have some information
 // on that in our equality classes data structure.
-getRangeForEqualities(Sym),
+getRangeForEqualities(SSE),
 // And we should always check what we can get from the operands.
-VisitBinaryOperator(Sym));
+VisitBinaryOperator(SSE));
   }
 
 private:
@@ -1264,25 +1283,13 @@ class SymbolicRangeInferrer
   }
 
   RangeSet infer(SymbolRef Sym) {
-return intersect(
-RangeFactory,
-// Of course, we should take the constraint directly associated with
-// this symbol into consideration.
-getConstraint(State, Sym),
-// If Sym is a 
diff erence of symbols A - B, then maybe we have range
-// set stored for B - A.
-//
-// If we have range set stored for both A - B and B - A then
-// calculate the effective range set by intersecting the range set
-// for A - B and the negated range set of B - A.
-getRangeForNegatedSub(Sym),
-// If Sym is a comparison expression (except <=>),
-// find any other comparisons with the same operands.
-// See function description.
-getRangeForComparisonSymbol(Sym),
-// Apart from the Sym itself, we can infer quite a lot if we look
-// into subexpressions of Sym.
-Visit(Sym));
+return intersect(RangeFactory,
+ // Of course, we should take the constraint directly
+ // associated with this symbol into consideration.
+ getConstraint(State, Sym),
+ // Apart from the Sym itself, we can infer quite a lot if
+ // we look into subexpressions of Sym.
+ Visit(Sym));
   }
 
   RangeSet infer(EquivalenceClass Class) {
@@ -1443,38 +1450,53 @@ class SymbolicRangeInferrer
 return RangeFactory.deletePoint(Domain, IntType.getZeroValue());
   }
 
-  Optional getRangeForNegatedSub(SymbolRef Sym) {
+  template 
+  Optional getRangeForNegatedExpr(ProduceNegatedSymFunc F,
+QualType T) {
 // Do

[PATCH] D129678: [analyzer][NFC] Tidy up handler-functions in SymbolicRangeInferrer

2022-07-15 Thread Denys Petrov via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG82f76c04774f: [analyzer][NFC] Tidy up handler-functions in 
SymbolicRangeInferrer (authored by ASDenysPetrov).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129678

Files:
  clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp

Index: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
===
--- clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
+++ clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
@@ -1213,13 +1213,21 @@
   }
 
   RangeSet VisitSymExpr(SymbolRef Sym) {
-// If we got to this function, the actual type of the symbolic
+if (Optional RS = getRangeForNegatedSym(Sym))
+  return *RS;
+// If we've reached this line, the actual type of the symbolic
 // expression is not supported for advanced inference.
 // In this case, we simply backoff to the default "let's simply
 // infer the range from the expression's type".
 return infer(Sym->getType());
   }
 
+  RangeSet VisitUnarySymExpr(const UnarySymExpr *USE) {
+if (Optional RS = getRangeForNegatedUnarySym(USE))
+  return *RS;
+return infer(USE->getType());
+  }
+
   RangeSet VisitSymIntExpr(const SymIntExpr *Sym) {
 return VisitBinaryOperator(Sym);
   }
@@ -1228,14 +1236,25 @@
 return VisitBinaryOperator(Sym);
   }
 
-  RangeSet VisitSymSymExpr(const SymSymExpr *Sym) {
+  RangeSet VisitSymSymExpr(const SymSymExpr *SSE) {
 return intersect(
 RangeFactory,
+// If Sym is a difference of symbols A - B, then maybe we have range
+// set stored for B - A.
+//
+// If we have range set stored for both A - B and B - A then
+// calculate the effective range set by intersecting the range set
+// for A - B and the negated range set of B - A.
+getRangeForNegatedSymSym(SSE),
+// If Sym is a comparison expression (except <=>),
+// find any other comparisons with the same operands.
+// See function description.
+getRangeForComparisonSymbol(SSE),
 // If Sym is (dis)equality, we might have some information
 // on that in our equality classes data structure.
-getRangeForEqualities(Sym),
+getRangeForEqualities(SSE),
 // And we should always check what we can get from the operands.
-VisitBinaryOperator(Sym));
+VisitBinaryOperator(SSE));
   }
 
 private:
@@ -1264,25 +1283,13 @@
   }
 
   RangeSet infer(SymbolRef Sym) {
-return intersect(
-RangeFactory,
-// Of course, we should take the constraint directly associated with
-// this symbol into consideration.
-getConstraint(State, Sym),
-// If Sym is a difference of symbols A - B, then maybe we have range
-// set stored for B - A.
-//
-// If we have range set stored for both A - B and B - A then
-// calculate the effective range set by intersecting the range set
-// for A - B and the negated range set of B - A.
-getRangeForNegatedSub(Sym),
-// If Sym is a comparison expression (except <=>),
-// find any other comparisons with the same operands.
-// See function description.
-getRangeForComparisonSymbol(Sym),
-// Apart from the Sym itself, we can infer quite a lot if we look
-// into subexpressions of Sym.
-Visit(Sym));
+return intersect(RangeFactory,
+ // Of course, we should take the constraint directly
+ // associated with this symbol into consideration.
+ getConstraint(State, Sym),
+ // Apart from the Sym itself, we can infer quite a lot if
+ // we look into subexpressions of Sym.
+ Visit(Sym));
   }
 
   RangeSet infer(EquivalenceClass Class) {
@@ -1443,38 +1450,53 @@
 return RangeFactory.deletePoint(Domain, IntType.getZeroValue());
   }
 
-  Optional getRangeForNegatedSub(SymbolRef Sym) {
+  template 
+  Optional getRangeForNegatedExpr(ProduceNegatedSymFunc F,
+QualType T) {
 // Do not negate if the type cannot be meaningfully negated.
-if (!Sym->getType()->isUnsignedIntegerOrEnumerationType() &&
-!Sym->getType()->isSignedIntegerOrEnumerationType())
+if (!T->isUnsignedIntegerOrEnumerationType() &&
+!T->isSignedIntegerOrEnumerationType())
   return llvm::None;
 
-const RangeSet *NegatedRange = nullptr;
-SymbolManager &SymMgr = State->getSymbolManager();
-if (const auto *USE = dyn_cast(Sym)) {
-  if (USE->getOpcode() == UO_Minus) {
-// Just get the operand when we negate a symbol that is already negated.
-// -(-a) == a
-NegatedRange = getConstraint(State, USE->getOperand());

[PATCH] D129832: [sanitizer] Add "mainsrc" prefix to sanitizer special case list

2022-07-15 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment.



In D129832#3654436 , @vitalybuka 
wrote:

> The patch is LGTM, but please consider to keep an ignorelist limited.

Thanks!

In D129832#3655393 , @kstoimenov 
wrote:

> I think the name 'mainsrc' is slightly misleading because of the association 
> with the 'main' function. Maybe something like primarysrc would be better to 
> avoid this confusion?

How   about `mainfile`? Clang resources to the file as the `main file`. I 
picked `src` suffix just to be similar to `src`...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129832

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


[PATCH] D129835: [clang] adds a discardable attribute

2022-07-15 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb added a subscriber: ldionne.
cjdb added a comment.

In D129835#3655487 , @aaron.ballman 
wrote:

>> When we have a function that can have its value discarded, we can use 
>> [[clang::discardable]] to indicate that the `[[nodiscard]]` attribute should 
>> be ignored.
>
> Alternatively, you can scope the pragma and declarations appropriately and 
> then we don't need to add a new attribute. Is there a reason that wouldn't 
> work for you?

A key problem here (IMO) is that this requires manually pushing and popping the 
pragma as it is required. With `[[discardable]]` one just needs to push/pop at 
the extremes of a file (and if a future version of module maps supports global 
pragmas for a module, there too, but that's a discussion that requires a design 
doc).

It's also good for documenting "hey you can discard this thing in my sea of 
nodiscard interfaces" on the interface itself. That coupling is also good for 
correctness because it means something can't accidentally slip in/out of the 
pragma's scope.

> (This attribute would be interesting to consider if we added a feature flag 
> like `-fdiagnose-discarded-results` where the default is as-if everything was 
> declared `nodiscard` and then you could mark the APIs with 
> `[[clang::discardable]]` for the few whose results don't matter. However, 
> that's also a much bigger feature because system headers are a challenge.)

The primary reason for me wanting to add this attribute comes from my libc++ 
days, where I wanted to mark whatever made sense as nodiscard. @ldionne was in 
favour in principle, but against in practice; noting that nodiscard would 
clutter the interface too much, and it'd be better if the situation were 
reversed (which is what this patch does). I think adding this is still a good 
step for getting us to a world where `-fdiagnose-discarded-results` can be a 
real consideration, especially if libc++ and llvm-libc adopt it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129835

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


[PATCH] D129748: [Modules] Disable preferred_name attribute in C++20 Modules

2022-07-15 Thread Tom Honermann via Phabricator via cfe-commits
tahonermann added a comment.

I'm not following what the root problem is. You stated:

> When we write the attribute preferred_name(foo) in ASTWriter, the compiler 
> would try to write the type for the argument foo. Then when the compiler 
> write the type for foo, the compiler find the type for foo is a TypeDef type. 
> So the compiler would write the corresponding type foo_templ. The key 
> point here is that the AST for foo_templ is complete now. Since the AST 
> for foo_templ is constructed in Sema.

Are you saying that the act of writing the AST is triggering the 
instantiation/completion of `foo_templ`? Serialization should certainly 
not cause such side effects.

I'm likewise not following the concern with reading the AST:

> When we read the attribute preferred_name(foo), we would read the type for 
> the argument foo and then we would try to read the type foo_templ 
> later. However, the key problem here is that when we read foo_templ, 
> its AST is not constructed yet! So we get a different type with the writer 
> writes. So here is the ODR violation.

I wouldn't expect the same `Sema` instance to be used to both write and read 
the same AST.

The test case defines two modules (`A` and `Use`) that both have definitions of 
`foo_templ`, `foo`, and `foo_templ` in their global module fragments. It 
sounds to me like the issue here might be that these definitions are not 
getting merged (perhaps because of the issue you are trying to describe and 
address).

Can you try again to explain the root problem? Perhaps with references to 
relevant code?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129748

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


[PATCH] D128372: [Clang-Tidy] Empty Check

2022-07-15 Thread Abraham Corea Diaz via Phabricator via cfe-commits
abrahamcd added a comment.

Gentle ping!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128372

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


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

2022-07-15 Thread Denys Petrov via Phabricator via cfe-commits
ASDenysPetrov added a comment.

@manas

I'm sorry but it seems like I brought you a new work :-) I've just loaded these 
two patches (D129678  and D129498 
) and now you have to rebase your changes. 
But there is a good news as well. You will be able to use 
`clang_analyzer_value` function for your tests if needed.

I appreciate your efforts but I should tell that I'm currently working on the 
thing that should resolve the issue you are trying to solve D103096 
.

> The coverage showing unreachability of `VisitBinaryOperator` for 
> concrete integer cases.

Maybe it's better to remove that unreachable part but leave the tests for 
concrete ints just to verify that all the cases are covered.

Also I expect to see test case for `short-ushort`, `char-uchar` pairs, because 
if it would turn out that the `int-uint` is only pair that we handle the patch 
would be disappointing, unfortunately.


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


[clang] bc08c3c - [analyzer] Add new function `clang_analyzer_value` to ExprInspectionChecker

2022-07-15 Thread Denys Petrov via cfe-commits

Author: Denys Petrov
Date: 2022-07-15T20:07:04+03:00
New Revision: bc08c3cb7f8e797fee14e96eedd3dc358608ada3

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

LOG: [analyzer] Add new function `clang_analyzer_value` to ExprInspectionChecker

Summary: Introduce a new function 'clang_analyzer_value'. It emits a report 
that in turn prints a RangeSet or APSInt associated with SVal. If there is no 
associated value, prints "n/a".

Added: 
clang/test/Analysis/print-ranges.cpp

Modified: 
clang/docs/analyzer/developer-docs/DebugChecks.rst
clang/include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
clang/lib/StaticAnalyzer/Core/SVals.cpp

Removed: 




diff  --git a/clang/docs/analyzer/developer-docs/DebugChecks.rst 
b/clang/docs/analyzer/developer-docs/DebugChecks.rst
index 7a837659576d9..d038cb5d7ccac 100644
--- a/clang/docs/analyzer/developer-docs/DebugChecks.rst
+++ b/clang/docs/analyzer/developer-docs/DebugChecks.rst
@@ -309,6 +309,33 @@ ExprInspection checks
   clang_analyzer_dumpExtent(a);   // expected-warning {{8 S64b}}
   clang_analyzer_dumpElementCount(a); // expected-warning {{2 S64b}}
 }
+
+- ``clang_analyzer_value(a single argument of integer or pointer type)``
+
+  Prints an associated value for the given argument.
+  Supported argument types are integers, enums and pointers.
+  The value can be represented either as a range set or as a concrete integer.
+  For the rest of the types function prints ``n/a`` (aka not available).
+  
+  **Note:** This function will print nothing for clang built with Z3 
constraint manager.
+  This may cause crashes of your tests. To manage this use one of the test 
constraining
+  techniques:
+  
+  * llvm-lit commands ``REQUIRES no-z3`` or ``UNSUPPORTED z3`` `See for 
details. `_
+  
+  * a preprocessor directive ``#ifndef ANALYZER_CM_Z3``
+  
+  * a clang command argument ``-analyzer-constraints=range``
+
+  Example usage::
+
+void print(char c, unsigned u) {
+  clang_analyzer_value(c); // expected-warning {{8s:{ [-128, 127] }}}
+  if(u != 42)
+ clang_analyzer_value(u); // expected-warning {{32u:{ [0, 41], [43, 
4294967295] }}}
+  else
+ clang_analyzer_value(u); // expected-warning {{32u:42}}
+}
 
 Statistics
 ==

diff  --git 
a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h 
b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
index 22b405919bc17..ca6d7849d6210 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
@@ -119,6 +119,9 @@ class ConstraintManager {
  const char *NL, unsigned int Space,
  bool IsDot) const = 0;
 
+  virtual void printValue(raw_ostream &Out, ProgramStateRef State,
+  SymbolRef Sym) {}
+
   /// Convenience method to query the state to see if a symbol is null or
   /// not null, or if neither assumption can be made.
   ConditionTruthVal isNull(ProgramStateRef State, SymbolRef Sym) {

diff  --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h 
b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
index 69f19f7d85655..c9c21fcf230ef 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
@@ -169,6 +169,11 @@ class SVal {
   /// should continue to the base regions if the region is not symbolic.
   SymbolRef getAsSymbol(bool IncludeBaseRegions = false) const;
 
+  /// If this SVal is loc::ConcreteInt or nonloc::ConcreteInt,
+  /// return a pointer to APSInt which is held in it.
+  /// Otherwise, return nullptr.
+  const llvm::APSInt *getAsInteger() const;
+
   const MemRegion *getAsRegion() const;
 
   /// printJson - Pretty-prints in JSON format.

diff  --git a/clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
index 1c33648b2b321..ec1b0a70d7d3b 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
@@ -40,6 +40,7 @@ class ExprInspectionChecker
   void analyzerNumTimesReached(const CallExpr *CE, CheckerContext &C) const;
   void analyzerCrash(const CallExpr *CE, CheckerContext &C) const;
   void analyzerWarnOnDeadSymbol(const CallExpr *CE, CheckerContext &C) const;
+  void analyzerVal

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

2022-07-15 Thread Roy Jacobson via Phabricator via cfe-commits
royjacobson updated this revision to Diff 445044.
royjacobson edited the summary of this revision.
royjacobson added a comment.

Address Corentin's feedback.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128619

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/AST/DeclCXX.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/AST/conditionally-trivial-smfs.cpp
  clang/test/Lexer/cxx-features.cpp
  clang/test/SemaCXX/constrained-special-member-functions.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -927,7 +927,7 @@
   

 https://wg21.link/p0848r3";>P0848R3
-No
+Clang 15
   
   
 https://wg21.link/p1616r1";>P1616R1
Index: clang/test/SemaCXX/constrained-special-member-functions.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/constrained-special-member-functions.cpp
@@ -0,0 +1,130 @@
+// RUN: %clang_cc1 -verify -std=c++20 %s
+// expected-no-diagnostics
+
+template 
+concept C0 = (N == 0);
+template 
+concept C1 = (N == 1);
+template 
+concept C2 = (N == 2);
+
+// Checks are indexed by:
+// Definition:
+//  1. Explicitly defaulted definition
+//  2. Deleted definition
+//  3. User provided definition
+// We have a less constrained user provided method that should not disable
+// the (copyable) triviality of the type.
+
+// Note that because Clang does not implement DRs 1496 and 1734, we say some
+// classes are trivial when the SMFs are deleted.
+
+template 
+struct DefaultConstructorChecker {
+DefaultConstructorChecker() requires C0 = default;
+DefaultConstructorChecker() requires C1 = delete;
+DefaultConstructorChecker() requires C2;
+DefaultConstructorChecker();
+};
+static_assert(__is_trivially_copyable(DefaultConstructorChecker<0>));
+static_assert(__is_trivially_copyable(DefaultConstructorChecker<1>));
+static_assert(__is_trivially_copyable(DefaultConstructorChecker<2>));
+static_assert(__is_trivially_copyable(DefaultConstructorChecker<3>));
+static_assert(__is_trivial(DefaultConstructorChecker<0>));
+// FIXME: DR1496
+static_assert(__is_trivial(DefaultConstructorChecker<1>));
+static_assert(!__is_trivial(DefaultConstructorChecker<2>));
+static_assert(!__is_trivial(DefaultConstructorChecker<3>));
+
+template 
+struct CopyConstructorChecker {
+CopyConstructorChecker(const CopyConstructorChecker&) requires C0 = default;
+CopyConstructorChecker(const CopyConstructorChecker&) requires C1 = delete;
+CopyConstructorChecker(const CopyConstructorChecker&) requires C2;
+CopyConstructorChecker(const CopyConstructorChecker&);
+};
+
+static_assert(__is_trivially_copyable(CopyConstructorChecker<0>));
+// FIXME: DR1734
+static_assert(__is_trivially_copyable(CopyConstructorChecker<1>));
+static_assert(!__is_trivially_copyable(CopyConstructorChecker<2>));
+static_assert(!__is_trivially_copyable(CopyConstructorChecker<3>));
+static_assert(!__is_trivial(CopyConstructorChecker<0>));
+static_assert(!__is_trivial(CopyConstructorChecker<1>));
+static_assert(!__is_trivial(CopyConstructorChecker<2>));
+static_assert(!__is_trivial(CopyConstructorChecker<3>));
+
+template 
+struct MoveConstructorChecker {
+MoveConstructorChecker(MoveConstructorChecker&&) requires C0 = default;
+MoveConstructorChecker(MoveConstructorChecker&&) requires C1 = delete;
+MoveConstructorChecker(MoveConstructorChecker&&) requires C2;
+MoveConstructorChecker(MoveConstructorChecker&&);
+};
+
+static_assert(__is_trivially_copyable(MoveConstructorChecker<0>));
+// FIXME: DR1734
+static_assert(__is_trivially_copyable(MoveConstructorChecker<1>));
+static_assert(!__is_trivially_copyable(MoveConstructorChecker<2>));
+static_assert(!__is_trivially_copyable(MoveConstructorChecker<3>));
+static_assert(!__is_trivial(MoveConstructorChecker<0>));
+static_assert(!__is_trivial(MoveConstructorChecker<1>));
+static_assert(!__is_trivial(MoveConstructorChecker<2>));
+static_assert(!__is_trivial(MoveConstructorChecker<3>));
+
+template 
+struct MoveAssignmentChecker {
+MoveAssignmentChecker& operator=(MoveAssignmentChecker&&) requires C0 = default;
+MoveAssignmentChecker& operator=(MoveAssignmentChecker&&) requires C1 = delete;
+MoveAssignmentChecker& operator=(MoveAssignmentChecker&&) requires C2;
+MoveAssignmentChecker& operator=(MoveAssignmentChecker&&);
+};
+
+static_assert(__is_trivially_copyable(MoveAssignmentChecker<0>));
+// FIXME: DR1734.
+static_assert(__is_trivially_copyable(MoveAssignmentChecker<1>));
+static_assert(!__is_trivially_copyable(MoveAssignmentChecker<2>));
+static_assert(!__is_trivially_copyable(MoveAssignmentChecker<3>));
+static

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

2022-07-15 Thread Roy Jacobson via Phabricator via cfe-commits
royjacobson marked 4 inline comments as done and an inline comment as not done.
royjacobson added inline comments.



Comment at: clang/lib/Sema/SemaDecl.cpp:17960-17961
+}
+if (AnotherMethodIsMoreConstrained)
+  break;
+  }

cor3ntin wrote:
> Is that codepath ever taken?
> I wonder if there is a way to do that that is not n^2. Maybe not.
It's not, I didn't mean to leave it there. Thanks for the catch :)


Generally finding a maximal set in a partial ordering is O(n^2) in the worst 
case which is when no two objects are comparable.

We could optimize this a bit for some cases: We can group the functions by 
their kind, and we can skip comparing functions if we already know that they 
are not maximal. But since the number of SMFs expected in a class is very (1-3) 
small, I didn't think those possible improvements are worth the extra 
complexity.

Another possible optimization is we could short-circuit this evaluation if we 
know that a type is not trivially [copyable], since that's the only outcome we 
really care about, but then the AST is left in a very awkward state.




Comment at: clang/test/AST/conditionally-trivial-smfs.cpp:39
+
+template struct DefaultConstructorCheck<2>;
+// CHECK: "kind": "ClassTemplateSpecializationDecl",

BRevzin wrote:
> It's possible that I just don't understand what these tests actually mean 
> but... where is the test for `DefaultConstructorCheck<2>` having a deleted 
> default constructor, or `DefaultConstructorCheck<3>` having a non-defaulted 
> one?
> 
> It'd also be worthwhile to have at least one test with constaints that 
> subsume each other instead of being mutually exclusive. 
Should I check this? Except destructors, the other SMFs are found during 
overload resolution using the usual lookup that already takes into account 
delete/default/constraints etc.

This patch is about making sure that we set the triviality attributes correctly 
according to the eligible functions, so this is what I added tests for.

Most of this testing is done in the sema test, but I need this AST test as well 
to make sure we get the `canPassInRegisters` attribute correctly - we're doing 
some custom processing over the functions without the usual type attributes 
because there are some weird compatibility edge cases.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128619

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


[PATCH] D129835: [clang] adds a discardable attribute

2022-07-15 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D129835#3655548 , @cjdb wrote:

> In D129835#3655487 , @aaron.ballman 
> wrote:
>
>>> When we have a function that can have its value discarded, we can use 
>>> [[clang::discardable]] to indicate that the `[[nodiscard]]` attribute 
>>> should be ignored.
>>
>> Alternatively, you can scope the pragma and declarations appropriately and 
>> then we don't need to add a new attribute. Is there a reason that wouldn't 
>> work for you?
>
> A key problem here (IMO) is that this requires manually pushing and popping 
> the pragma as it is required.

That's a feature to me, not a bug. It means you have to actually think about 
what you're doing instead of being surprised. This is the same reason our 
community standards want you to keep anonymous namespaces as small as possible 
-- it's too easy to have absolutely no idea about the behavior when you're in 
the middle of a block of code and the relevant part is off-screen somewhere.

> With `[[discardable]]` one just needs to push/pop at the extremes of a file 
> (and if a future version of module maps supports global pragmas for a module, 
> there too, but that's a discussion that requires a design doc).

I understood that, I just don't think that's a good thing. This is basically an 
attribute that says "I know we said we wanted everything here to be nodiscard, 
but JUST KIDDING not this one!" which is not a very clean approach to writing 
headers.

> It's also good for documenting "hey you can discard this thing in my sea of 
> nodiscard interfaces" on the interface itself. That coupling is also good for 
> correctness because it means something can't accidentally slip in/out of the 
> pragma's scope.

I'd like to see some examples of that. In my experience, attributes are most 
often hidden behind macros which are sometimes then combined with other macros, 
etc, so whether the attribute is effective or not is already not always obvious 
from the interface; I don't see how this will move the needle.

>> (This attribute would be interesting to consider if we added a feature flag 
>> like `-fdiagnose-discarded-results` where the default is as-if everything 
>> was declared `nodiscard` and then you could mark the APIs with 
>> `[[clang::discardable]]` for the few whose results don't matter. However, 
>> that's also a much bigger feature because system headers are a challenge.)
>
> The primary reason for me wanting to add this attribute comes from my libc++ 
> days, where I wanted to mark whatever made sense as nodiscard. @ldionne was 
> in favour in principle, but against in practice; noting that nodiscard would 
> clutter the interface too much, and it'd be better if the situation were 
> reversed (which is what this patch does). I think adding this is still a good 
> step for getting us to a world where `-fdiagnose-discarded-results` can be a 
> real consideration, especially if libc++ and llvm-libc adopt it.

Yes, I wish the default were correct as well, but it's not and it won't ever be 
corrected for practicality reasons. I don't know what libc++'s goals are in 
terms of other compilers, but it seems like using a clang-specific pragma would 
be making it more difficult to use libc++ with any other compiler.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129835

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


[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

2022-07-15 Thread Barry Revzin via Phabricator via cfe-commits
BRevzin added inline comments.



Comment at: clang/test/AST/conditionally-trivial-smfs.cpp:39
+
+template struct DefaultConstructorCheck<2>;
+// CHECK: "kind": "ClassTemplateSpecializationDecl",

royjacobson wrote:
> BRevzin wrote:
> > It's possible that I just don't understand what these tests actually mean 
> > but... where is the test for `DefaultConstructorCheck<2>` having a deleted 
> > default constructor, or `DefaultConstructorCheck<3>` having a non-defaulted 
> > one?
> > 
> > It'd also be worthwhile to have at least one test with constaints that 
> > subsume each other instead of being mutually exclusive. 
> Should I check this? Except destructors, the other SMFs are found during 
> overload resolution using the usual lookup that already takes into account 
> delete/default/constraints etc.
> 
> This patch is about making sure that we set the triviality attributes 
> correctly according to the eligible functions, so this is what I added tests 
> for.
> 
> Most of this testing is done in the sema test, but I need this AST test as 
> well to make sure we get the `canPassInRegisters` attribute correctly - we're 
> doing some custom processing over the functions without the usual type 
> attributes because there are some weird compatibility edge cases.
> 
One of the motivations for the paper is to ensure that like given:

```
template 
struct optional {
optional(optional const&) requires copyable && trivially_copyableT> = 
default;
optional(optional const&) requires copyable;
};
```

`optional` is copyable (but not trivially copyable), `optional` is 
trivially copyable, and `optional>` isn't copyable. I'm not 
sure what in here checks if that works. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128619

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


[PATCH] D129832: [sanitizer] Add "mainsrc" prefix to sanitizer special case list

2022-07-15 Thread Vitaly Buka via Phabricator via cfe-commits
vitalybuka added a comment.

I guess Kirills' complain is about **main** part, not **src**.
But I have no idea what name could be better.
I guess thechnically it's preprocessed src? :)
To me as is name is fine.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129832

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


[PATCH] D129832: [sanitizer] Add "mainsrc" prefix to sanitizer special case list

2022-07-15 Thread Vitaly Buka via Phabricator via cfe-commits
vitalybuka added a comment.

"unit:" ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129832

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


[PATCH] D129832: [sanitizer] Add "mainfile" prefix to sanitizer special case list

2022-07-15 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 445051.
MaskRay retitled this revision from "[sanitizer] Add "mainsrc" prefix to 
sanitizer special case list" to "[sanitizer] Add "mainfile" prefix to sanitizer 
special case list".
MaskRay edited the summary of this revision.
MaskRay added a comment.

mainsrc => mainfile

Clang names the file "main file". CC1 has options like -main-file-name.
Using "mainfile" instead of "mainsrc" avoids introducing new terminology.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129832

Files:
  clang/docs/SanitizerSpecialCaseList.rst
  clang/include/clang/Basic/NoSanitizeList.h
  clang/lib/Basic/NoSanitizeList.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGen/sanitize-ignorelist-mainfile.c
  llvm/include/llvm/Support/SpecialCaseList.h

Index: llvm/include/llvm/Support/SpecialCaseList.h
===
--- llvm/include/llvm/Support/SpecialCaseList.h
+++ llvm/include/llvm/Support/SpecialCaseList.h
@@ -19,9 +19,9 @@
 //   prefix:wildcard_expression[=category]
 // If category is not specified, it is assumed to be empty string.
 // Definitions of "prefix" and "category" are sanitizer-specific. For example,
-// sanitizer exclusion support prefixes "src", "fun" and "global".
-// Wildcard expressions define, respectively, source files, functions or
-// globals which shouldn't be instrumented.
+// sanitizer exclusion support prefixes "src", "mainfile", "fun" and "global".
+// Wildcard expressions define, respectively, source files, main files,
+// functions or globals which shouldn't be instrumented.
 // Examples of categories:
 //   "functional": used in DFSan to list functions with pure functional
 // semantics.
@@ -37,6 +37,7 @@
 // type:*Namespace::ClassName*=init
 // src:file_with_tricky_code.cc
 // src:ignore-global-initializers-issues.cc=init
+// mainfile:main_file.cc
 //
 // [dataflow]
 // # Functions with pure functional semantics:
Index: clang/test/CodeGen/sanitize-ignorelist-mainfile.c
===
--- /dev/null
+++ clang/test/CodeGen/sanitize-ignorelist-mainfile.c
@@ -0,0 +1,41 @@
+/// Test mainfile in a sanitizer special case list.
+// RUN: rm -rf %t && split-file %s %t && cd %t
+// RUN: %clang_cc1 -emit-llvm -triple x86_64 -fsanitize=address,alignment a.c -o - | FileCheck %s --check-prefixes=CHECK,DEFAULT
+// RUN: %clang_cc1 -emit-llvm -triple x86_64 -fsanitize=address,alignment -fsanitize-ignorelist=a.list a.c -o - | FileCheck %s --check-prefixes=CHECK,IGNORE
+// RUN: %clang_cc1 -emit-llvm -triple x86_64 -fsanitize=address,alignment -fsanitize-ignorelist=b.list a.c -o - | FileCheck %s --check-prefixes=CHECK,IGNORE
+
+//--- a.list
+mainfile:*a.c
+
+//--- b.list
+[address]
+mainfile:*a.c
+
+[alignment]
+mainfile:*.c
+
+//--- a.h
+int global_h;
+
+static inline int load(int *x) {
+  return *x;
+}
+
+//--- a.c
+#include "a.h"
+
+int global_c;
+
+int foo(void *x) {
+  return load(x);
+}
+
+// DEFAULT: @___asan_gen_{{.*}} = {{.*}} c"global_h\00"
+// DEFAULT: @___asan_gen_{{.*}} = {{.*}} c"global_c\00"
+// IGNORE-NOT:  @___asan_gen_
+
+// CHECK-LABEL: define {{.*}}@load(
+// DEFAULT:   call void @__ubsan_handle_type_mismatch_v1_abort(
+// DEFAULT:   call void @__asan_report_load4(
+// IGNORE-NOT:call void @__ubsan_handle_type_mismatch_v1_abort(
+// IGNORE-NOT:call void @__asan_report_load4(
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -2780,16 +2780,18 @@
   // NoSanitize by function name.
   if (NoSanitizeL.containsFunction(Kind, Fn->getName()))
 return true;
-  // NoSanitize by location.
+  // NoSanitize by location. Check "mainfile" prefix.
+  auto &SM = Context.getSourceManager();
+  const FileEntry &MainFile = *SM.getFileEntryForID(SM.getMainFileID());
+  if (NoSanitizeL.containsMainFile(Kind, MainFile.getName()))
+return true;
+
+  // Check "src" prefix.
   if (Loc.isValid())
 return NoSanitizeL.containsLocation(Kind, Loc);
   // If location is unknown, this may be a compiler-generated function. Assume
   // it's located in the main file.
-  auto &SM = Context.getSourceManager();
-  if (const auto *MainFile = SM.getFileEntryForID(SM.getMainFileID())) {
-return NoSanitizeL.containsFile(Kind, MainFile->getName());
-  }
-  return false;
+  return NoSanitizeL.containsFile(Kind, MainFile.getName());
 }
 
 bool CodeGenModule::isInNoSanitizeList(SanitizerMask Kind,
@@ -2799,8 +2801,13 @@
   const auto &NoSanitizeL = getContext().getNoSanitizeList();
   if (NoSanitizeL.containsGlobal(Kind, GV->getName(), Category))
 return true;
+  auto &SM = Context.getSourceManager();
+  if (NoSanitizeL.containsMainFile(
+  Kind, SM.getFileEntryForID(SM.getMainFileID())->getName(), Cat

[PATCH] D129881: [C] Strengthen -Wint-conversion to default to an error

2022-07-15 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman created this revision.
aaron.ballman added reviewers: efriedma, jyknight, erichkeane, 
clang-language-wg.
Herald added subscribers: steakhal, Enna1, kosarev, arphaman, jvesely.
Herald added a project: All.
aaron.ballman requested review of this revision.
Herald added projects: clang, Sanitizers, clang-tools-extra.
Herald added a subscriber: Sanitizers.

Clang has traditionally allowed C programs to implicitly convert integers to 
pointers and pointers to integers, despite it not being valid to do so except 
under special circumstances (like converting the integer 0, which is the null 
pointer constant, to a pointer). In C89, this would result in undefined 
behavior per 3.3.4, and in C99 this rule was strengthened to be a constraint 
violation instead. Constraint violations are most often handled as an error.

This patch changes the warning to default to an error in all C modes (it is 
already an error in C++). This gives us better security posture by calling out 
potential programmer mistakes in code but still allows users who need this 
behavior to use `-Wno-error=int-conversion` to retain the warning behavior, or 
`-Wno-int-conversion` to silence the diagnostic entirely.

When modifying the diagnostics, I observed that 
`ext_typecheck_convert_int_pointer` was already marked `SFINAEFailure` but 
`ext_typecheck_convert_pointer_int` was not. This appeared to be an oversight 
which I corrected. That resulted in needing a change to not perform SFINAE 
traps in C mode. If desired, I can split those changes into a separate review, 
but they felt related enough to this change to be reasonable to roll in here by 
virtue of changing the behavior of the diagnostic.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D129881

Files:
  clang-tools-extra/test/clang-tidy/checkers/bugprone/no-escape.m
  clang-tools-extra/test/clang-tidy/checkers/performance/no-int-to-ptr.c
  clang/bindings/python/tests/cindex/test_diagnostics.py
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/Analysis/ObjCProperties.m
  clang/test/Analysis/bsd-string.c
  clang/test/Analysis/novoidtypecrash.c
  clang/test/Analysis/null-deref-ps.c
  clang/test/Analysis/number-object-conversion.c
  clang/test/Analysis/number-object-conversion.m
  clang/test/Analysis/pr22954.c
  clang/test/C/drs/dr0xx.c
  clang/test/C/drs/dr2xx.c
  clang/test/CodeGen/2008-03-05-syncPtr.c
  clang/test/CodeGen/2008-07-31-promotion-of-compound-pointer-arithmetic.c
  clang/test/CodeGen/aarch64-mops.c
  clang/test/CodeGen/address-space-cast.c
  clang/test/CodeGen/const-init.c
  clang/test/CodeGen/pointer-arithmetic.c
  clang/test/CodeGen/pointer-to-int.c
  clang/test/CodeGen/statements.c
  clang/test/CodeGen/struct-init.c
  clang/test/CodeGen/vla.c
  clang/test/CodeGenObjC/block-ptr-type-crash.m
  clang/test/CodeGenOpenCL/builtins-generic-amdgcn.cl
  clang/test/FixIt/dereference-addressof.c
  clang/test/FixIt/selector-fixit.m
  clang/test/Misc/serialized-diags.c
  clang/test/Misc/tabstop.c
  clang/test/Modules/config_macros.m
  clang/test/PCH/objc_exprs.m
  clang/test/Parser/implicit-casts.c
  clang/test/Sema/array-init.c
  clang/test/Sema/atomic-ops.c
  clang/test/Sema/block-return.c
  clang/test/Sema/builtin-alloca-with-align.c
  clang/test/Sema/builtin-assume-aligned.c
  clang/test/Sema/builtin-dump-struct.c
  clang/test/Sema/builtins-bpf.c
  clang/test/Sema/builtins.c
  clang/test/Sema/compound-literal.c
  clang/test/Sema/conditional-expr.c
  clang/test/Sema/enum.c
  clang/test/Sema/extern-redecl.c
  clang/test/Sema/format-strings.c
  clang/test/Sema/function-redecl.c
  clang/test/Sema/function.c
  clang/test/Sema/i-c-e.c
  clang/test/Sema/indirect-goto.c
  clang/test/Sema/matrix-type-builtins.c
  clang/test/Sema/nullability.c
  clang/test/SemaObjC/argument-checking.m
  clang/test/SemaObjC/comptypes-7.m
  clang/test/SemaObjC/ivar-lookup-resolution-builtin.m
  clang/test/SemaObjC/message.m
  clang/test/SemaObjC/method-lookup-5.m
  clang/test/SemaObjC/nullability.m
  clang/test/SemaObjC/objc-container-subscripting-3.m
  clang/test/SemaObjC/objc-literal-fixit.m
  clang/test/SemaObjC/signed-char-bool-conversion.m
  clang/test/SemaOpenCL/atomic-ops.cl
  clang/test/SemaOpenCL/builtins-amdgcn-error.cl
  clang/test/VFS/Inputs/external-names.h
  clang/test/VFS/external-names.c
  compiler-rt/test/dfsan/gep.c
  compiler-rt/test/dfsan/sigaction.c

Index: compiler-rt/test/dfsan/sigaction.c
===
--- compiler-rt/test/dfsan/sigaction.c
+++ compiler-rt/test/dfsan/sigaction.c
@@ -1,5 +1,5 @@
-// RUN: %clang_dfsan -DUSE_SIGNAL_ACTION %s -o %t && %run %t
-// RUN: %clang_dfsan %s -o %t && %run %t
+// RUN: %clang_dfsan -DUSE_SIGNAL_ACTION -Wno-error=int-conversion %s -o %t && %run %t
+// RUN: %clang_dfsan -Wno-error=int-conversion %s -o %t && %run %t
 //
 // REQUIRES: x86_64-target-arch
 
Index: compiler-rt/test/dfsan/gep.c
===

[clang] 0d5a62f - [sanitizer] Add "mainfile" prefix to sanitizer special case list

2022-07-15 Thread Fangrui Song via cfe-commits

Author: Fangrui Song
Date: 2022-07-15T10:39:26-07:00
New Revision: 0d5a62faca5924c5a197faa946b5b78b3d80a0b2

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

LOG: [sanitizer] Add "mainfile" prefix to sanitizer special case list

When an issue exists in the main file (caller) instead of an included file
(callee), using a `src` pattern applying to the included file may be
inappropriate if it's the caller's responsibility. Add `mainfile` prefix to 
check
the main filename.

For the example below, the issue may reside in a.c (foo should not be called
with a misaligned pointer or foo should switch to an unaligned load), but with
`src` we can only apply to the innocent callee a.h. With this patch we can use
the more appropriate `mainfile:a.c`.
```
//--- a.h
// internal linkage
static inline int load(int *x) { return *x; }

//--- a.c, -fsanitize=alignment
#include "a.h"
int foo(void *x) { return load(x); }
```

See the updated clang/docs/SanitizerSpecialCaseList.rst for a caveat due
to C++ vague linkage functions.

Reviewed By: #sanitizers, kstoimenov, vitalybuka

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

Added: 
clang/test/CodeGen/sanitize-ignorelist-mainfile.c

Modified: 
clang/docs/SanitizerSpecialCaseList.rst
clang/include/clang/Basic/NoSanitizeList.h
clang/lib/Basic/NoSanitizeList.cpp
clang/lib/CodeGen/CodeGenModule.cpp
llvm/include/llvm/Support/SpecialCaseList.h

Removed: 




diff  --git a/clang/docs/SanitizerSpecialCaseList.rst 
b/clang/docs/SanitizerSpecialCaseList.rst
index cbb00fde9bb7..15e19b9c129c 100644
--- a/clang/docs/SanitizerSpecialCaseList.rst
+++ b/clang/docs/SanitizerSpecialCaseList.rst
@@ -75,6 +75,9 @@ tool-specific docs.
 # Turn off checks for the source file (use absolute path or path relative
 # to the current working directory):
 src:/path/to/source/file.c
+# Turn off checks for this main file, including files included by it.
+# Useful when the main file instead of an included file should be ignored.
+mainfile:file.c
 # Turn off checks for a particular functions (use mangled names):
 fun:MyFooBar
 fun:_Z8MyFooBarv
@@ -93,3 +96,18 @@ tool-specific docs.
 [cfi-vcall|cfi-icall]
 fun:*BadCfiCall
 # Entries without sections are placed into [*] and apply to all sanitizers
+
+``mainfile`` is similar to applying ``-fno-sanitize=`` to a set of files but
+does not need plumbing into the build system. This works well for internal
+linkage functions but has a caveat for C++ vague linkage functions.
+
+C++ vague linkage functions (e.g. inline functions, template instantiations) 
are
+deduplicated at link time. A function (in an included file) ignored by a
+specific ``mainfile`` pattern may not be the prevailing copy picked by the
+linker. Therefore, using ``mainfile`` requires caution. It may still be useful,
+e.g. when patterns are picked in a way to ensure the prevailing one is ignored.
+(There is action-at-a-distance risk.)
+
+``mainfile`` can be useful enabling a ubsan check for a large code base when
+finding the direct stack frame triggering the failure for every failure is
+
diff icult.

diff  --git a/clang/include/clang/Basic/NoSanitizeList.h 
b/clang/include/clang/Basic/NoSanitizeList.h
index 3f80e0fdedda..43415859fcd5 100644
--- a/clang/include/clang/Basic/NoSanitizeList.h
+++ b/clang/include/clang/Basic/NoSanitizeList.h
@@ -41,6 +41,8 @@ class NoSanitizeList {
   bool containsFunction(SanitizerMask Mask, StringRef FunctionName) const;
   bool containsFile(SanitizerMask Mask, StringRef FileName,
 StringRef Category = StringRef()) const;
+  bool containsMainFile(SanitizerMask Mask, StringRef FileName,
+StringRef Category = StringRef()) const;
   bool containsLocation(SanitizerMask Mask, SourceLocation Loc,
 StringRef Category = StringRef()) const;
 };

diff  --git a/clang/lib/Basic/NoSanitizeList.cpp 
b/clang/lib/Basic/NoSanitizeList.cpp
index 3efd613b0d33..e7e63c1f419e 100644
--- a/clang/lib/Basic/NoSanitizeList.cpp
+++ b/clang/lib/Basic/NoSanitizeList.cpp
@@ -47,6 +47,11 @@ bool NoSanitizeList::containsFile(SanitizerMask Mask, 
StringRef FileName,
   return SSCL->inSection(Mask, "src", FileName, Category);
 }
 
+bool NoSanitizeList::containsMainFile(SanitizerMask Mask, StringRef FileName,
+  StringRef Category) const {
+  return SSCL->inSection(Mask, "mainfile", FileName, Category);
+}
+
 bool NoSanitizeList::containsLocation(SanitizerMask Mask, SourceLocation Loc,
   StringRef Category) const {
   return Loc.isValid() &&

diff  --git a/clang/lib/CodeGen/CodeGenModule.cpp 
b/clang/lib/CodeGen/CodeGenModule.cpp
index 1e90b0914538..4daf4120e0ca 10

[PATCH] D129832: [sanitizer] Add "mainfile" prefix to sanitizer special case list

2022-07-15 Thread Fangrui Song via 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 rG0d5a62faca59: [sanitizer] Add "mainfile" prefix to 
sanitizer special case list (authored by MaskRay).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129832

Files:
  clang/docs/SanitizerSpecialCaseList.rst
  clang/include/clang/Basic/NoSanitizeList.h
  clang/lib/Basic/NoSanitizeList.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGen/sanitize-ignorelist-mainfile.c
  llvm/include/llvm/Support/SpecialCaseList.h

Index: llvm/include/llvm/Support/SpecialCaseList.h
===
--- llvm/include/llvm/Support/SpecialCaseList.h
+++ llvm/include/llvm/Support/SpecialCaseList.h
@@ -19,9 +19,9 @@
 //   prefix:wildcard_expression[=category]
 // If category is not specified, it is assumed to be empty string.
 // Definitions of "prefix" and "category" are sanitizer-specific. For example,
-// sanitizer exclusion support prefixes "src", "fun" and "global".
-// Wildcard expressions define, respectively, source files, functions or
-// globals which shouldn't be instrumented.
+// sanitizer exclusion support prefixes "src", "mainfile", "fun" and "global".
+// Wildcard expressions define, respectively, source files, main files,
+// functions or globals which shouldn't be instrumented.
 // Examples of categories:
 //   "functional": used in DFSan to list functions with pure functional
 // semantics.
@@ -37,6 +37,7 @@
 // type:*Namespace::ClassName*=init
 // src:file_with_tricky_code.cc
 // src:ignore-global-initializers-issues.cc=init
+// mainfile:main_file.cc
 //
 // [dataflow]
 // # Functions with pure functional semantics:
Index: clang/test/CodeGen/sanitize-ignorelist-mainfile.c
===
--- /dev/null
+++ clang/test/CodeGen/sanitize-ignorelist-mainfile.c
@@ -0,0 +1,41 @@
+/// Test mainfile in a sanitizer special case list.
+// RUN: rm -rf %t && split-file %s %t && cd %t
+// RUN: %clang_cc1 -emit-llvm -triple x86_64 -fsanitize=address,alignment a.c -o - | FileCheck %s --check-prefixes=CHECK,DEFAULT
+// RUN: %clang_cc1 -emit-llvm -triple x86_64 -fsanitize=address,alignment -fsanitize-ignorelist=a.list a.c -o - | FileCheck %s --check-prefixes=CHECK,IGNORE
+// RUN: %clang_cc1 -emit-llvm -triple x86_64 -fsanitize=address,alignment -fsanitize-ignorelist=b.list a.c -o - | FileCheck %s --check-prefixes=CHECK,IGNORE
+
+//--- a.list
+mainfile:*a.c
+
+//--- b.list
+[address]
+mainfile:*a.c
+
+[alignment]
+mainfile:*.c
+
+//--- a.h
+int global_h;
+
+static inline int load(int *x) {
+  return *x;
+}
+
+//--- a.c
+#include "a.h"
+
+int global_c;
+
+int foo(void *x) {
+  return load(x);
+}
+
+// DEFAULT: @___asan_gen_{{.*}} = {{.*}} c"global_h\00"
+// DEFAULT: @___asan_gen_{{.*}} = {{.*}} c"global_c\00"
+// IGNORE-NOT:  @___asan_gen_
+
+// CHECK-LABEL: define {{.*}}@load(
+// DEFAULT:   call void @__ubsan_handle_type_mismatch_v1_abort(
+// DEFAULT:   call void @__asan_report_load4(
+// IGNORE-NOT:call void @__ubsan_handle_type_mismatch_v1_abort(
+// IGNORE-NOT:call void @__asan_report_load4(
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -2780,16 +2780,18 @@
   // NoSanitize by function name.
   if (NoSanitizeL.containsFunction(Kind, Fn->getName()))
 return true;
-  // NoSanitize by location.
+  // NoSanitize by location. Check "mainfile" prefix.
+  auto &SM = Context.getSourceManager();
+  const FileEntry &MainFile = *SM.getFileEntryForID(SM.getMainFileID());
+  if (NoSanitizeL.containsMainFile(Kind, MainFile.getName()))
+return true;
+
+  // Check "src" prefix.
   if (Loc.isValid())
 return NoSanitizeL.containsLocation(Kind, Loc);
   // If location is unknown, this may be a compiler-generated function. Assume
   // it's located in the main file.
-  auto &SM = Context.getSourceManager();
-  if (const auto *MainFile = SM.getFileEntryForID(SM.getMainFileID())) {
-return NoSanitizeL.containsFile(Kind, MainFile->getName());
-  }
-  return false;
+  return NoSanitizeL.containsFile(Kind, MainFile.getName());
 }
 
 bool CodeGenModule::isInNoSanitizeList(SanitizerMask Kind,
@@ -2799,8 +2801,13 @@
   const auto &NoSanitizeL = getContext().getNoSanitizeList();
   if (NoSanitizeL.containsGlobal(Kind, GV->getName(), Category))
 return true;
+  auto &SM = Context.getSourceManager();
+  if (NoSanitizeL.containsMainFile(
+  Kind, SM.getFileEntryForID(SM.getMainFileID())->getName(), Category))
+return true;
   if (NoSanitizeL.containsLocation(Kind, Loc, Category))
 return true;
+
   // Check global type.
   if (!Ty.isNull()) {
 // Drill down the array types: if global variable o

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

2022-07-15 Thread Roy Jacobson via Phabricator via cfe-commits
royjacobson marked an inline comment as done.
royjacobson added inline comments.



Comment at: clang/test/AST/conditionally-trivial-smfs.cpp:39
+
+template struct DefaultConstructorCheck<2>;
+// CHECK: "kind": "ClassTemplateSpecializationDecl",

BRevzin wrote:
> royjacobson wrote:
> > BRevzin wrote:
> > > It's possible that I just don't understand what these tests actually mean 
> > > but... where is the test for `DefaultConstructorCheck<2>` having a 
> > > deleted default constructor, or `DefaultConstructorCheck<3>` having a 
> > > non-defaulted one?
> > > 
> > > It'd also be worthwhile to have at least one test with constaints that 
> > > subsume each other instead of being mutually exclusive. 
> > Should I check this? Except destructors, the other SMFs are found during 
> > overload resolution using the usual lookup that already takes into account 
> > delete/default/constraints etc.
> > 
> > This patch is about making sure that we set the triviality attributes 
> > correctly according to the eligible functions, so this is what I added 
> > tests for.
> > 
> > Most of this testing is done in the sema test, but I need this AST test as 
> > well to make sure we get the `canPassInRegisters` attribute correctly - 
> > we're doing some custom processing over the functions without the usual 
> > type attributes because there are some weird compatibility edge cases.
> > 
> One of the motivations for the paper is to ensure that like given:
> 
> ```
> template 
> struct optional {
> optional(optional const&) requires copyable && trivially_copyableT> = 
> default;
> optional(optional const&) requires copyable;
> };
> ```
> 
> `optional` is copyable (but not trivially copyable), `optional` 
> is trivially copyable, and `optional>` isn't copyable. I'm 
> not sure what in here checks if that works. 
That's more-or-less the check in `constrained-special-member-functions.cpp:50`, 
I think.

Didn't acknowledge it in my first response - but yeah, I need some more 
complicated subsumption test cases



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128619

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


[PATCH] D129883: [HLSL] Support cbuffer/tbuffer for hlsl.

2022-07-15 Thread Xiang Li via Phabricator via cfe-commits
python3kgae created this revision.
python3kgae added reviewers: svenvh, asavonic, beanz, pow2clk, Anastasia, 
aaron.ballman.
Herald added a subscriber: mgorny.
Herald added a project: All.
python3kgae requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This is first part for support cbuffer/tbuffer.

The format for cbuffer/tbuffer is
BufferType [Name] [: register(b#)] { VariableDeclaration [: 
packoffset(c#.xyzw)]; ... };

More details at 
https://docs.microsoft.com/en-us/windows/win32/direct3dhlsl/dx-graphics-hlsl-constants

New keyword 'cbuffer' and 'tbuffer' are added.
New AST node HLSLBufferDecl is added.
Build AST for simple cbuffer/tbuffer without attribute support.

The special thing is variables declared inside cbuffer is exposed into global 
scope.
So isTransparentContext should return true for HLSLBuffer.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D129883

Files:
  clang/include/clang/AST/Decl.h
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/include/clang/AST/TextNodeDumper.h
  clang/include/clang/Basic/DeclNodes.td
  clang/include/clang/Basic/TokenKinds.def
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/Decl.cpp
  clang/lib/AST/DeclBase.cpp
  clang/lib/AST/DeclPrinter.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/Basic/IdentifierTable.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseHLSL.cpp
  clang/lib/Parse/Parser.cpp
  clang/lib/Sema/CMakeLists.txt
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaHLSL.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/lib/Serialization/ASTCommon.cpp
  clang/test/SemaHLSL/cbuffer_tbuffer.hlsl

Index: clang/test/SemaHLSL/cbuffer_tbuffer.hlsl
===
--- /dev/null
+++ clang/test/SemaHLSL/cbuffer_tbuffer.hlsl
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -x hlsl -ast-dump -o - %s | FileCheck %s
+
+// CHECK:HLSLBufferDecl 0x[[CB:[0-9a-f]+]] {{.*}} line:5:9 cbuffer CB
+// CHECK-NEXT:VarDecl 0x[[A:[0-9a-f]+]] {{.*}} col:9 used a 'float'
+cbuffer CB {
+  float a;
+};
+
+// CHECK:HLSLBufferDecl 0x[[TB:[0-9a-f]+]] {{.*}} line:11:9 tbuffer TB
+// CHECK-NEXT:VarDecl 0x[[B:[0-9a-f]+]] {{.*}} col:9 used b 'float'
+tbuffer TB {
+  float b;
+};
+
+float foo() {
+// CHECK: BinaryOperator 0x{{[0-9a-f]+}}  'float' '+'
+// CHECK-NEXT: ImplicitCastExpr 0x{{[0-9a-f]+}}  'float' 
+// CHECK-NEXT: DeclRefExpr 0x{{[0-9a-f]+}}  'float' lvalue Var 0x[[A]] 'a' 'float'
+// CHECK-NEXT: ImplicitCastExpr 0x{{[0-9a-f]+}}  'float' 
+// CHECK-NEXT: DeclRefExpr 0x{{[0-9a-f]+}}  'float' lvalue Var 0x[[B]] 'b' 'float'
+  return a + b;
+}
Index: clang/lib/Serialization/ASTCommon.cpp
===
--- clang/lib/Serialization/ASTCommon.cpp
+++ clang/lib/Serialization/ASTCommon.cpp
@@ -433,6 +433,7 @@
   case Decl::LifetimeExtendedTemporary:
   case Decl::RequiresExprBody:
   case Decl::UnresolvedUsingIfExists:
+  case Decl::HLSLBuffer:
 return false;
 
   // These indirectly derive from Redeclarable but are not actually
Index: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -873,6 +873,10 @@
   llvm_unreachable("Translation units cannot be instantiated");
 }
 
+Decl *TemplateDeclInstantiator::VisitHLSLBufferDecl(HLSLBufferDecl *Decl) {
+  llvm_unreachable("HLSL buffer declarations cannot be instantiated");
+}
+
 Decl *
 TemplateDeclInstantiator::VisitPragmaCommentDecl(PragmaCommentDecl *D) {
   llvm_unreachable("pragma comment cannot be instantiated");
Index: clang/lib/Sema/SemaHLSL.cpp
===
--- /dev/null
+++ clang/lib/Sema/SemaHLSL.cpp
@@ -0,0 +1,37 @@
+//===- SemaHLSL.cpp - Semantic Analysis for HLSL constructs ---===//
+//
+// 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 implements Semantic Analysis for HLSL constructs.
+//===--===//
+
+#include "clang/Sema/Sema.h"
+
+using namespace clang;
+
+Decl *Sema::ActOnStartHLSLBuffer(Scope *BufferScope, bool CBuffer,
+ SourceLocation KwLoc, IdentifierInfo *Ident,
+ SourceLocation IdentLoc,
+ SourceLocation LBrace) {
+  // For anonymous namespace, take the location of the left brace.
+  DeclContext *LexicalParent = getCurLexicalContext();
+  HLSLBufferDecl *Result = HLSLBufferDecl::Create(
+  Context, Lexica

[PATCH] D129884: [clang][deps] Include canonical invocation in ContextHash

2022-07-15 Thread Ben Langmuir via Phabricator via cfe-commits
benlangmuir created this revision.
benlangmuir added reviewers: jansvoboda11, Bigcheese, akyrtzi.
Herald added a project: All.
benlangmuir requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The "strict context hash" is insufficient to identify module
dependencies during scanning, leading to different module build commands
being produced for a single module, and non-deterministically choosing
between them. This commit switches to hashing the canonicalized
`CompilerInvocation` of the module. By hashing the invocation we are
converting these from correctness issues to performance issues, and we
can then incrementally improve our ability to canonicalize
command-lines.

This change can cause a regression in the number of modules needed. Of
the 4 projects I tested, 3 had no regression, but 1, which was
clang+llvm itself, had a 66% regression in number of modules (4%
regression in total invocations). This is almost entirely due to
differences between -W options across targets.  Of this, 25% of the
additional modules are system modules, which we could avoid if we
canonicalized -W options when -Wsystem-headers is not present --
unfortunately this is non-trivial due to some warnings being enabled in
system headers by default. The rest of the additional modules are mostly
real differences in potential warnings, reflecting incorrect behaviour
in the current scanner.

There were also a couple of differences due to `-DFOO`
`-fmodule-ignore-macro=FOO`, which I fixed here.

Since the output paths for the module depend on its context hash, we
hash the invocation before filling in outputs, and rely on the build
system to always return the same output paths for a given module.

Note: since the scanner itself uses an implicit modules build, there can
still be non-determinism, but it will now present as different
module+hashes rather than different command-lines for the same
module+hash.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D129884

Files:
  clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
  clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
  clang/test/ClangScanDeps/modules-context-hash-ignore-macros.c
  clang/test/ClangScanDeps/modules-context-hash-module-map-path.c
  clang/test/ClangScanDeps/modules-context-hash-warnings.c

Index: clang/test/ClangScanDeps/modules-context-hash-warnings.c
===
--- /dev/null
+++ clang/test/ClangScanDeps/modules-context-hash-warnings.c
@@ -0,0 +1,74 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: sed "s|DIR|%/t|g" %t/cdb.json.template > %t/cdb.json
+
+// RUN: clang-scan-deps -compilation-database %t/cdb.json -j 1 \
+// RUN:   -format experimental-full > %t/deps.json
+// RUN: cat %t/deps.json | sed 's:\?:/:g' | FileCheck -DPREFIX=%/t %s
+
+// CHECK:  {
+// CHECK-NEXT:   "modules": [
+// CHECK:  {
+// CHECK:"command-line": [
+// CHECK:  "-Wall"
+// CHECK:]
+// CHECK:"context-hash": "[[HASH1:.*]]"
+// CHECK:"name": "Mod"
+// CHECK-NEXT: }
+// CHECK-NEXT: {
+// CHECK:"command-line": [
+// CHECK-NOT:  "-Wall"
+// CHECK:]
+// CHECK:"context-hash": "[[HASH2:.*]]"
+// CHECK:"name": "Mod"
+// CHECK-NEXT: }
+// CHECK-NEXT:   ]
+// CHECK-NEXT:   "translation-units": [
+// CHECK-NEXT: {
+// CHECK:"clang-module-deps": [
+// CHECK-NEXT: {
+// CHECK-NEXT:   "context-hash": "[[HASH1]]"
+// CHECK-NEXT:"module-name": "Mod"
+// CHECK-NEXT: }
+// CHECK-NEXT:   ]
+// CHECK-NEXT:   "command-line": [
+// CHECK:  "-Wall"
+// CHECK:]
+// CHECK:"input-file": "{{.*}}tu1.c"
+// CHECK-NEXT: }
+// CHECK-NEXT: {
+// CHECK:"clang-module-deps": [
+// CHECK-NEXT: {
+// CHECK-NEXT:   "context-hash": "[[HASH2]]"
+// CHECK-NEXT:"module-name": "Mod"
+// CHECK-NEXT: }
+// CHECK-NEXT:   ]
+// CHECK-NEXT:   "command-line": [
+// CHECK-NOT:  "-Wall"
+// CHECK:]
+// CHECK:"input-file": "{{.*}}tu2.c"
+
+//--- cdb.json.template
+[
+  {
+"directory": "DIR",
+"command": "clang -Wall -fsyntax-only DIR/tu1.c -fmodules -fimplicit-module-maps -fmodules-cache-path=DIR/cache",
+"file": "DIR/tu1.c"
+  },
+  {
+"directory": "DIR",
+"command": "clang -fsyntax-only DIR/tu2.c -fmodules -fimplicit-module-maps -fmodules-cache-path=DIR/cache",
+"file": "DIR/tu2.c"
+  },
+]
+
+//--- module.modulemap
+module Mod { header "Mod.h" }
+
+//--- Mod.h
+
+//--- tu1.c
+#include "Mod.h"
+
+//--- tu2.c
+#include "Mod.h"
Index: clang/test/ClangScanDeps/modules-context-hash-module-map-path.c
===
--- /dev/null
+++ clang/test/ClangScanDeps/modules-context-hash-module-map-

[PATCH] D129881: [C] Strengthen -Wint-conversion to default to an error

2022-07-15 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment.

This seems like a good idea despite the breaking change.  I dont see anything 
in the code to be concerned about, but I'm hoping others will comment as to 
whether this is completely acceptable.




Comment at: clang/lib/Sema/SemaExprCXX.cpp:8482
 ExprResult Res = TransformExpr(E);
-if (Trap.hasErrorOccurred() || Res.isInvalid())
+if ((SemaRef.getLangOpts().CPlusPlus && Trap.hasErrorOccurred()) ||
+Res.isInvalid())

This makes sense to me, I don't see why this hasn't bit us before, but SFINAE 
in C mode seems wrong?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129881

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


  1   2   >