date:20221013

[PATCH] D135472: [ODRHash] Hash attributes on declarations.

2022-10-13 Thread Volodymyr Sapsai via Phabricator via cfe-commits

vsapsai updated this revision to Diff 467570.
vsapsai added a comment.

Fix the starting point of the diff.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135472

Files:
  clang/include/clang/AST/ODRDiagsEmitter.h
  clang/include/clang/AST/ODRHash.h
  clang/include/clang/Basic/DiagnosticASTKinds.td
  clang/lib/AST/ODRDiagsEmitter.cpp
  clang/lib/AST/ODRHash.cpp
  clang/test/Modules/odr_hash.cpp

Index: clang/test/Modules/odr_hash.cpp
===
--- clang/test/Modules/odr_hash.cpp
+++ clang/test/Modules/odr_hash.cpp
@@ -3637,6 +3637,228 @@
 #endif
 }  // namespace DependentSizedExtVector
 
+namespace Attributes {
+#if defined(FIRST)
+struct __attribute__((packed)) PackingPresence {
+  char x;
+  long y;
+};
+#elif defined(SECOND)
+struct PackingPresence {
+  char x;
+  long y;
+};
+#else
+PackingPresence testPackingAttributePresence;
+// expected-error@first.h:* {{'Types::Attributes::PackingPresence' has different definitions in different modules; first difference is definition in module 'FirstModule' found attribute 'packed'}}
+// expected-note@first.h:* {{attribute specified here}}
+// expected-note@second.h:* {{but in 'SecondModule' found no attribute}}
+#endif
+
+#if defined(FIRST)
+struct AlignmentPresence {
+  char x;
+  alignas(8) char y;
+};
+struct DifferentAlignmentKeywords {
+  char x;
+  char __attribute__((aligned(8))) y;
+};
+struct DifferentAlignmentValues {
+  char x;
+  alignas(4) char y;
+};
+struct AlignmentWithTypedefType {
+  char x;
+  alignas(float) char y;
+};
+#elif defined(SECOND)
+struct AlignmentPresence {
+  char x;
+  char y;
+};
+struct DifferentAlignmentKeywords {
+  char x;
+  alignas(8) char y;
+};
+struct DifferentAlignmentValues {
+  char x;
+  alignas(8) char y;
+};
+typedef float MyFloat;
+struct AlignmentWithTypedefType {
+  char x;
+  alignas(MyFloat) char y;
+};
+#else
+AlignmentPresence testAlignmentAttributePresence;
+// expected-error@first.h:* {{'Types::Attributes::AlignmentPresence' has different definitions in different modules; first difference is definition in module 'FirstModule' found 'y' with attribute 'alignas'}}
+// expected-note@first.h:* {{attribute specified here}}
+// expected-note@second.h:* {{but in 'SecondModule' found 'y' with no attribute}}
+DifferentAlignmentKeywords testDifferentAlignmentKeywords; // expected no errors as different keywords have the same meaning
+DifferentAlignmentValues testDifferentAlignments;
+// expected-error@first.h:* {{'Types::Attributes::DifferentAlignmentValues' has different definitions in different modules; first difference is definition in module 'FirstModule' found 'y' with attribute ' alignas(4)'}}
+// expected-note@first.h:* {{attribute specified here}}
+// expected-note@second.h:* {{but in 'SecondModule' found 'y' with different attribute argument ' alignas(8)'}}
+// expected-note@second.h:* {{attribute specified here}}
+AlignmentWithTypedefType testAligningAsTheSameTypeButDifferentName;
+// expected-error@first.h:* {{'Types::Attributes::AlignmentWithTypedefType' has different definitions in different modules; first difference is definition in module 'FirstModule' found 'y' with attribute ' alignas(alignof(float))'}}
+// expected-note@first.h:* {{attribute specified here}}
+// expected-note@second.h:* {{but in 'SecondModule' found 'y' with different attribute argument ' alignas(alignof(MyFloat))'}}
+// expected-note@second.h:* {{attribute specified here}}
+#endif
+
+#if defined(FIRST)
+struct AttributeOnMethod {
+  int test(int x, int y);
+};
+struct AttributeOnStaticField {
+  __attribute__((unused)) static int x;
+};
+#elif defined(SECOND)
+struct AttributeOnMethod {
+  __attribute__((optnone)) int test(int x, int y);
+};
+struct AttributeOnStaticField {
+  static int x;
+};
+#else
+AttributeOnMethod testAttributeDifferenceOnMethods;
+// expected-error@first.h:* {{'Types::Attributes::AttributeOnMethod' has different definitions in different modules; first difference is definition in module 'FirstModule' found 'test' with no attribute}}
+// expected-note@second.h:* {{but in 'SecondModule' found 'test' with attribute 'optnone'}}
+// expected-note@second.h:* {{attribute specified here}}
+AttributeOnStaticField testAttributeDifferenceOnStaticFields;
+// expected-error@first.h:* {{'Types::Attributes::AttributeOnStaticField' has different definitions in different modules; first difference is definition in module 'FirstModule' found 'x' with attribute 'unused'}}
+// expected-note@first.h:* {{attribute specified here}}
+// expected-note@second.h:* {{but in 'SecondModule' found 'x' with no attribute}}
+#endif
+
+#if defined(FIRST)
+struct __attribute__((packed)) __attribute__((aligned(8))) AttributeOrder {
+  char x;
+  long y;
+};
+#elif defined(SECOND)
+struct __attribute__((aligned(8))) __attribute__((packed)) AttributeOrder {
+  char x;
+  long y;
+};
+#else
+AttributeOrde

[PATCH] D135472: [ODRHash] Hash attributes on declarations.

2022-10-13 Thread Volodymyr Sapsai via Phabricator via cfe-commits

vsapsai marked 3 inline comments as done.
vsapsai added a comment.

Given all the discussion about which attributes should be added to ODR hash, I 
think it is useful at this point to have TableGen infrastructure to get this 
information from Attr.td. So I'll work on that.




Comment at: clang/lib/AST/ODRDiagsEmitter.cpp:159
 
 bool ODRDiagsEmitter::diagnoseSubMismatchTypedef(
 const NamedDecl *FirstRecord, StringRef FirstModule, StringRef 
SecondModule,

vsapsai wrote:
> aaron.ballman wrote:
> > Typedefs can have attributes, so should this be updated as well? 
> > (alignment, ext vector types, mode attributes, etc all come to mind)
> It should be tested for sure, thanks for pointing it out.
Typedefs are done. But while adding support for them I've realized we don't 
check method parameters. So it requires a little bit more work.



Comment at: clang/lib/AST/ODRDiagsEmitter.cpp:337-354
+if (!LHS || !RHS || LHS->getKind() != RHS->getKind()) {
+  DiagError(AttributeKind)
+  << (LHS != nullptr) << LHS << FirstDecl->getSourceRange();
+  DiagNoteAttrLoc(LHS);
+  DiagNote(AttributeKind)
+  << (RHS != nullptr) << RHS << SecondDecl->getSourceRange();
+  DiagNoteAttrLoc(RHS);

ChuanqiXu wrote:
> vsapsai wrote:
> > ChuanqiXu wrote:
> > > I feel like we can merge these two statements.
> > Sorry, I don't really get what two statements you are talking about. Is it 
> > * `!LHS || !RHS || LHS->getKind() != RHS->getKind()`
> > * `ComputeAttrODRHash(LHS) != ComputeAttrODRHash(RHS)`
> > ?
> Sorry for the ambiguity. Since `LHS->getKind() != RHS->getKind()` is covered 
> by `ComputeAttrODRHash(LHS) != ComputeAttrODRHash(RHS)`. I feel like it is 
> reasonable to:
> 
> ```
> if (!LHS || !RHS || ComputeAttrODRHash(LHS) != ComputeAttrODRHash(RHS)) {
> DiagError();
> DiagNote();
> DiagNote();
> DiagNoteAttrLoc();
> return true;
> }
> ```
There are 2 separate cases to improve the diagnostic. In the first case we'd 
have
```
... first difference is definition in module 'FirstModule' found attribute 
'enum_extensibility'
attribute specified here
but in 'SecondModule' found no attribute
```

And if we reach the second ones, it implies the kind of attributes is the same 
and the only difference is attribute arguments, so the diagnostics are like
```
first difference is definition in module 'FirstModule' found attribute ' 
__attribute__((enum_extensibility("open")))'
attribute specified here
but in 'SecondModule' found different attribute argument ' 
__attribute__((enum_extensibility("closed")))'
attribute specified here
```

From my limited experience, it is actually useful to have more details than 
trying to figure out the difference in attributes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135472

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

[PATCH] D135908: [clang][LTO] Setting Desired Default AIX Debugging Options

2022-10-13 Thread Qiongsi Wu via Phabricator via cfe-commits

qiongsiwu1 created this revision.
qiongsiwu1 added reviewers: shchenz, lkail, w2yehia, daltenty, MaskRay.
qiongsiwu1 added a project: clang.
Herald added subscribers: ormris, StephenFan, steven_wu, hiraditya, inglorion.
Herald added a project: All.
qiongsiwu1 requested review of this revision.
Herald added a subscriber: cfe-commits.

Two debug options defaults on AIX are different from other platforms:

1. The default debugger is `dbx` instead of `gdb`.
2. `strict-dwarf` defaults to `true`.

This patch implements these two default behaviours for AIX.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D135908

Files:
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/test/Driver/lto-aix.c


Index: clang/test/Driver/lto-aix.c
===
--- clang/test/Driver/lto-aix.c
+++ clang/test/Driver/lto-aix.c
@@ -4,3 +4,28 @@
 //
 // LTOPATH: "-bplugin:{{.*}}libLTO.{{so|dll|dylib}}"
 // MCPUOPTLEVEL: "-bplugin_opt:-mcpu={{.*}}" "-bplugin_opt:-O3"
+//
+// Test debugging options
+// RUN: %clang -target powerpc-ibm-aix-xcoff -### %s -flto -g 2>&1 \
+// RUN:   | FileCheck -check-prefix=STRICT %s
+// RUN: %clang -target powerpc64-ibm-aix-xcoff -### %s -flto -g 2>&1 \
+// RUN:   | FileCheck -check-prefix=STRICT %s
+// RUN: %clang -target powerpc-ibm-aix-xcoff -### %s -flto -g -gdbx 2>&1 \
+// RUN:   | FileCheck -check-prefix=DBX -check-prefix=STRICT %s
+// RUN: %clang -target powerpc-ibm-aix-xcoff -### %s -flto -g -ggdb 2>&1 \
+// RUN:   | FileCheck -check-prefix=GDB -check-prefix=STRICT %s
+// RUN: %clang -target powerpc-ibm-aix-xcoff -### %s -flto -g -ggdb0 2>&1 \
+// RUN:   | FileCheck -check-prefix=GDB -check-prefix=NOSTRICT %s
+// RUN: %clang -target powerpc-ibm-aix-xcoff -### %s -flto -g -ggdb1 2>&1 \
+// RUN:   | FileCheck -check-prefix=GDB -check-prefix=STRICT %s
+// RUN: %clang -target powerpc-ibm-aix-xcoff -### %s -flto -g -g0 2>&1 \
+// RUN:   | FileCheck -check-prefix=NOSTRICT %s
+// RUN: %clang -target powerpc-ibm-aix-xcoff -### %s -flto -g 
-gno-strict-dwarf 2>&1 \
+// RUN:   | FileCheck -check-prefix=NOSTRICT %s
+// RUN: %clang -target powerpc-ibm-aix-xcoff -### %s -flto -gstrict-dwarf 2>&1 
\
+// RUN:   | FileCheck -check-prefix=NOSTRICT %s
+//
+// DBX:"-bplugin_opt:-debugger-tune=dbx"
+// GDB:"-bplugin_opt:-debugger-tune=gdb"
+// STRICT:   "-bplugin_opt:-strict-dwarf=true"
+// NOSTRICT-NOT: "-bplugin_opt:-strict-dwarf=true"
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -584,9 +584,27 @@
 else if (A->getOption().matches(options::OPT_gdbx))
   CmdArgs.push_back(
   Args.MakeArgString(Twine(PluginOptPrefix) + "-debugger-tune=dbx"));
-else
+else {
+  // On platforms other than AIX, gdb is the default option.
+  // On AIX, dbx will be automatically pick up in the presense of the
+  // debugger tuning argument, so set gdb only if it is specified.
+  if (!IsOSAIX || A->getOption().matches(options::OPT_ggdb) ||
+  Args.getLastArg(options::OPT_ggdbN_Group))
+CmdArgs.push_back(
+Args.MakeArgString(Twine(PluginOptPrefix) + "-debugger-tune=gdb"));
+}
+  }
+
+  if (IsOSAIX) {
+// On AIX, strict-dwarf is assumed to be true if any debug option is
+// specified, unless clang is told explicitly not to assume so.
+Arg *A = Args.getLastArg(options::OPT_g_Group);
+bool EnableDebugInfo = A && !A->getOption().matches(options::OPT_g0) &&
+   !A->getOption().matches(options::OPT_ggdb0);
+if (EnableDebugInfo && Args.hasFlag(options::OPT_gstrict_dwarf,
+options::OPT_gno_strict_dwarf, true))
   CmdArgs.push_back(
-  Args.MakeArgString(Twine(PluginOptPrefix) + "-debugger-tune=gdb"));
+  Args.MakeArgString(Twine(PluginOptPrefix) + "-strict-dwarf=true"));
   }
 
   bool UseSeparateSections =


Index: clang/test/Driver/lto-aix.c
===
--- clang/test/Driver/lto-aix.c
+++ clang/test/Driver/lto-aix.c
@@ -4,3 +4,28 @@
 //
 // LTOPATH: "-bplugin:{{.*}}libLTO.{{so|dll|dylib}}"
 // MCPUOPTLEVEL: "-bplugin_opt:-mcpu={{.*}}" "-bplugin_opt:-O3"
+//
+// Test debugging options
+// RUN: %clang -target powerpc-ibm-aix-xcoff -### %s -flto -g 2>&1 \
+// RUN:   | FileCheck -check-prefix=STRICT %s
+// RUN: %clang -target powerpc64-ibm-aix-xcoff -### %s -flto -g 2>&1 \
+// RUN:   | FileCheck -check-prefix=STRICT %s
+// RUN: %clang -target powerpc-ibm-aix-xcoff -### %s -flto -g -gdbx 2>&1 \
+// RUN:   | FileCheck -check-prefix=DBX -check-prefix=STRICT %s
+// RUN: %clang -target powerpc-ibm-aix-xcoff -### %s -flto -g -ggdb 2>&1 \
+// RUN:   | FileCheck -check-prefix=GDB -check-prefix=STRICT %s
+// RUN: %clang -target powerpc-ibm-aix-xcoff -### %s -flto -g -ggdb0 2>&1 \
+// R

[clang] 4e6b071 - Fix clang version check in SARIF diagnostics test

2022-10-13 Thread Aaron Ballman via cfe-commits


Author: Yabin Cui
Date: 2022-10-13T15:16:58-04:00
New Revision: 4e6b071509d6300db024d47c8c2791da71470b11

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

LOG: Fix clang version check in SARIF diagnostics test

This is to allow future clang versions and use of LLVM_VERSION_PATCH.

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

Added: 


Modified: 
clang/test/Frontend/sarif-diagnostics.cpp

Removed: 




diff  --git a/clang/test/Frontend/sarif-diagnostics.cpp 
b/clang/test/Frontend/sarif-diagnostics.cpp
index c7512bd1777f4..9e390285b30dd 100644
--- a/clang/test/Frontend/sarif-diagnostics.cpp
+++ b/clang/test/Frontend/sarif-diagnostics.cpp
@@ -64,5 +64,5 @@ void f1(t1 x, t1 y) {
 // CHECK: 
{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"{{[0-9]+}}","name":""},{"defaultConfiguration":
 // CHECK: 
{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"{{[0-9]+}}","name":""},{"defaultConfiguration":
 // CHECK: {"enabled":true,"level":"error","rank":50},"fullDescription":
-// CHECK: 
{"text":""},"id":"{{[0-9]+}}","name":""}],"version":"16.0.0"}}}],"version":"2.1.0"}
+// CHECK: 
{"text":""},"id":"{{[0-9]+}}","name":""}],"version":"{{[0-9]+\.[0-9]+\.[0-9]+}}"}}}],"version":"2.1.0"}
 // CHECK: 2 warnings and 6 errors generated.
\ No newline at end of file



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

[PATCH] D135896: [clang] Fix clang version check in SARIF diagnostics test

2022-10-13 Thread Aaron Ballman via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG4e6b071509d6: Fix clang version check in SARIF diagnostics 
test (authored by yabinc, committed by aaron.ballman).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135896

Files:
  clang/test/Frontend/sarif-diagnostics.cpp


Index: clang/test/Frontend/sarif-diagnostics.cpp
===
--- clang/test/Frontend/sarif-diagnostics.cpp
+++ clang/test/Frontend/sarif-diagnostics.cpp
@@ -64,5 +64,5 @@
 // CHECK: 
{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"{{[0-9]+}}","name":""},{"defaultConfiguration":
 // CHECK: 
{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"{{[0-9]+}}","name":""},{"defaultConfiguration":
 // CHECK: {"enabled":true,"level":"error","rank":50},"fullDescription":
-// CHECK: 
{"text":""},"id":"{{[0-9]+}}","name":""}],"version":"16.0.0"}}}],"version":"2.1.0"}
+// CHECK: 
{"text":""},"id":"{{[0-9]+}}","name":""}],"version":"{{[0-9]+\.[0-9]+\.[0-9]+}}"}}}],"version":"2.1.0"}
 // CHECK: 2 warnings and 6 errors generated.
\ No newline at end of file


Index: clang/test/Frontend/sarif-diagnostics.cpp
===
--- clang/test/Frontend/sarif-diagnostics.cpp
+++ clang/test/Frontend/sarif-diagnostics.cpp
@@ -64,5 +64,5 @@
 // CHECK: {"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"{{[0-9]+}}","name":""},{"defaultConfiguration":
 // CHECK: {"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"{{[0-9]+}}","name":""},{"defaultConfiguration":
 // CHECK: {"enabled":true,"level":"error","rank":50},"fullDescription":
-// CHECK: {"text":""},"id":"{{[0-9]+}}","name":""}],"version":"16.0.0"}}}],"version":"2.1.0"}
+// CHECK: {"text":""},"id":"{{[0-9]+}}","name":""}],"version":"{{[0-9]+\.[0-9]+\.[0-9]+}}"}}}],"version":"2.1.0"}
 // CHECK: 2 warnings and 6 errors generated.
\ No newline at end of file
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D135896: [clang] Fix clang version check in SARIF diagnostics test

2022-10-13 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added a comment.

In D135896#3856506 , @yabinc wrote:

> Thanks @aaron.ballman. I don’t have commit access, can you land this patch 
> for me? Please use “Yabin Cui yab...@google.com” to commit the change.

Happy to do so, thank you for the fix!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135896

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

[PATCH] D134928: [Sema] Don't treat a non-null template argument as if it were null.

2022-10-13 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added inline comments.



Comment at: clang/lib/Sema/SemaTemplate.cpp:6490-6491
 
   // C++11 [temp.arg.nontype]p1:
   //   - an address constant expression of type std::nullptr_t
   if (Arg->getType()->isNullPtrType())

It's worth noting that the rules we follow changed slightly: 
http://eel.is/c++draft/temp.arg.nontype#2 so if we're making repairs in this 
area, are there broader changes we need to consider? (That's fine as a FIXME 
comment if the changes end up being unrelated to this specific fix.)



Comment at: clang/test/CXX/temp/temp.arg/temp.arg.nontype/p1-11.cpp:31
 IP<&tl> ip7; // expected-error{{non-type template argument of type 'int *' is 
not a constant expression}}
+IP<(int*)1> ip8; // expected-error {{non-type template argument does not refer 
to any declaration}}
 

shafik wrote:
> rsmith wrote:
> > efriedma wrote:
> > > erichkeane wrote:
> > > > shafik wrote:
> > > > > shafik wrote:
> > > > > > It looks like in C++17 mode we catch this case: 
> > > > > > https://godbolt.org/z/s43oE5qWE
> > > > > Another case to check for:
> > > > > 
> > > > > ```
> > > > > IP<(int*)(1-1)> ip9;
> > > > > ```
> > > > > 
> > > > > In C++11 the wording use to allow `integer constant expressions`
> > > > The new diagnostic here is unfortunate.  That 'does not refer to any 
> > > > declaration' doesn't really let me know that it is illegal because that 
> > > > isn't a NPE.  
> > > > 
> > > > The 'treat it as a null ptr' here is obviously awful, but I find myself 
> > > > wondering if we can do better on this diagnostic trivially enough?
> > > It's easy enough to use a different message; we just need to detect the 
> > > particular case we're considering, print an error, and return NPV_Error.  
> > > But I'm not sure what a better diagnostic looks like.   "standard C++ 
> > > does not allow using '(int*)1' as a non-type template argument"?  (The 
> > > fact that null is allowed isn't really relevant here.)
> > > It looks like in C++17 mode we catch this case: 
> > > https://godbolt.org/z/s43oE5qWE
> > 
> > That's diagnosing the cast, not the template argument value. You can get 
> > around the cast diagnostic by forcing constant folding with a 
> > `__builtin_constant_p` conditional 
> > ([example](https://godbolt.org/z/8f8WWTGeM)). Then we diagnose as
> > > :7:4: error: non-type template argument refers to subobject '(int 
> > > *)1'
> > ... which seems like a worse diagnostic than the C++11 one, because it's 
> > not even true.
> 
> > That's diagnosing the cast, not the template argument value. You can get 
> > around the cast diagnostic by forcing constant folding with a 
> > `__builtin_constant_p` conditional 
> > ([example](https://godbolt.org/z/8f8WWTGeM)). Then we diagnose as
> > > :7:4: error: non-type template argument refers to subobject '(int 
> > > *)1'
> > ... which seems like a worse diagnostic than the C++11 one, because it's 
> > not even true.
> 
> Great point, I missed that earlier.
> 
> My point was that if we are catching this in C++17 mode maybe we can reuse 
> the machinery to catch this other modes as well and fix the diagnostic 
> quality as well. Although I realize that may not be easy but worth at least 
> understanding. 
For comparison, ICC diagnoses this with:
`error: invalid nontype template argument of type "int *"`
and GCC diagnoses it as:
`error: 'reinterpret_cast' from integer to pointer` and 
`error: '1' is not a valid template argument for 'int*' because it is not the 
address of a variable`

Either one of those diagnostics seems like an improvement over our status quo.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134928

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

[PATCH] D135472: [ODRHash] Hash attributes on declarations.

2022-10-13 Thread Volodymyr Sapsai via Phabricator via cfe-commits

vsapsai updated this revision to Diff 467564.
vsapsai added a comment.

- Small fixes to address review comments.
- Try to make diagnostics more understandable.
- Check attributes on typedefs.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135472

Files:
  clang/lib/AST/ODRDiagsEmitter.cpp
  clang/lib/AST/ODRHash.cpp
  clang/test/Modules/odr_hash.cpp


Index: clang/test/Modules/odr_hash.cpp
===
--- clang/test/Modules/odr_hash.cpp
+++ clang/test/Modules/odr_hash.cpp
@@ -3788,6 +3788,32 @@
 // expected-note@second.h:* {{attribute specified here}}
 #endif
 
+#if defined(FIRST)
+struct TypedefAttributePresence {
+  typedef double *AlignedDoublePtr __attribute__((align_value(64)));
+};
+struct TypedefDifferentAttributeValue {
+  typedef double *AlignedDoublePtr __attribute__((align_value(64)));
+};
+#elif defined(SECOND)
+struct TypedefAttributePresence {
+  typedef double *AlignedDoublePtr;
+};
+struct TypedefDifferentAttributeValue {
+  typedef double *AlignedDoublePtr __attribute__((align_value(32)));
+};
+#else
+TypedefAttributePresence testTypedefAttrPresence;
+// expected-error@first.h:* {{'Types::Attributes::TypedefAttributePresence' 
has different definitions in different modules; first difference is definition 
in module 'FirstModule' found 'AlignedDoublePtr' with attribute 'align_value'}}
+// expected-note@first.h:* {{attribute specified here}}
+// expected-note@second.h:* {{but in 'SecondModule' found 'AlignedDoublePtr' 
with no attribute}}
+TypedefDifferentAttributeValue testDifferentArgumentsInTypedefAttribute;
+// expected-error@first.h:* 
{{'Types::Attributes::TypedefDifferentAttributeValue' has different definitions 
in different modules; first difference is definition in module 'FirstModule' 
found 'AlignedDoublePtr' with attribute ' __attribute__((align_value(64)))'}}
+// expected-note@first.h:* {{attribute specified here}}
+// expected-note@second.h:* {{but in 'SecondModule' found 'AlignedDoublePtr' 
with different attribute argument ' __attribute__((align_value(32)))'}}
+// expected-note@second.h:* {{attribute specified here}}
+#endif
+
 #if defined(FIRST)
 #define PACKED __attribute__((packed))
 struct PACKED AttributeInMacro {
Index: clang/lib/AST/ODRHash.cpp
===
--- clang/lib/AST/ODRHash.cpp
+++ clang/lib/AST/ODRHash.cpp
@@ -506,6 +506,14 @@
 }
 break;
   }
+  case attr::AlignValue: {
+auto *M = cast(A);
+Expr *AlignmentExpr = M->getAlignment();
+ID.AddBoolean(AlignmentExpr);
+if (AlignmentExpr)
+  AddStmt(AlignmentExpr);
+break;
+  }
   case attr::EnumExtensibility: {
 auto *M = cast(A);
 ID.AddInteger(M->getExtensibility());
Index: clang/lib/AST/ODRDiagsEmitter.cpp
===
--- clang/lib/AST/ODRDiagsEmitter.cpp
+++ clang/lib/AST/ODRDiagsEmitter.cpp
@@ -193,6 +193,10 @@
 DiagNote(TypedefType) << IsTypeAlias << SecondName << SecondType;
 return true;
   }
+
+  if (diagnoseSubMismatchAttr(FirstRecord, FirstModule, SecondModule, FirstTD,
+  SecondTD))
+return true;
   return false;
 }
 


Index: clang/test/Modules/odr_hash.cpp
===
--- clang/test/Modules/odr_hash.cpp
+++ clang/test/Modules/odr_hash.cpp
@@ -3788,6 +3788,32 @@
 // expected-note@second.h:* {{attribute specified here}}
 #endif
 
+#if defined(FIRST)
+struct TypedefAttributePresence {
+  typedef double *AlignedDoublePtr __attribute__((align_value(64)));
+};
+struct TypedefDifferentAttributeValue {
+  typedef double *AlignedDoublePtr __attribute__((align_value(64)));
+};
+#elif defined(SECOND)
+struct TypedefAttributePresence {
+  typedef double *AlignedDoublePtr;
+};
+struct TypedefDifferentAttributeValue {
+  typedef double *AlignedDoublePtr __attribute__((align_value(32)));
+};
+#else
+TypedefAttributePresence testTypedefAttrPresence;
+// expected-error@first.h:* {{'Types::Attributes::TypedefAttributePresence' has different definitions in different modules; first difference is definition in module 'FirstModule' found 'AlignedDoublePtr' with attribute 'align_value'}}
+// expected-note@first.h:* {{attribute specified here}}
+// expected-note@second.h:* {{but in 'SecondModule' found 'AlignedDoublePtr' with no attribute}}
+TypedefDifferentAttributeValue testDifferentArgumentsInTypedefAttribute;
+// expected-error@first.h:* {{'Types::Attributes::TypedefDifferentAttributeValue' has different definitions in different modules; first difference is definition in module 'FirstModule' found 'AlignedDoublePtr' with attribute ' __attribute__((align_value(64)))'}}
+// expected-note@first.h:* {{attribute specified here}}
+// expected-note@second.h:* {{but in 'SecondModule' found 'AlignedDoublePtr' with different attribute argument ' _

[PATCH] D135557: Add needsImplicitDefaultConstructor and friends

2022-10-13 Thread Roy Jacobson via Phabricator via cfe-commits

royjacobson added inline comments.



Comment at: clang/bindings/python/clang/cindex.py:1530
+
+def record_needs_implicit_default_constructor(self):
+"""Returns True if the cursor refers to a C++ record declaration

anderslanglands wrote:
> royjacobson wrote:
> > aaron.ballman wrote:
> > > aaron.ballman wrote:
> > > > I don't think we should expose any of the "needs" functions like this 
> > > > -- those are internal implementation details of the class and I don't 
> > > > think we want to calcify that into something we have to support 
> > > > forever. As we add members to a class, we recalculate whether the added 
> > > > member causes us to delete defaulted special members (among other 
> > > > things), and the "needs" functions are basically used when the class is 
> > > > completed to handle lazily created special members. I'm pretty sure 
> > > > that lazy creation is not mandated by the standard, which is why I 
> > > > think the "needs" functions are more of an implementation detail.
> > > CC @erichkeane and @royjacobson as folks who have been in this same area 
> > > of the compiler to see if they agree or disagree with my assessment there.
> > I think so. The 'needs_*' functions query `DeclaredSpecialMembers` and I'm 
> > pretty sure it's modified when we add the implicit definitions in the class 
> > completion code. So this looks a bit suspicious. Is this API //meant// to 
> > be used with incomplete classes?
> > For complete classes I think looking up the default/move/copy constructor 
> > and calling `isImplicit()` is the way to do it.
> > 
> > About the 'is deleted' API - can't the same be done for those functions as 
> > well so we have a smaller API? 
> > 
> > If this //is// meant to be used with incomplete classes for efficiency that 
> > would be another thing, I guess.
> > 
> So the intended use case here is I'm using libclang to parse an existing C++ 
> libray's headers and generate a C interface to it. To do that I need to know 
> if I need to generate default constructors etc, which the needs* methods do 
> for me (I believe). The alternative is I have to check manually whether all 
> the constructors/assignment operators exist, then implement the implicit 
> declaration rules myself correctly for each version of the standard, which 
> I'd rather avoid.
> 
> Would putting a note in the doc comment about the behaviour differing when 
> the class is being constructed as originally suggested work for everyone?
Why is the `__is_default_constructible` builtin type trait not enough? Do you 
have different behavior for user provided and implicit default constructors?



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135557

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

[clang] 8c7b346 - [clang] Update ASM goto documentation to reflect how Clang differs from GCC

2022-10-13 Thread Bill Wendling via cfe-commits


Author: Bill Wendling
Date: 2022-10-13T12:05:40-07:00
New Revision: 8c7b3461a5346a66088d884500c2b3c2829b4652

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

LOG: [clang] Update ASM goto documentation to reflect how Clang differs from GCC

That said, we are planning to add this support in the near future.

Link: https://github.com/llvm/llvm-project/issues/53562

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

Added: 


Modified: 
clang/docs/LanguageExtensions.rst

Removed: 




diff  --git a/clang/docs/LanguageExtensions.rst 
b/clang/docs/LanguageExtensions.rst
index b449809c88d5..fca4227a5812 100644
--- a/clang/docs/LanguageExtensions.rst
+++ b/clang/docs/LanguageExtensions.rst
@@ -1546,28 +1546,25 @@ Query for this feature with ``__has_extension(blocks)``.
 ASM Goto with Output Constraints
 
 
-In addition to the functionality provided by `GCC's extended
-assembly `_, clang
-supports output constraints with the `goto` form.
+.. note::
 
-The goto form of GCC's extended assembly allows the programmer to branch to a C
-label from within an inline assembly block. Clang extends this behavior by
-allowing the programmer to use output constraints:
+  Clang's implementation of ASM goto 
diff ers from `GCC's
+  implementation`_ in
+  that Clang doesn't yet support outputs on the indirect branch. Use of an
+  output on the indirect branch may result in undefined behavior and should be
+  avoided. E.g., in the following `z` isn't valid when used and may have
+  
diff erent values depending on optimization levels. (Clang may not warn about
+  such constructs.)
 
-.. code-block:: c++
+  .. code-block:: c++
 
-  int foo(int x) {
-  int y;
-  asm goto("# %0 %1 %l2" : "=r"(y) : "r"(x) : : err);
+int foo(int x) {
+  int y, z;
+  asm goto(... : "=r"(y), "=r"(z): "r"(x) : : err);
   return y;
 err:
-  return -1;
-  }
-
-It's important to note that outputs are valid only on the "fallthrough" branch.
-Using outputs on an indirect branch may result in undefined behavior. For
-example, in the function above, use of the value assigned to `y` in the `err`
-block is undefined behavior.
+  return z;
+}
 
 When using tied-outputs (i.e. outputs that are inputs and outputs, not just
 outputs) with the `+r` constraint, there is a hidden input that's created



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

[PATCH] D135818: [clang] Update ASM goto documentation to reflect how Clang differs from GCC

2022-10-13 Thread Bill Wendling via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
void marked an inline comment as done.
Closed by commit rG8c7b3461a534: [clang] Update ASM goto documentation to 
reflect how Clang differs from GCC (authored by void).

Changed prior to commit:
  https://reviews.llvm.org/D135818?vs=467284&id=467560#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135818

Files:
  clang/docs/LanguageExtensions.rst


Index: clang/docs/LanguageExtensions.rst
===
--- clang/docs/LanguageExtensions.rst
+++ clang/docs/LanguageExtensions.rst
@@ -1546,28 +1546,25 @@
 ASM Goto with Output Constraints
 
 
-In addition to the functionality provided by `GCC's extended
-assembly `_, clang
-supports output constraints with the `goto` form.
+.. note::
 
-The goto form of GCC's extended assembly allows the programmer to branch to a C
-label from within an inline assembly block. Clang extends this behavior by
-allowing the programmer to use output constraints:
+  Clang's implementation of ASM goto differs from `GCC's
+  implementation`_ in
+  that Clang doesn't yet support outputs on the indirect branch. Use of an
+  output on the indirect branch may result in undefined behavior and should be
+  avoided. E.g., in the following `z` isn't valid when used and may have
+  different values depending on optimization levels. (Clang may not warn about
+  such constructs.)
 
-.. code-block:: c++
+  .. code-block:: c++
 
-  int foo(int x) {
-  int y;
-  asm goto("# %0 %1 %l2" : "=r"(y) : "r"(x) : : err);
+int foo(int x) {
+  int y, z;
+  asm goto(... : "=r"(y), "=r"(z): "r"(x) : : err);
   return y;
 err:
-  return -1;
-  }
-
-It's important to note that outputs are valid only on the "fallthrough" branch.
-Using outputs on an indirect branch may result in undefined behavior. For
-example, in the function above, use of the value assigned to `y` in the `err`
-block is undefined behavior.
+  return z;
+}
 
 When using tied-outputs (i.e. outputs that are inputs and outputs, not just
 outputs) with the `+r` constraint, there is a hidden input that's created


Index: clang/docs/LanguageExtensions.rst
===
--- clang/docs/LanguageExtensions.rst
+++ clang/docs/LanguageExtensions.rst
@@ -1546,28 +1546,25 @@
 ASM Goto with Output Constraints
 
 
-In addition to the functionality provided by `GCC's extended
-assembly `_, clang
-supports output constraints with the `goto` form.
+.. note::
 
-The goto form of GCC's extended assembly allows the programmer to branch to a C
-label from within an inline assembly block. Clang extends this behavior by
-allowing the programmer to use output constraints:
+  Clang's implementation of ASM goto differs from `GCC's
+  implementation`_ in
+  that Clang doesn't yet support outputs on the indirect branch. Use of an
+  output on the indirect branch may result in undefined behavior and should be
+  avoided. E.g., in the following `z` isn't valid when used and may have
+  different values depending on optimization levels. (Clang may not warn about
+  such constructs.)
 
-.. code-block:: c++
+  .. code-block:: c++
 
-  int foo(int x) {
-  int y;
-  asm goto("# %0 %1 %l2" : "=r"(y) : "r"(x) : : err);
+int foo(int x) {
+  int y, z;
+  asm goto(... : "=r"(y), "=r"(z): "r"(x) : : err);
   return y;
 err:
-  return -1;
-  }
-
-It's important to note that outputs are valid only on the "fallthrough" branch.
-Using outputs on an indirect branch may result in undefined behavior. For
-example, in the function above, use of the value assigned to `y` in the `err`
-block is undefined behavior.
+  return z;
+}
 
 When using tied-outputs (i.e. outputs that are inputs and outputs, not just
 outputs) with the `+r` constraint, there is a hidden input that's created
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D135818: [clang] Update ASM goto documentation to reflect how Clang differs from GCC

2022-10-13 Thread Bill Wendling via Phabricator via cfe-commits

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



Comment at: clang/docs/LanguageExtensions.rst:1553
+  implementation`_ in
+  that Clang doesn't support outputs on the indirect branch. Use of an output
+  on the indirect branch may result in undefined behavior and should be

nickdesaulniers wrote:
> Consider adding a link to https://github.com/llvm/llvm-project/issues/53562?
> 
> Clang doesn't yet support... but support is planned.
> 
> or something.
I'll add the link to the commit message. I don't know if it's very useful here, 
as it pertains mostly to compiler developers rather than end users.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135818

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

[PATCH] D135818: [clang] Update ASM goto documentation to reflect how Clang differs from GCC

2022-10-13 Thread Shafik Yaghmour via Phabricator via cfe-commits

shafik added inline comments.



Comment at: clang/docs/LanguageExtensions.rst:1555
+  on the indirect branch may result in undefined behavior and should be
+  avoided.
 

void wrote:
> shafik wrote:
> > It would be helpful to provide an example of what is not supported. If you 
> > can provide an example that actually results in undefined behavior that 
> > would be even better.
> I added an example, but it's hard to determine whether a given piece of code 
> will have undefined behavior as it sometimes occurs only on a specific 
> optimization level.
Understood, I think what you have is good. Thank you.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135818

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

[PATCH] D135896: [clang] Fix clang version check in SARIF diagnostics test

2022-10-13 Thread Yabin Cui via Phabricator via cfe-commits

yabinc added a comment.

Thanks @aaron.ballman. I don’t have commit access, can you land this patch for 
me? Please use “Yabin Cui yab...@google.com” to commit the change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135896

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

[PATCH] D135557: Add needsImplicitDefaultConstructor and friends

2022-10-13 Thread Anders Langlands via Phabricator via cfe-commits

anderslanglands added inline comments.



Comment at: clang/bindings/python/clang/cindex.py:1530
+
+def record_needs_implicit_default_constructor(self):
+"""Returns True if the cursor refers to a C++ record declaration

royjacobson wrote:
> aaron.ballman wrote:
> > aaron.ballman wrote:
> > > I don't think we should expose any of the "needs" functions like this -- 
> > > those are internal implementation details of the class and I don't think 
> > > we want to calcify that into something we have to support forever. As we 
> > > add members to a class, we recalculate whether the added member causes us 
> > > to delete defaulted special members (among other things), and the "needs" 
> > > functions are basically used when the class is completed to handle lazily 
> > > created special members. I'm pretty sure that lazy creation is not 
> > > mandated by the standard, which is why I think the "needs" functions are 
> > > more of an implementation detail.
> > CC @erichkeane and @royjacobson as folks who have been in this same area of 
> > the compiler to see if they agree or disagree with my assessment there.
> I think so. The 'needs_*' functions query `DeclaredSpecialMembers` and I'm 
> pretty sure it's modified when we add the implicit definitions in the class 
> completion code. So this looks a bit suspicious. Is this API //meant// to be 
> used with incomplete classes?
> For complete classes I think looking up the default/move/copy constructor and 
> calling `isImplicit()` is the way to do it.
> 
> About the 'is deleted' API - can't the same be done for those functions as 
> well so we have a smaller API? 
> 
> If this //is// meant to be used with incomplete classes for efficiency that 
> would be another thing, I guess.
> 
So the intended use case here is I'm using libclang to parse an existing C++ 
libray's headers and generate a C interface to it. To do that I need to know if 
I need to generate default constructors etc, which the needs* methods do for me 
(I believe). The alternative is I have to check manually whether all the 
constructors/assignment operators exist, then implement the implicit 
declaration rules myself correctly for each version of the standard, which I'd 
rather avoid.

Would putting a note in the doc comment about the behaviour differing when the 
class is being constructed as originally suggested work for everyone?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135557

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

[PATCH] D134507: [Clang] add missing ClangABICompat check

2022-10-13 Thread Yuanfang Chen via Phabricator via cfe-commits

ychen abandoned this revision.
ychen added a comment.

Per further discussions in D128745 , this is 
not needed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134507

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

[PATCH] D134637: clang-tblgen build: avoid duplicate inclusion of libLLVMSupport

2022-10-13 Thread Nicolai Hähnle via Phabricator via cfe-commits

nhaehnle updated this revision to Diff 467550.
nhaehnle added a comment.

- use the object library version of clangSupport if available

Thank you for the pointers. Using this object library makes a lot of
sense to me and it does appear to work. Judging by AddClang.cmake though,
the object library is only available if (NOT XCODE), so I kept the
previous way of doing things as an alternative. In all cases, I'm using an
alias so that the complexity is contained within the clangSupport CMakeLists.txt


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134637

Files:
  clang/lib/Support/CMakeLists.txt
  clang/utils/TableGen/CMakeLists.txt


Index: clang/utils/TableGen/CMakeLists.txt
===
--- clang/utils/TableGen/CMakeLists.txt
+++ clang/utils/TableGen/CMakeLists.txt
@@ -25,6 +25,6 @@
   TableGen.cpp
   )
 
-target_link_libraries(clang-tblgen PRIVATE clangSupport)
+target_link_libraries(clang-tblgen PRIVATE clangSupport_tablegen)
 
 set_target_properties(clang-tblgen PROPERTIES FOLDER "Clang tablegenning")
Index: clang/lib/Support/CMakeLists.txt
===
--- clang/lib/Support/CMakeLists.txt
+++ clang/lib/Support/CMakeLists.txt
@@ -9,8 +9,24 @@
   Support
   )
 
-add_clang_library(clangSupport
+set(clangSupport_sources
   RISCVVIntrinsicUtils.cpp
   )
 
+add_clang_library(clangSupport ${clangSupport_sources})
+
+if (NOT XCODE)
+  add_library(clangSupport_tablegen ALIAS obj.clangSupport)
+elseif (NOT LLVM_LINK_LLVM_DYLIB)
+  add_library(clangSupport_tablegen ALIAS clangSupport)
+else()
+  # Build a version of the support library that does not link against
+  # libLLVM-*.so, to be used by clang-tblgen. This is so clang-tblgen doesn't
+  # link against libLLVMSupport twice (once statically and once via
+  # libLLVM-*.so).
+  add_llvm_library(clangSupport_tablegen
+BUILDTREE_ONLY STATIC DISABLE_LLVM_LINK_LLVM_DYLIB
+${clangSupport_sources})
+endif()
+
 set(LLVM_COMMON_DEPENDS ${LLVM_COMMON_DEPENDS_OLD})


Index: clang/utils/TableGen/CMakeLists.txt
===
--- clang/utils/TableGen/CMakeLists.txt
+++ clang/utils/TableGen/CMakeLists.txt
@@ -25,6 +25,6 @@
   TableGen.cpp
   )
 
-target_link_libraries(clang-tblgen PRIVATE clangSupport)
+target_link_libraries(clang-tblgen PRIVATE clangSupport_tablegen)
 
 set_target_properties(clang-tblgen PROPERTIES FOLDER "Clang tablegenning")
Index: clang/lib/Support/CMakeLists.txt
===
--- clang/lib/Support/CMakeLists.txt
+++ clang/lib/Support/CMakeLists.txt
@@ -9,8 +9,24 @@
   Support
   )
 
-add_clang_library(clangSupport
+set(clangSupport_sources
   RISCVVIntrinsicUtils.cpp
   )
 
+add_clang_library(clangSupport ${clangSupport_sources})
+
+if (NOT XCODE)
+  add_library(clangSupport_tablegen ALIAS obj.clangSupport)
+elseif (NOT LLVM_LINK_LLVM_DYLIB)
+  add_library(clangSupport_tablegen ALIAS clangSupport)
+else()
+  # Build a version of the support library that does not link against
+  # libLLVM-*.so, to be used by clang-tblgen. This is so clang-tblgen doesn't
+  # link against libLLVMSupport twice (once statically and once via
+  # libLLVM-*.so).
+  add_llvm_library(clangSupport_tablegen
+BUILDTREE_ONLY STATIC DISABLE_LLVM_LINK_LLVM_DYLIB
+${clangSupport_sources})
+endif()
+
 set(LLVM_COMMON_DEPENDS ${LLVM_COMMON_DEPENDS_OLD})
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D134699: [clang][Interp] Implement This pointer passing to methods

2022-10-13 Thread Aaron Ballman via Phabricator via cfe-commits

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

LGTM




Comment at: clang/lib/AST/Interp/EvalEmitter.cpp:108
 
-  S.Current =
-  new InterpFrame(S, const_cast(Func), S.Current, {}, {});
+  S.Current = new InterpFrame(S, const_cast(Func), {});
   // Result of call will be on the stack and needs to be handled by the caller.

Not related to this review: it'd be nice to fix the interface so we get const 
correct behavior rather than needing to use `const_cast` like this.


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

https://reviews.llvm.org/D134699

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

[PATCH] D135896: [clang] Fix clang version check in SARIF diagnostics test

2022-10-13 Thread Aaron Ballman via Phabricator via cfe-commits

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

LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135896

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

[PATCH] D135025: [clang][Interp] Support base class constructors

2022-10-13 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added inline comments.



Comment at: clang/lib/AST/Interp/ByteCodeExprGen.h:258
+  /// or nullptr if no such decl exists.
+  const CXXRecordDecl * getRecordDecl(const Expr *E) const {
+QualType T = E->getType();





Comment at: clang/test/AST/Interp/records.cpp:209
+  static_assert(d.getA() == 20);
+  static_assert(d.getB() == 30);
+};

I'd appreciate some more failure test cases, if they're not already covered 
elsewhere:
```
struct Base {
  int Val;
};

struct Derived : Base {
  int OtherVal;

  constexpr Derived(int i) : OtherVal(i) {}
};

// Something here should be diagnosed; either because the Derived ctor is not a
// valid constexpr function or because we're accessing an uninitialized member.
constexpr Derived D(12);
static_assert(D.Val == 0);


// Another test is when a constexpr ctor calls a non-constexpr base class ctor.
struct AnotherBase {
  int Val;
  constexpr AnotherBase(int i) : Val(12 / i) {}
};

struct AnotherDerived : AnotherBase {
  constexpr AnotherDerived(int i) : AnotherBase(i) {}
};
constexpr AnotherDerived Derp(0);

// Skipping the derived class constructor is also
// interesting to consider:
struct YetAnotherBase {
  int Val;
  constexpr YetAnotherBase(int i) : Val(i) {}
};

struct YetAnotherDerived : YetAnotherBase {
  using YetAnotherBase::YetAnotherBase;
  
  int OtherVal;

  constexpr bool doit() const { return Val == OtherVal; }
};

constexpr YetAnotherDerived Oops(0);
```


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

https://reviews.llvm.org/D135025

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

[PATCH] D107290: [RISCV] Add support for the vscale_range attribute

2022-10-13 Thread Philip Reames via Phabricator via cfe-commits

reames added a comment.
Herald added subscribers: sunshaoce, StephenFan, arichardson.
Herald added a project: All.

Not knowing about this patch, I posted D135894 
 which addresses a small sub-set of this.  If 
that goes in, I plan to iterative split off some other parts of this into stand 
alone changes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107290

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

[PATCH] D135894: [clang][RISCV] Set vscale_range attribute based on presence of "v" extension

2022-10-13 Thread Philip Reames via Phabricator via cfe-commits

reames added a comment.

In D135894#3856229 , @craig.topper 
wrote:

> There's also this patch from @frasercrmck https://reviews.llvm.org/D107290

So there is; had not seen that.  Looking at it, that patch seems to roll 
several items together.  I'd strongly prefer if we worked incrementally here.  
I think we do need to merge the frontend and backend handling here, but we can 
we work one step at a time?

Steps as I currently see them:
Step 1 - get basic support for "v", to unblock practical codegen 
Step 2 - expose the necessary constants for frontend use, and rephrase in terms 
of VLEN
Step 3 - figure out why getMinVLen on RISCVISAInfo is currently always 
returning 0, fix that
Step 4 - revisit question if whether the result replaces TTI callback (I don't 
think it does, but don't want to speculate too much now.)

I'm willing to commit my time through at least step 3, and possibly beyond.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135894

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

[PATCH] D135557: Add needsImplicitDefaultConstructor and friends

2022-10-13 Thread Roy Jacobson via Phabricator via cfe-commits

royjacobson added inline comments.



Comment at: clang/bindings/python/clang/cindex.py:1530
+
+def record_needs_implicit_default_constructor(self):
+"""Returns True if the cursor refers to a C++ record declaration

aaron.ballman wrote:
> aaron.ballman wrote:
> > I don't think we should expose any of the "needs" functions like this -- 
> > those are internal implementation details of the class and I don't think we 
> > want to calcify that into something we have to support forever. As we add 
> > members to a class, we recalculate whether the added member causes us to 
> > delete defaulted special members (among other things), and the "needs" 
> > functions are basically used when the class is completed to handle lazily 
> > created special members. I'm pretty sure that lazy creation is not mandated 
> > by the standard, which is why I think the "needs" functions are more of an 
> > implementation detail.
> CC @erichkeane and @royjacobson as folks who have been in this same area of 
> the compiler to see if they agree or disagree with my assessment there.
I think so. The 'needs_*' functions query `DeclaredSpecialMembers` and I'm 
pretty sure it's modified when we add the implicit definitions in the class 
completion code. So this looks a bit suspicious. Is this API //meant// to be 
used with incomplete classes?
For complete classes I think looking up the default/move/copy constructor and 
calling `isImplicit()` is the way to do it.

About the 'is deleted' API - can't the same be done for those functions as well 
so we have a smaller API? 

If this //is// meant to be used with incomplete classes for efficiency that 
would be another thing, I guess.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135557

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

[PATCH] D135896: [clang] Fix clang version check in SARIF diagnostics test

2022-10-13 Thread Yabin Cui via Phabricator via cfe-commits

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

This is to allow future clang versions and use of LLVM_VERSION_PATCH.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D135896

Files:
  clang/test/Frontend/sarif-diagnostics.cpp


Index: clang/test/Frontend/sarif-diagnostics.cpp
===
--- clang/test/Frontend/sarif-diagnostics.cpp
+++ clang/test/Frontend/sarif-diagnostics.cpp
@@ -64,5 +64,5 @@
 // CHECK: 
{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"{{[0-9]+}}","name":""},{"defaultConfiguration":
 // CHECK: 
{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"{{[0-9]+}}","name":""},{"defaultConfiguration":
 // CHECK: {"enabled":true,"level":"error","rank":50},"fullDescription":
-// CHECK: 
{"text":""},"id":"{{[0-9]+}}","name":""}],"version":"16.0.0"}}}],"version":"2.1.0"}
+// CHECK: 
{"text":""},"id":"{{[0-9]+}}","name":""}],"version":"{{[0-9]+\.[0-9]+\.[0-9]+}}"}}}],"version":"2.1.0"}
 // CHECK: 2 warnings and 6 errors generated.
\ No newline at end of file


Index: clang/test/Frontend/sarif-diagnostics.cpp
===
--- clang/test/Frontend/sarif-diagnostics.cpp
+++ clang/test/Frontend/sarif-diagnostics.cpp
@@ -64,5 +64,5 @@
 // CHECK: {"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"{{[0-9]+}}","name":""},{"defaultConfiguration":
 // CHECK: {"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"{{[0-9]+}}","name":""},{"defaultConfiguration":
 // CHECK: {"enabled":true,"level":"error","rank":50},"fullDescription":
-// CHECK: {"text":""},"id":"{{[0-9]+}}","name":""}],"version":"16.0.0"}}}],"version":"2.1.0"}
+// CHECK: {"text":""},"id":"{{[0-9]+}}","name":""}],"version":"{{[0-9]+\.[0-9]+\.[0-9]+}}"}}}],"version":"2.1.0"}
 // CHECK: 2 warnings and 6 errors generated.
\ No newline at end of file
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D135894: [clang][RISCV] Set vscale_range attribute based on presence of "v" extension

2022-10-13 Thread Craig Topper via Phabricator via cfe-commits

craig.topper added a comment.

There's also this patch from @frasercrmck https://reviews.llvm.org/D107290


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135894

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

[PATCH] D129755: Thread safety analysis: Support copy-elided production of scoped capabilities through arbitrary calls

2022-10-13 Thread Aaron Puchert 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 rG54bfd0484615: Thread safety analysis: Support copy-elided 
production of scoped capabilities… (authored by aaronpuchert).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129755

Files:
  clang/docs/ReleaseNotes.rst
  clang/docs/ThreadSafetyAnalysis.rst
  clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h
  clang/include/clang/Analysis/Analyses/ThreadSafetyTIL.h
  clang/include/clang/Analysis/Analyses/ThreadSafetyTraverse.h
  clang/lib/Analysis/ThreadSafety.cpp
  clang/lib/Analysis/ThreadSafetyCommon.cpp
  clang/test/SemaCXX/warn-thread-safety-analysis.cpp

Index: clang/test/SemaCXX/warn-thread-safety-analysis.cpp
===
--- clang/test/SemaCXX/warn-thread-safety-analysis.cpp
+++ clang/test/SemaCXX/warn-thread-safety-analysis.cpp
@@ -1606,6 +1606,30 @@
   dlr.unlockData(d1);
 }
   };
+
+  // Automatic object destructor calls don't appear as expressions in the CFG,
+  // so we have to handle them separately whenever substitutions are required.
+  struct DestructorRequires {
+Mutex mu;
+~DestructorRequires() EXCLUSIVE_LOCKS_REQUIRED(mu);
+  };
+
+  void destructorRequires() {
+DestructorRequires rd;
+rd.mu.AssertHeld();
+  }
+
+  struct DestructorExcludes {
+Mutex mu;
+~DestructorExcludes() LOCKS_EXCLUDED(mu);
+  };
+
+  void destructorExcludes() {
+DestructorExcludes ed;
+ed.mu.Lock(); // expected-note {{mutex acquired here}}
+  } // expected-warning {{cannot call function '~DestructorExcludes' while mutex 'ed.mu' is held}}
+// expected-warning@-1 {{mutex 'ed.mu' is still held at the end of function}}
+
 } // end namespace substituation_test
 
 
@@ -1690,6 +1714,15 @@
   }
 #endif
 
+  void temporary() {
+MutexLock{&mu1}, a = 5;
+  }
+
+  void lifetime_extension() {
+const MutexLock &mulock = MutexLock(&mu1);
+a = 5;
+  }
+
   void foo2() {
 ReaderMutexLock mulock1(&mu1);
 if (getBool()) {
@@ -1708,6 +1741,12 @@
   // expected-warning {{acquiring mutex 'mu1' that is already held}}
   }
 
+  void temporary_double_lock() {
+MutexLock mulock_a(&mu1); // expected-note{{mutex acquired here}}
+MutexLock{&mu1};  // \
+  // expected-warning {{acquiring mutex 'mu1' that is already held}}
+  }
+
   void foo4() {
 MutexLock mulock1(&mu1), mulock2(&mu2);
 a = b+1;
@@ -4187,6 +4226,20 @@
   void foo() EXCLUSIVE_LOCKS_REQUIRED(this);
 };
 
+class SelfLockDeferred {
+public:
+  SelfLockDeferred() LOCKS_EXCLUDED(mu_);
+  ~SelfLockDeferred() UNLOCK_FUNCTION(mu_);
+
+  Mutex mu_;
+};
+
+class LOCKABLE SelfLockDeferred2 {
+public:
+  SelfLockDeferred2() LOCKS_EXCLUDED(this);
+  ~SelfLockDeferred2() UNLOCK_FUNCTION();
+};
+
 
 void test() {
   SelfLock s;
@@ -4198,6 +4251,14 @@
   s2.foo();
 }
 
+void testDeferredTemporary() {
+  SelfLockDeferred(); // expected-warning {{releasing mutex '.mu_' that was not held}}
+}
+
+void testDeferredTemporary2() {
+  SelfLockDeferred2(); // expected-warning {{releasing mutex '' that was not held}}
+}
+
 }  // end namespace SelfConstructorTest
 
 
@@ -5892,47 +5953,75 @@
 void f() { c[A()]->g(); }
 } // namespace PR34800
 
+#ifdef __cpp_guaranteed_copy_elision
+
 namespace ReturnScopedLockable {
-  template class SCOPED_LOCKABLE ReadLockedPtr {
-  public:
-ReadLockedPtr(Object *ptr) SHARED_LOCK_FUNCTION((*this)->mutex);
-ReadLockedPtr(ReadLockedPtr &&) SHARED_LOCK_FUNCTION((*this)->mutex);
-~ReadLockedPtr() UNLOCK_FUNCTION();
 
-Object *operator->() const { return object; }
+class Object {
+public:
+  MutexLock lock() EXCLUSIVE_LOCK_FUNCTION(mutex) {
+// TODO: False positive because scoped lock isn't destructed.
+return MutexLock(&mutex); // expected-note {{mutex acquired here}}
+  }   // expected-warning {{mutex 'mutex' is still held at the end of function}}
 
-  private:
-Object *object;
-  };
+  ReaderMutexLock lockShared() SHARED_LOCK_FUNCTION(mutex) {
+// TODO: False positive because scoped lock isn't destructed.
+return ReaderMutexLock(&mutex); // expected-note {{mutex acquired here}}
+  } // expected-warning {{mutex 'mutex' is still held at the end of function}}
 
-  struct Object {
-int f() SHARED_LOCKS_REQUIRED(mutex);
-Mutex mutex;
-  };
+  MutexLock adopt() EXCLUSIVE_LOCKS_REQUIRED(mutex) {
+// TODO: False positive because scoped lock isn't destructed.
+return MutexLock(&mutex, true); // expected-note {{mutex acquired here}}
+  } // expected-warning {{mutex 'mutex' is still held at the end of function}}
 
-  ReadLockedPtr get();
-  int use() {
-auto ptr = get();
-return ptr->f();
-  }
-  void use_constructor() {
-auto ptr = ReadLockedPtr(null

[clang] 54bfd04 - Thread safety analysis: Support copy-elided production of scoped capabilities through arbitrary calls

2022-10-13 Thread Aaron Puchert via cfe-commits


Author: Aaron Puchert
Date: 2022-10-13T19:36:15+02:00
New Revision: 54bfd04846156dbd5e0a6b88f539c3d4569a455f

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

LOG: Thread safety analysis: Support copy-elided production of scoped 
capabilities through arbitrary calls

When support for copy elision was initially added in e97654b2f2807, it
was taking attributes from a constructor call, although that constructor
call is actually not involved. It seems more natural to use attributes
on the function returning the scoped capability, which is where it's
actually coming from. This would also support a number of interesting
use cases, like producing different scope kinds without the need for tag
types, or producing scopes from a private mutex.

Changing the behavior was surprisingly difficult: we were not handling
CXXConstructorExpr calls like regular calls but instead handled them
through the DeclStmt they're contained in. This was based on the
assumption that constructors are basically only called in variable
declarations (not true because of temporaries), and that variable
declarations necessitate constructors (not true with C++17 anymore).

Untangling this required separating construction from assigning a
variable name. When a call produces an object, we use a placeholder
til::LiteralPtr for `this`, and we collect the call expression and
placeholder in a map. Later when going through a DeclStmt, we look up
the call expression and set the placeholder to the new VarDecl.

The change has a couple of nice side effects:
* We don't miss constructor calls not contained in DeclStmts anymore,
  allowing patterns like
MutexLock{&mu}, requiresMutex();
  The scoped lock temporary will be destructed at the end of the full
  statement, so it protects the following call without the need for a
  scope, but with the ability to unlock in case of an exception.
* We support lifetime extension of temporaries. While unusual, one can
  now write
const MutexLock &scope = MutexLock(&mu);
  and have it behave as expected.
* Destructors used to be handled in a weird way: since there is no
  expression in the AST for implicit destructor calls, we instead
  provided a made-up DeclRefExpr to the variable being destructed, and
  passed that instead of a CallExpr. Then later in translateAttrExpr
  there was special code that knew that destructor expressions worked a
  bit different.
* We were producing dummy DeclRefExprs in a number of places, this has
  been eliminated. We now use til::SExprs instead.

Technically this could break existing code, but the current handling
seems unexpected enough to justify this change.

Reviewed By: aaron.ballman

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

Added: 


Modified: 
clang/docs/ReleaseNotes.rst
clang/docs/ThreadSafetyAnalysis.rst
clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h
clang/include/clang/Analysis/Analyses/ThreadSafetyTIL.h
clang/include/clang/Analysis/Analyses/ThreadSafetyTraverse.h
clang/lib/Analysis/ThreadSafety.cpp
clang/lib/Analysis/ThreadSafetyCommon.cpp
clang/test/SemaCXX/warn-thread-safety-analysis.cpp

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 1fe4be39e7aba..b036764803007 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -307,6 +307,13 @@ Improvements to Clang's diagnostics
   function definition without a prototype which is preceded by a static
   declaration of the function with a prototype. Fixes
   `Issue 58181 `_.
+- Copy-elided initialization of lock scopes is now handled 
diff erently in
+  ``-Wthread-safety-analysis``: annotations on the move constructor are no
+  longer taken into account, in favor of annotations on the function returning
+  the lock scope by value. This could result in new warnings if code depended
+  on the previous undocumented behavior. As a side effect of this change,
+  constructor calls outside of initializer expressions are no longer ignored,
+  which can result in new warnings (or make existing warnings disappear).
 
 Non-comprehensive list of changes in this release
 -

diff  --git a/clang/docs/ThreadSafetyAnalysis.rst 
b/clang/docs/ThreadSafetyAnalysis.rst
index 23f460b248e11..dcde0c706c704 100644
--- a/clang/docs/ThreadSafetyAnalysis.rst
+++ b/clang/docs/ThreadSafetyAnalysis.rst
@@ -408,7 +408,8 @@ and destructor refer to the capability via 
diff erent names; see the
 Scoped capabilities are treated as capabilities that are implicitly acquired
 on construction and released on destruction. They are associated with
 the set of (regular) capabilities named in thread safety attributes on t

[PATCH] D135818: [clang] Update ASM goto documentation to reflect how Clang differs from GCC

2022-10-13 Thread Nick Desaulniers via Phabricator via cfe-commits

nickdesaulniers accepted this revision.
nickdesaulniers added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/docs/LanguageExtensions.rst:1553
+  implementation`_ in
+  that Clang doesn't support outputs on the indirect branch. Use of an output
+  on the indirect branch may result in undefined behavior and should be

Consider adding a link to https://github.com/llvm/llvm-project/issues/53562?

Clang doesn't yet support... but support is planned.

or something.



Comment at: clang/docs/LanguageExtensions.rst:1557
 
-.. code-block:: c++
-

Do we need to keep the code block to retain formatting?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135818

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

[PATCH] D135894: [clang][RISCV] Set vscale_range attribute based on presence of "v" extension

2022-10-13 Thread Philip Reames via Phabricator via cfe-commits

reames created this revision.
reames added reviewers: asb, frasercrmck, craig.topper.
Herald added subscribers: sunshaoce, VincentWu, ctetreau, StephenFan, vkmr, 
evandro, luismarques, apazos, sameer.abuasal, s.egerton, Jim, benna, psnobl, 
jocewei, PkmX, the_o, brucehoult, MartinMosbeck, rogfer01, edward-jones, 
javed.absar, zzheng, jrtc27, shiva0217, kito-cheng, niosHD, sabuasal, bollu, 
simoncook, johnrusso, rbar, kristof.beyls, arichardson, mcrosier.
Herald added a project: All.
reames requested review of this revision.
Herald added subscribers: cfe-commits, pcwang-thead, eopXD, MaskRay.
Herald added a project: clang.

This follows the path that AArch64 SVE has taken.  Doing this via a function 
attribute set in the frontend is basically a workaround for the fact that 
several analyzes which need the information (i.e. known bits, lvi, scev) can't 
easily use TTI without significant amounts of plumbing changes.

This patch hard codes "v" numbers, and directly follows the SVE precedent as a 
result.  In a follow up, I hope to drive this from RISCVISAInfo.h/cpp instead, 
but the MinVLen number being returned from that interface seemed to always be 0 
(which is wrong), and I haven't figured out what's going wrong there.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D135894

Files:
  clang/lib/Basic/Targets/RISCV.cpp
  clang/lib/Basic/Targets/RISCV.h
  clang/test/CodeGen/riscv-vector-bits-vscale-range.c


Index: clang/test/CodeGen/riscv-vector-bits-vscale-range.c
===
--- /dev/null
+++ clang/test/CodeGen/riscv-vector-bits-vscale-range.c
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -triple riscv64-none-linux-gnu -target-feature +v 
-mvscale-min=1 -mvscale-max=1 -S -emit-llvm -o - %s | FileCheck %s -D#VBITS=1
+// RUN: %clang_cc1 -triple riscv64-none-linux-gnu -target-feature +v 
-mvscale-min=2 -mvscale-max=2 -S -emit-llvm -o - %s | FileCheck %s -D#VBITS=2
+// RUN: %clang_cc1 -triple riscv64-none-linux-gnu -target-feature +v 
-mvscale-min=4 -mvscale-max=4 -S -emit-llvm -o - %s | FileCheck %s -D#VBITS=4
+// RUN: %clang_cc1 -triple riscv64-none-linux-gnu -target-feature +v 
-mvscale-min=8 -mvscale-max=8 -S -emit-llvm -o - %s | FileCheck %s -D#VBITS=8
+// RUN: %clang_cc1 -triple riscv64-none-linux-gnu -target-feature +v 
-mvscale-min=16 -mvscale-max=16 -S -emit-llvm -o - %s | FileCheck %s -D#VBITS=16
+// RUN: %clang_cc1 -triple riscv64-none-linux-gnu -target-feature +v 
-mvscale-min=1 -S -emit-llvm -o - %s | FileCheck %s -D#VBITS=1 
--check-prefix=CHECK-NOMAX
+// RUN: %clang_cc1 -triple riscv64-none-linux-gnu -target-feature +v 
-mvscale-min=2 -S -emit-llvm -o - %s | FileCheck %s -D#VBITS=2 
--check-prefix=CHECK-NOMAX
+// RUN: %clang_cc1 -triple riscv64-none-linux-gnu -target-feature +v 
-mvscale-min=4 -S -emit-llvm -o - %s | FileCheck %s -D#VBITS=4 
--check-prefix=CHECK-NOMAX
+// RUN: %clang_cc1 -triple riscv64-none-linux-gnu -target-feature +v 
-mvscale-min=8 -S -emit-llvm -o - %s | FileCheck %s -D#VBITS=8 
--check-prefix=CHECK-NOMAX
+// RUN: %clang_cc1 -triple riscv64-none-linux-gnu -target-feature +v 
-mvscale-min=16 -S -emit-llvm -o - %s | FileCheck %s -D#VBITS=16 
--check-prefix=CHECK-NOMAX
+// RUN: %clang_cc1 -triple riscv64-none-linux-gnu -target-feature +v 
-mvscale-min=1 -mvscale-max=0 -S -emit-llvm -o - %s | FileCheck %s 
--check-prefix=CHECK-UNBOUNDED
+// RUN: %clang_cc1 -triple riscv64-none-linux-gnu -target-feature +v -S 
-emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-NONE
+
+// CHECK-LABEL: @func() #0
+// CHECK: attributes #0 = { {{.*}} vscale_range([[#VBITS]],[[#VBITS]]) {{.*}} }
+// CHECK-NOMAX: attributes #0 = { {{.*}} vscale_range([[#VBITS]],0) {{.*}} }
+// CHECK-UNBOUNDED: attributes #0 = { {{.*}} vscale_range(1,0) {{.*}} }
+// CHECK-NONE: attributes #0 = { {{.*}} vscale_range(2,1024) {{.*}} }
+void func(void) {}
Index: clang/lib/Basic/Targets/RISCV.h
===
--- clang/lib/Basic/Targets/RISCV.h
+++ clang/lib/Basic/Targets/RISCV.h
@@ -90,6 +90,9 @@
  StringRef CPU,
  const std::vector &FeaturesVec) const override;
 
+  Optional>
+  getVScaleRange(const LangOptions &LangOpts) const override;
+
   bool hasFeature(StringRef Feature) const override;
 
   bool handleTargetFeatures(std::vector &Features,
Index: clang/lib/Basic/Targets/RISCV.cpp
===
--- clang/lib/Basic/Targets/RISCV.cpp
+++ clang/lib/Basic/Targets/RISCV.cpp
@@ -246,6 +246,19 @@
   return TargetInfo::initFeatureMap(Features, Diags, CPU, ImpliedFeatures);
 }
 
+Optional>
+RISCVTargetInfo::getVScaleRange(const LangOptions &LangOpts) const {
+  if (LangOpts.VScaleMin || LangOpts.VScaleMax)
+return std::pair(
+LangOpts.VScaleMin ? LangOpts.VScaleMin : 1, LangOpts.VScaleMax);
+
+  if (hasFeature("v"))
+// Minium VLEN=128, Maximum VLEN=64k, and RISCV::RVVBitsPerBlock is 64.
+return std::

[PATCH] D135220: [clang] Update ModuleMap::getModuleMapFile* to use FileEntryRef

2022-10-13 Thread Hans Wennborg via Phabricator via cfe-commits

hans added a comment.

Here's a reproducer of what's happening in our case. It's using the 
preprocessor because that's the easiest, but the real problem for us is that 
the debug info is pointing to the wrong file (and that it's different depending 
on the symlinkedness of the include).

  $ cat /tmp/foo.h
  #ifdef X
  #undef X
  int x;
  #endif
  
  #ifdef Y
  #undef Y
  int y;
  #endif
  
  $ ln -s /tmp/foo.h /tmp/bar.h
  
  $ cat /tmp/a.cc
  #define X
  #include "foo.h"
  #define Y
  #include "bar.h"
  
  $ build/bin/clang -E -o - /tmp/a.cc
  # 1 "/tmp/a.cc"
  # 1 "" 1
  # 1 "" 3
  # 437 "" 3
  # 1 "" 1
  # 1 "" 2
  # 1 "/tmp/a.cc" 2
  
  # 1 "/tmp/foo.h" 1
  
  
  int x;
  # 3 "/tmp/a.cc" 2
  
  # 1 "/tmp/foo.h" 1
  
  
  
  
  
  
  
  int y;
  # 5 "/tmp/a.cc" 2

Note that the second `# 1 "/tmp/foo.h" 1` should really be `bar.h`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135220

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

[PATCH] D135557: Add needsImplicitDefaultConstructor and friends

2022-10-13 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added subscribers: royjacobson, erichkeane.
aaron.ballman added inline comments.



Comment at: clang/bindings/python/clang/cindex.py:1530
+
+def record_needs_implicit_default_constructor(self):
+"""Returns True if the cursor refers to a C++ record declaration

aaron.ballman wrote:
> I don't think we should expose any of the "needs" functions like this -- 
> those are internal implementation details of the class and I don't think we 
> want to calcify that into something we have to support forever. As we add 
> members to a class, we recalculate whether the added member causes us to 
> delete defaulted special members (among other things), and the "needs" 
> functions are basically used when the class is completed to handle lazily 
> created special members. I'm pretty sure that lazy creation is not mandated 
> by the standard, which is why I think the "needs" functions are more of an 
> implementation detail.
CC @erichkeane and @royjacobson as folks who have been in this same area of the 
compiler to see if they agree or disagree with my assessment there.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135557

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

[PATCH] D135557: Add needsImplicitDefaultConstructor and friends

2022-10-13 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added inline comments.



Comment at: clang/bindings/python/clang/cindex.py:1512-1514
+def record_defaulted_copy_constructor_is_deleted(self):
+"""Returns True if the cursor refers to a C++ record declaration
+that has its default copy constructor deleted.

We might want to document that this API may return different results while the 
record is being formed from after the record is complete. e.g., we may say the 
defaulted copy constructor is not deleted at the start of the record but later 
find a data member that causes us to delete the defaulted copy constructor. 
(Similar for the others.)



Comment at: clang/bindings/python/clang/cindex.py:1530
+
+def record_needs_implicit_default_constructor(self):
+"""Returns True if the cursor refers to a C++ record declaration

I don't think we should expose any of the "needs" functions like this -- those 
are internal implementation details of the class and I don't think we want to 
calcify that into something we have to support forever. As we add members to a 
class, we recalculate whether the added member causes us to delete defaulted 
special members (among other things), and the "needs" functions are basically 
used when the class is completed to handle lazily created special members. I'm 
pretty sure that lazy creation is not mandated by the standard, which is why I 
think the "needs" functions are more of an implementation detail.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135557

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

[PATCH] D135892: [clangd] Add missing readonly modifier for const generic parameters

2022-10-13 Thread Tom Praschan via Phabricator via cfe-commits

tom-anders added a comment.

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135892

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

[clang-tools-extra] a8124ee - [clangd] Add missing readonly modifier for const generic parameters

2022-10-13 Thread Tom Praschan via cfe-commits


Author: Tom Praschan
Date: 2022-10-13T21:09:25+02:00
New Revision: a8124eea9df513a5565805b480c227fa88751359

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

LOG: [clangd] Add missing readonly modifier for const generic parameters

Fixes https://github.com/clangd/clangd/issues/1222. As discussed there, we saw 
no reason to keep this check.

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

Added: 


Modified: 
clang-tools-extra/clangd/SemanticHighlighting.cpp
clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/SemanticHighlighting.cpp 
b/clang-tools-extra/clangd/SemanticHighlighting.cpp
index 5b6fcf923e3f..2375d6f7dd35 100644
--- a/clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ b/clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -164,7 +164,7 @@ kindForType(const Type *TP, const HeuristicResolver 
*Resolver) {
 
 // Whether T is const in a loose sense - is a variable with this type readonly?
 bool isConst(QualType T) {
-  if (T.isNull() || T->isDependentType())
+  if (T.isNull())
 return false;
   T = T.getNonReferenceType();
   if (T.isConstQualified())

diff  --git a/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp 
b/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
index e851bea5e632..6011aca7fa39 100644
--- a/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ b/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -855,6 +855,17 @@ sizeof...($TemplateParameter[[Elements]]);
 const char *$LocalVariable_decl_readonly[[s]] = 
$LocalVariable_readonly_static[[__func__]];
 }
   )cpp",
+  // Issue 1022: readonly modifier for generic parameter
+  R"cpp(
+template 
+auto $Function_decl[[foo]](const $TemplateParameter[[T]] 
$Parameter_decl_readonly[[template_type]], 
+   const $TemplateParameter[[auto]] 
$Parameter_decl_readonly[[auto_type]], 
+   const int 
$Parameter_decl_readonly[[explicit_type]]) {
+return $Parameter_readonly[[template_type]] 
+ + $Parameter_readonly[[auto_type]] 
+ + $Parameter_readonly[[explicit_type]];
+}
+  )cpp",
   // Explicit template specialization
   R"cpp(
 struct $Class_decl[[Base]]{};



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

[PATCH] D135892: [clangd] Add missing readonly modifier for const generic parameters

2022-10-13 Thread Tom Praschan 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 rGa8124eea9df5: [clangd] Add missing readonly modifier for 
const generic parameters (authored by tom-anders).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135892

Files:
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp


Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -855,6 +855,17 @@
 const char *$LocalVariable_decl_readonly[[s]] = 
$LocalVariable_readonly_static[[__func__]];
 }
   )cpp",
+  // Issue 1022: readonly modifier for generic parameter
+  R"cpp(
+template 
+auto $Function_decl[[foo]](const $TemplateParameter[[T]] 
$Parameter_decl_readonly[[template_type]], 
+   const $TemplateParameter[[auto]] 
$Parameter_decl_readonly[[auto_type]], 
+   const int 
$Parameter_decl_readonly[[explicit_type]]) {
+return $Parameter_readonly[[template_type]] 
+ + $Parameter_readonly[[auto_type]] 
+ + $Parameter_readonly[[explicit_type]];
+}
+  )cpp",
   // Explicit template specialization
   R"cpp(
 struct $Class_decl[[Base]]{};
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -164,7 +164,7 @@
 
 // Whether T is const in a loose sense - is a variable with this type readonly?
 bool isConst(QualType T) {
-  if (T.isNull() || T->isDependentType())
+  if (T.isNull())
 return false;
   T = T.getNonReferenceType();
   if (T.isConstQualified())


Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -855,6 +855,17 @@
 const char *$LocalVariable_decl_readonly[[s]] = $LocalVariable_readonly_static[[__func__]];
 }
   )cpp",
+  // Issue 1022: readonly modifier for generic parameter
+  R"cpp(
+template 
+auto $Function_decl[[foo]](const $TemplateParameter[[T]] $Parameter_decl_readonly[[template_type]], 
+   const $TemplateParameter[[auto]] $Parameter_decl_readonly[[auto_type]], 
+   const int $Parameter_decl_readonly[[explicit_type]]) {
+return $Parameter_readonly[[template_type]] 
+ + $Parameter_readonly[[auto_type]] 
+ + $Parameter_readonly[[explicit_type]];
+}
+  )cpp",
   // Explicit template specialization
   R"cpp(
 struct $Class_decl[[Base]]{};
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -164,7 +164,7 @@
 
 // Whether T is const in a loose sense - is a variable with this type readonly?
 bool isConst(QualType T) {
-  if (T.isNull() || T->isDependentType())
+  if (T.isNull())
 return false;
   T = T.getNonReferenceType();
   if (T.isConstQualified())
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D135220: [clang] Update ModuleMap::getModuleMapFile* to use FileEntryRef

2022-10-13 Thread Hans Wennborg via Phabricator via cfe-commits

hans added a comment.

Heads up that we're seeing a problem in Chromium that bisects to this change. 
(http://crbug.com/1373836)

In our case what's happening is that the compiler is getting the name of an 
included file wrong, which ends up in the debug info (and also shows in the 
preprocessor output). What makes it interesting is that this only reproduces on 
a remote build system, not when building locally.

I see there's discussion about symlinks of identical files, and I just realized 
that in our case the files are indeed identical. I haven't heard back from the 
build system folks, but I wouldn't be surprised if they symlink these against 
some content-addressed storage system.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135220

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

[clang] 576283c - [clang] Support `constexpr` for some `ASTNodeKind` member functions

2022-10-13 Thread Eric Li via cfe-commits


Author: Eric Li
Date: 2022-10-13T13:00:48-04:00
New Revision: 576283c3a8ef5078b3ec12fa442c14f3a1b5fea2

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

LOG: [clang] Support `constexpr` for some `ASTNodeKind` member functions

Add `constexpr` support for:

  * The `getFromNodeKind` factory function
  * `isSame`
  * `isNone`
  * `hasPointerIdentity`

This enables these functions to be used in SFINAE context for AST node
types.

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

Added: 


Modified: 
clang/include/clang/AST/ASTTypeTraits.h
clang/unittests/AST/ASTTypeTraitsTest.cpp

Removed: 




diff  --git a/clang/include/clang/AST/ASTTypeTraits.h 
b/clang/include/clang/AST/ASTTypeTraits.h
index cd6b5143bf79..8713221a7378 100644
--- a/clang/include/clang/AST/ASTTypeTraits.h
+++ b/clang/include/clang/AST/ASTTypeTraits.h
@@ -51,11 +51,10 @@ enum TraversalKind {
 class ASTNodeKind {
 public:
   /// Empty identifier. It matches nothing.
-  ASTNodeKind() : KindId(NKI_None) {}
+  constexpr ASTNodeKind() : KindId(NKI_None) {}
 
   /// Construct an identifier for T.
-  template 
-  static ASTNodeKind getFromNodeKind() {
+  template  static constexpr ASTNodeKind getFromNodeKind() {
 return ASTNodeKind(KindToKindId::Id);
   }
 
@@ -71,12 +70,12 @@ class ASTNodeKind {
   /// \}
 
   /// Returns \c true if \c this and \c Other represent the same kind.
-  bool isSame(ASTNodeKind Other) const {
+  constexpr bool isSame(ASTNodeKind Other) const {
 return KindId != NKI_None && KindId == Other.KindId;
   }
 
   /// Returns \c true only for the default \c ASTNodeKind()
-  bool isNone() const { return KindId == NKI_None; }
+  constexpr bool isNone() const { return KindId == NKI_None; }
 
   /// Returns \c true if \c this is a base kind of (or same as) \c Other.
   /// \param Distance If non-null, used to return the distance between \c this
@@ -87,7 +86,7 @@ class ASTNodeKind {
   StringRef asStringRef() const;
 
   /// Strict weak ordering for ASTNodeKind.
-  bool operator<(const ASTNodeKind &Other) const {
+  constexpr bool operator<(const ASTNodeKind &Other) const {
 return KindId < Other.KindId;
   }
 
@@ -121,7 +120,7 @@ class ASTNodeKind {
 
   /// Check if the given ASTNodeKind identifies a type that offers pointer
   /// identity. This is useful for the fast path in DynTypedNode.
-  bool hasPointerIdentity() const {
+  constexpr bool hasPointerIdentity() const {
 return KindId > NKI_LastKindWithoutPointerIdentity;
   }
 
@@ -165,7 +164,7 @@ class ASTNodeKind {
   };
 
   /// Use getFromNodeKind() to construct the kind.
-  ASTNodeKind(NodeKindId KindId) : KindId(KindId) {}
+  constexpr ASTNodeKind(NodeKindId KindId) : KindId(KindId) {}
 
   /// Returns \c true if \c Base is a base kind of (or same as) \c
   ///   Derived.

diff  --git a/clang/unittests/AST/ASTTypeTraitsTest.cpp 
b/clang/unittests/AST/ASTTypeTraitsTest.cpp
index 9fcb00630209..dcea32d1f1c0 100644
--- a/clang/unittests/AST/ASTTypeTraitsTest.cpp
+++ b/clang/unittests/AST/ASTTypeTraitsTest.cpp
@@ -117,6 +117,47 @@ TEST(ASTNodeKind, UnknownKind) {
   EXPECT_FALSE(DNT().isSame(DNT()));
 }
 
+template 
+constexpr bool HasPointerIdentity =
+ASTNodeKind::getFromNodeKind().hasPointerIdentity();
+
+TEST(ASTNodeKind, ConstexprHasPointerIdentity) {
+  EXPECT_TRUE(HasPointerIdentity);
+  EXPECT_TRUE(HasPointerIdentity);
+  EXPECT_FALSE(HasPointerIdentity);
+  EXPECT_FALSE(HasPointerIdentity);
+  EXPECT_FALSE(HasPointerIdentity);
+
+  constexpr bool DefaultConstructedHasPointerIdentity =
+  ASTNodeKind().hasPointerIdentity();
+  EXPECT_FALSE(DefaultConstructedHasPointerIdentity);
+}
+
+template 
+constexpr bool NodeKindIsSame =
+
ASTNodeKind::getFromNodeKind().isSame(ASTNodeKind::getFromNodeKind());
+
+TEST(ASTNodeKind, ConstexprIsSame) {
+  EXPECT_TRUE((NodeKindIsSame));
+  EXPECT_FALSE((NodeKindIsSame));
+  EXPECT_FALSE((NodeKindIsSame));
+
+  constexpr bool DefaultConstructedIsSameToDefaultConstructed =
+  ASTNodeKind().isSame(ASTNodeKind());
+  EXPECT_FALSE(DefaultConstructedIsSameToDefaultConstructed);
+}
+
+template 
+constexpr bool NodeKindIsNone = ASTNodeKind::getFromNodeKind().isNone();
+
+TEST(ASTNodeKind, ConstexprIsNone) {
+  EXPECT_FALSE(NodeKindIsNone);
+  EXPECT_TRUE(NodeKindIsNone);
+
+  constexpr bool DefaultConstructedIsNone = ASTNodeKind().isNone();
+  EXPECT_TRUE(DefaultConstructedIsNone);
+}
+
 TEST(ASTNodeKind, Name) {
   EXPECT_EQ("", ASTNodeKind().asStringRef());
 #define VERIFY_NAME(Node) EXPECT_EQ(#Node, DNT().asStringRef());



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

[PATCH] D135816: [clang] Support `constexpr` for some `ASTNodeKind` member functions

2022-10-13 Thread Eric Li via Phabricator via cfe-commits

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
li.zhe.hua marked an inline comment as done.
Closed by commit rG576283c3a8ef: [clang] Support `constexpr` for some 
`ASTNodeKind` member functions (authored by li.zhe.hua).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135816

Files:
  clang/include/clang/AST/ASTTypeTraits.h
  clang/unittests/AST/ASTTypeTraitsTest.cpp

Index: clang/unittests/AST/ASTTypeTraitsTest.cpp
===
--- clang/unittests/AST/ASTTypeTraitsTest.cpp
+++ clang/unittests/AST/ASTTypeTraitsTest.cpp
@@ -117,6 +117,47 @@
   EXPECT_FALSE(DNT().isSame(DNT()));
 }
 
+template 
+constexpr bool HasPointerIdentity =
+ASTNodeKind::getFromNodeKind().hasPointerIdentity();
+
+TEST(ASTNodeKind, ConstexprHasPointerIdentity) {
+  EXPECT_TRUE(HasPointerIdentity);
+  EXPECT_TRUE(HasPointerIdentity);
+  EXPECT_FALSE(HasPointerIdentity);
+  EXPECT_FALSE(HasPointerIdentity);
+  EXPECT_FALSE(HasPointerIdentity);
+
+  constexpr bool DefaultConstructedHasPointerIdentity =
+  ASTNodeKind().hasPointerIdentity();
+  EXPECT_FALSE(DefaultConstructedHasPointerIdentity);
+}
+
+template 
+constexpr bool NodeKindIsSame =
+ASTNodeKind::getFromNodeKind().isSame(ASTNodeKind::getFromNodeKind());
+
+TEST(ASTNodeKind, ConstexprIsSame) {
+  EXPECT_TRUE((NodeKindIsSame));
+  EXPECT_FALSE((NodeKindIsSame));
+  EXPECT_FALSE((NodeKindIsSame));
+
+  constexpr bool DefaultConstructedIsSameToDefaultConstructed =
+  ASTNodeKind().isSame(ASTNodeKind());
+  EXPECT_FALSE(DefaultConstructedIsSameToDefaultConstructed);
+}
+
+template 
+constexpr bool NodeKindIsNone = ASTNodeKind::getFromNodeKind().isNone();
+
+TEST(ASTNodeKind, ConstexprIsNone) {
+  EXPECT_FALSE(NodeKindIsNone);
+  EXPECT_TRUE(NodeKindIsNone);
+
+  constexpr bool DefaultConstructedIsNone = ASTNodeKind().isNone();
+  EXPECT_TRUE(DefaultConstructedIsNone);
+}
+
 TEST(ASTNodeKind, Name) {
   EXPECT_EQ("", ASTNodeKind().asStringRef());
 #define VERIFY_NAME(Node) EXPECT_EQ(#Node, DNT().asStringRef());
Index: clang/include/clang/AST/ASTTypeTraits.h
===
--- clang/include/clang/AST/ASTTypeTraits.h
+++ clang/include/clang/AST/ASTTypeTraits.h
@@ -51,11 +51,10 @@
 class ASTNodeKind {
 public:
   /// Empty identifier. It matches nothing.
-  ASTNodeKind() : KindId(NKI_None) {}
+  constexpr ASTNodeKind() : KindId(NKI_None) {}
 
   /// Construct an identifier for T.
-  template 
-  static ASTNodeKind getFromNodeKind() {
+  template  static constexpr ASTNodeKind getFromNodeKind() {
 return ASTNodeKind(KindToKindId::Id);
   }
 
@@ -71,12 +70,12 @@
   /// \}
 
   /// Returns \c true if \c this and \c Other represent the same kind.
-  bool isSame(ASTNodeKind Other) const {
+  constexpr bool isSame(ASTNodeKind Other) const {
 return KindId != NKI_None && KindId == Other.KindId;
   }
 
   /// Returns \c true only for the default \c ASTNodeKind()
-  bool isNone() const { return KindId == NKI_None; }
+  constexpr bool isNone() const { return KindId == NKI_None; }
 
   /// Returns \c true if \c this is a base kind of (or same as) \c Other.
   /// \param Distance If non-null, used to return the distance between \c this
@@ -87,7 +86,7 @@
   StringRef asStringRef() const;
 
   /// Strict weak ordering for ASTNodeKind.
-  bool operator<(const ASTNodeKind &Other) const {
+  constexpr bool operator<(const ASTNodeKind &Other) const {
 return KindId < Other.KindId;
   }
 
@@ -121,7 +120,7 @@
 
   /// Check if the given ASTNodeKind identifies a type that offers pointer
   /// identity. This is useful for the fast path in DynTypedNode.
-  bool hasPointerIdentity() const {
+  constexpr bool hasPointerIdentity() const {
 return KindId > NKI_LastKindWithoutPointerIdentity;
   }
 
@@ -165,7 +164,7 @@
   };
 
   /// Use getFromNodeKind() to construct the kind.
-  ASTNodeKind(NodeKindId KindId) : KindId(KindId) {}
+  constexpr ASTNodeKind(NodeKindId KindId) : KindId(KindId) {}
 
   /// Returns \c true if \c Base is a base kind of (or same as) \c
   ///   Derived.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D135397: [clang][dataflow] Add support for a Top value in boolean formulas.

2022-10-13 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits

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

Add explicit test for creation of multiple tops.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135397

Files:
  clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
  clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
  clang/include/clang/Analysis/FlowSensitive/Value.h
  clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
  clang/lib/Analysis/FlowSensitive/DebugSupport.cpp
  clang/lib/Analysis/FlowSensitive/Transfer.cpp
  clang/lib/Analysis/FlowSensitive/WatchedLiteralsSolver.cpp
  clang/unittests/Analysis/FlowSensitive/DataflowAnalysisContextTest.cpp
  clang/unittests/Analysis/FlowSensitive/SolverTest.cpp
  clang/unittests/Analysis/FlowSensitive/TestingSupport.h
  clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
@@ -17,6 +17,7 @@
 #include "clang/Analysis/FlowSensitive/DataflowAnalysisContext.h"
 #include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
 #include "clang/Analysis/FlowSensitive/DataflowLattice.h"
+#include "clang/Analysis/FlowSensitive/DebugSupport.h"
 #include "clang/Analysis/FlowSensitive/NoopAnalysis.h"
 #include "clang/Analysis/FlowSensitive/Value.h"
 #include "clang/Analysis/FlowSensitive/WatchedLiteralsSolver.h"
@@ -166,7 +167,7 @@
 auto CS = Elt->getAs();
 if (!CS)
   return;
-auto S = CS->getStmt();
+const auto *S = CS->getStmt();
 if (auto *C = dyn_cast(S)) {
   if (auto *F = dyn_cast(C->getCalleeDecl())) {
 E.CalledFunctions.insert(F->getNameInfo().getAsString());
@@ -322,7 +323,7 @@
 auto CS = Elt->getAs();
 if (!CS)
   return;
-auto S = CS->getStmt();
+const auto *S = CS->getStmt();
 auto SpecialBoolRecordDecl = recordDecl(hasName("SpecialBool"));
 auto HasSpecialBoolType = hasType(SpecialBoolRecordDecl);
 
@@ -468,9 +469,8 @@
 class OptionalIntAnalysis
 : public DataflowAnalysis {
 public:
-  explicit OptionalIntAnalysis(ASTContext &Context, BoolValue &HasValueTop)
-  : DataflowAnalysis(Context),
-HasValueTop(HasValueTop) {}
+  explicit OptionalIntAnalysis(ASTContext &Context)
+  : DataflowAnalysis(Context) {}
 
   static NoopLattice initialElement() { return {}; }
 
@@ -478,22 +478,23 @@
 auto CS = Elt->getAs();
 if (!CS)
   return;
-auto S = CS->getStmt();
+const Stmt *S = CS->getStmt();
 auto OptionalIntRecordDecl = recordDecl(hasName("OptionalInt"));
 auto HasOptionalIntType = hasType(OptionalIntRecordDecl);
 
+SmallVector Matches = match(
+stmt(anyOf(cxxConstructExpr(HasOptionalIntType).bind("construct"),
+   cxxOperatorCallExpr(
+   callee(cxxMethodDecl(ofClass(OptionalIntRecordDecl
+   .bind("operator"))),
+*S, getASTContext());
 if (const auto *E = selectFirst(
-"call", match(cxxConstructExpr(HasOptionalIntType).bind("call"), *S,
-  getASTContext( {
+"construct", Matches)) {
   auto &ConstructorVal = *Env.createValue(E->getType());
   ConstructorVal.setProperty("has_value", Env.getBoolLiteralValue(false));
   Env.setValue(*Env.getStorageLocation(*E, SkipPast::None), ConstructorVal);
-} else if (const auto *E = selectFirst(
-   "call",
-   match(cxxOperatorCallExpr(callee(cxxMethodDecl(ofClass(
- OptionalIntRecordDecl
- .bind("call"),
- *S, getASTContext( {
+} else if (const auto *E =
+   selectFirst("operator", Matches)) {
   assert(E->getNumArgs() > 0);
   auto *Object = E->getArg(0);
   assert(Object != nullptr);
@@ -516,7 +517,11 @@
 Type->getAsCXXRecordDecl()->getQualifiedNameAsString() != "OptionalInt")
   return false;
 
-return Val1.getProperty("has_value") == Val2.getProperty("has_value");
+auto *Prop1  = Val1.getProperty("has_value");
+auto *Prop2 = Val2.getProperty("has_value");
+return Prop1 == Prop2 ||
+   (Prop1 != nullptr && Prop2 != nullptr && isa(Prop1) &&
+isa(Prop2));
   }
 
   bool merge(QualType Type, const Value &Val1, const Environment &Env1,
@@ -538,11 +543,9 @@
 if (HasValue1 == HasValue2)
   MergedVal.setProperty("has_value", *HasValue1);
 else
-  MergedVal.setProperty("has_value", HasValueTop);
+  MergedVal.setProperty("has_value", MergedEnv.makeTopBoolValue());
 return true;
   }
-
-  BoolValue &HasValueTop;
 };
 
 class Wid

[PATCH] D135816: [clang] Support `constexpr` for some `ASTNodeKind` member functions

2022-10-13 Thread Eric Li via Phabricator via cfe-commits

li.zhe.hua marked an inline comment as done.
li.zhe.hua added inline comments.



Comment at: clang/include/clang/AST/ASTTypeTraits.h:90-92
   bool operator<(const ASTNodeKind &Other) const {
 return KindId < Other.KindId;
   }

aaron.ballman wrote:
> Any reason for this to not be `constexpr` as well?
Not in particular. I really only needed `hasPointerIdentity`, but figured it'd 
be odd not to support `isSame` or `isNone`, so the same probably goes for 
`operator<`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135816

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

[PATCH] D135816: [clang] Support `constexpr` for some `ASTNodeKind` member functions

2022-10-13 Thread Eric Li via Phabricator via cfe-commits

li.zhe.hua updated this revision to Diff 467512.
li.zhe.hua added a comment.

Appease clang-format, add constexpr to operator<.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135816

Files:
  clang/include/clang/AST/ASTTypeTraits.h
  clang/unittests/AST/ASTTypeTraitsTest.cpp

Index: clang/unittests/AST/ASTTypeTraitsTest.cpp
===
--- clang/unittests/AST/ASTTypeTraitsTest.cpp
+++ clang/unittests/AST/ASTTypeTraitsTest.cpp
@@ -117,6 +117,47 @@
   EXPECT_FALSE(DNT().isSame(DNT()));
 }
 
+template 
+constexpr bool HasPointerIdentity =
+ASTNodeKind::getFromNodeKind().hasPointerIdentity();
+
+TEST(ASTNodeKind, ConstexprHasPointerIdentity) {
+  EXPECT_TRUE(HasPointerIdentity);
+  EXPECT_TRUE(HasPointerIdentity);
+  EXPECT_FALSE(HasPointerIdentity);
+  EXPECT_FALSE(HasPointerIdentity);
+  EXPECT_FALSE(HasPointerIdentity);
+
+  constexpr bool DefaultConstructedHasPointerIdentity =
+  ASTNodeKind().hasPointerIdentity();
+  EXPECT_FALSE(DefaultConstructedHasPointerIdentity);
+}
+
+template 
+constexpr bool NodeKindIsSame =
+ASTNodeKind::getFromNodeKind().isSame(ASTNodeKind::getFromNodeKind());
+
+TEST(ASTNodeKind, ConstexprIsSame) {
+  EXPECT_TRUE((NodeKindIsSame));
+  EXPECT_FALSE((NodeKindIsSame));
+  EXPECT_FALSE((NodeKindIsSame));
+
+  constexpr bool DefaultConstructedIsSameToDefaultConstructed =
+  ASTNodeKind().isSame(ASTNodeKind());
+  EXPECT_FALSE(DefaultConstructedIsSameToDefaultConstructed);
+}
+
+template 
+constexpr bool NodeKindIsNone = ASTNodeKind::getFromNodeKind().isNone();
+
+TEST(ASTNodeKind, ConstexprIsNone) {
+  EXPECT_FALSE(NodeKindIsNone);
+  EXPECT_TRUE(NodeKindIsNone);
+
+  constexpr bool DefaultConstructedIsNone = ASTNodeKind().isNone();
+  EXPECT_TRUE(DefaultConstructedIsNone);
+}
+
 TEST(ASTNodeKind, Name) {
   EXPECT_EQ("", ASTNodeKind().asStringRef());
 #define VERIFY_NAME(Node) EXPECT_EQ(#Node, DNT().asStringRef());
Index: clang/include/clang/AST/ASTTypeTraits.h
===
--- clang/include/clang/AST/ASTTypeTraits.h
+++ clang/include/clang/AST/ASTTypeTraits.h
@@ -51,11 +51,10 @@
 class ASTNodeKind {
 public:
   /// Empty identifier. It matches nothing.
-  ASTNodeKind() : KindId(NKI_None) {}
+  constexpr ASTNodeKind() : KindId(NKI_None) {}
 
   /// Construct an identifier for T.
-  template 
-  static ASTNodeKind getFromNodeKind() {
+  template  static constexpr ASTNodeKind getFromNodeKind() {
 return ASTNodeKind(KindToKindId::Id);
   }
 
@@ -71,12 +70,12 @@
   /// \}
 
   /// Returns \c true if \c this and \c Other represent the same kind.
-  bool isSame(ASTNodeKind Other) const {
+  constexpr bool isSame(ASTNodeKind Other) const {
 return KindId != NKI_None && KindId == Other.KindId;
   }
 
   /// Returns \c true only for the default \c ASTNodeKind()
-  bool isNone() const { return KindId == NKI_None; }
+  constexpr bool isNone() const { return KindId == NKI_None; }
 
   /// Returns \c true if \c this is a base kind of (or same as) \c Other.
   /// \param Distance If non-null, used to return the distance between \c this
@@ -87,7 +86,7 @@
   StringRef asStringRef() const;
 
   /// Strict weak ordering for ASTNodeKind.
-  bool operator<(const ASTNodeKind &Other) const {
+  constexpr bool operator<(const ASTNodeKind &Other) const {
 return KindId < Other.KindId;
   }
 
@@ -121,7 +120,7 @@
 
   /// Check if the given ASTNodeKind identifies a type that offers pointer
   /// identity. This is useful for the fast path in DynTypedNode.
-  bool hasPointerIdentity() const {
+  constexpr bool hasPointerIdentity() const {
 return KindId > NKI_LastKindWithoutPointerIdentity;
   }
 
@@ -165,7 +164,7 @@
   };
 
   /// Use getFromNodeKind() to construct the kind.
-  ASTNodeKind(NodeKindId KindId) : KindId(KindId) {}
+  constexpr ASTNodeKind(NodeKindId KindId) : KindId(KindId) {}
 
   /// Returns \c true if \c Base is a base kind of (or same as) \c
   ///   Derived.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D135892: [clangd] Add missing readonly modifier for const generic parameters

2022-10-13 Thread Tom Praschan via Phabricator via cfe-commits

tom-anders created this revision.
tom-anders added reviewers: nridge, sammccall.
Herald added subscribers: kadircet, arphaman.
Herald added a project: All.
tom-anders requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

Fixes https://github.com/clangd/clangd/issues/1222. As discussed there, we saw 
no reason to keep this check.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D135892

Files:
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp


Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -847,6 +847,17 @@
 const char *$LocalVariable_decl_readonly[[s]] = 
$LocalVariable_readonly_static[[__func__]];
 }
   )cpp",
+  // Issue 1022: readonly modifier for generic parameter
+  R"cpp(
+template 
+auto $Function_decl[[foo]](const $TemplateParameter[[T]] 
$Parameter_decl_readonly[[template_type]], 
+   const $TemplateParameter[[auto]] 
$Parameter_decl_readonly[[auto_type]], 
+   const int 
$Parameter_decl_readonly[[explicit_type]]) {
+return $Parameter_readonly[[template_type]] 
+ + $Parameter_readonly[[auto_type]] 
+ + $Parameter_readonly[[explicit_type]];
+}
+  )cpp",
   // Explicit template specialization
   R"cpp(
 struct $Class_decl[[Base]]{};
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -164,7 +164,7 @@
 
 // Whether T is const in a loose sense - is a variable with this type readonly?
 bool isConst(QualType T) {
-  if (T.isNull() || T->isDependentType())
+  if (T.isNull())
 return false;
   T = T.getNonReferenceType();
   if (T.isConstQualified())


Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -847,6 +847,17 @@
 const char *$LocalVariable_decl_readonly[[s]] = $LocalVariable_readonly_static[[__func__]];
 }
   )cpp",
+  // Issue 1022: readonly modifier for generic parameter
+  R"cpp(
+template 
+auto $Function_decl[[foo]](const $TemplateParameter[[T]] $Parameter_decl_readonly[[template_type]], 
+   const $TemplateParameter[[auto]] $Parameter_decl_readonly[[auto_type]], 
+   const int $Parameter_decl_readonly[[explicit_type]]) {
+return $Parameter_readonly[[template_type]] 
+ + $Parameter_readonly[[auto_type]] 
+ + $Parameter_readonly[[explicit_type]];
+}
+  )cpp",
   // Explicit template specialization
   R"cpp(
 struct $Class_decl[[Base]]{};
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -164,7 +164,7 @@
 
 // Whether T is const in a loose sense - is a variable with this type readonly?
 bool isConst(QualType T) {
-  if (T.isNull() || T->isDependentType())
+  if (T.isNull())
 return false;
   T = T.getNonReferenceType();
   if (T.isConstQualified())
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D135397: [clang][dataflow] Add support for a Top value in boolean formulas.

2022-10-13 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits

ymandel updated this revision to Diff 467510.
ymandel marked 6 inline comments as done.
ymandel added a comment.

Address comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135397

Files:
  clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
  clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
  clang/include/clang/Analysis/FlowSensitive/Value.h
  clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
  clang/lib/Analysis/FlowSensitive/DebugSupport.cpp
  clang/lib/Analysis/FlowSensitive/Transfer.cpp
  clang/lib/Analysis/FlowSensitive/WatchedLiteralsSolver.cpp
  clang/unittests/Analysis/FlowSensitive/SolverTest.cpp
  clang/unittests/Analysis/FlowSensitive/TestingSupport.h
  clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
@@ -17,6 +17,7 @@
 #include "clang/Analysis/FlowSensitive/DataflowAnalysisContext.h"
 #include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
 #include "clang/Analysis/FlowSensitive/DataflowLattice.h"
+#include "clang/Analysis/FlowSensitive/DebugSupport.h"
 #include "clang/Analysis/FlowSensitive/NoopAnalysis.h"
 #include "clang/Analysis/FlowSensitive/Value.h"
 #include "clang/Analysis/FlowSensitive/WatchedLiteralsSolver.h"
@@ -166,7 +167,7 @@
 auto CS = Elt->getAs();
 if (!CS)
   return;
-auto S = CS->getStmt();
+const auto *S = CS->getStmt();
 if (auto *C = dyn_cast(S)) {
   if (auto *F = dyn_cast(C->getCalleeDecl())) {
 E.CalledFunctions.insert(F->getNameInfo().getAsString());
@@ -322,7 +323,7 @@
 auto CS = Elt->getAs();
 if (!CS)
   return;
-auto S = CS->getStmt();
+const auto *S = CS->getStmt();
 auto SpecialBoolRecordDecl = recordDecl(hasName("SpecialBool"));
 auto HasSpecialBoolType = hasType(SpecialBoolRecordDecl);
 
@@ -468,9 +469,8 @@
 class OptionalIntAnalysis
 : public DataflowAnalysis {
 public:
-  explicit OptionalIntAnalysis(ASTContext &Context, BoolValue &HasValueTop)
-  : DataflowAnalysis(Context),
-HasValueTop(HasValueTop) {}
+  explicit OptionalIntAnalysis(ASTContext &Context)
+  : DataflowAnalysis(Context) {}
 
   static NoopLattice initialElement() { return {}; }
 
@@ -478,22 +478,23 @@
 auto CS = Elt->getAs();
 if (!CS)
   return;
-auto S = CS->getStmt();
+const Stmt *S = CS->getStmt();
 auto OptionalIntRecordDecl = recordDecl(hasName("OptionalInt"));
 auto HasOptionalIntType = hasType(OptionalIntRecordDecl);
 
+SmallVector Matches = match(
+stmt(anyOf(cxxConstructExpr(HasOptionalIntType).bind("construct"),
+   cxxOperatorCallExpr(
+   callee(cxxMethodDecl(ofClass(OptionalIntRecordDecl
+   .bind("operator"))),
+*S, getASTContext());
 if (const auto *E = selectFirst(
-"call", match(cxxConstructExpr(HasOptionalIntType).bind("call"), *S,
-  getASTContext( {
+"construct", Matches)) {
   auto &ConstructorVal = *Env.createValue(E->getType());
   ConstructorVal.setProperty("has_value", Env.getBoolLiteralValue(false));
   Env.setValue(*Env.getStorageLocation(*E, SkipPast::None), ConstructorVal);
-} else if (const auto *E = selectFirst(
-   "call",
-   match(cxxOperatorCallExpr(callee(cxxMethodDecl(ofClass(
- OptionalIntRecordDecl
- .bind("call"),
- *S, getASTContext( {
+} else if (const auto *E =
+   selectFirst("operator", Matches)) {
   assert(E->getNumArgs() > 0);
   auto *Object = E->getArg(0);
   assert(Object != nullptr);
@@ -516,7 +517,11 @@
 Type->getAsCXXRecordDecl()->getQualifiedNameAsString() != "OptionalInt")
   return false;
 
-return Val1.getProperty("has_value") == Val2.getProperty("has_value");
+auto *Prop1  = Val1.getProperty("has_value");
+auto *Prop2 = Val2.getProperty("has_value");
+return Prop1 == Prop2 ||
+   (Prop1 != nullptr && Prop2 != nullptr && isa(Prop1) &&
+isa(Prop2));
   }
 
   bool merge(QualType Type, const Value &Val1, const Environment &Env1,
@@ -538,11 +543,9 @@
 if (HasValue1 == HasValue2)
   MergedVal.setProperty("has_value", *HasValue1);
 else
-  MergedVal.setProperty("has_value", HasValueTop);
+  MergedVal.setProperty("has_value", MergedEnv.makeTopBoolValue());
 return true;
   }
-
-  BoolValue &HasValueTop;
 };
 
 class WideningTest : public Test {
@@ -561,11 +564,8 @@
 checkD

[PATCH] D131701: [CodeGen][ObjC] Call synthesized copy constructor/assignment operator functions in getter/setter functions of non-trivial C struct properties

2022-10-13 Thread John McCall via Phabricator via cfe-commits

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131701

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

[PATCH] D135816: [clang] Support `constexpr` for some `ASTNodeKind` member functions

2022-10-13 Thread Aaron Ballman via Phabricator via cfe-commits

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

LGTM aside from a minor question.




Comment at: clang/include/clang/AST/ASTTypeTraits.h:90-92
   bool operator<(const ASTNodeKind &Other) const {
 return KindId < Other.KindId;
   }

Any reason for this to not be `constexpr` as well?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135816

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

[PATCH] D135801: [clang][Lexer] Speedup HeaderSearch when there are many HeaderMaps

2022-10-13 Thread Troy Johnson via Phabricator via cfe-commits

troyj updated this revision to Diff 467496.
troyj added a comment.

Addressed Bruno's comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135801

Files:
  clang/include/clang/Lex/HeaderMap.h
  clang/include/clang/Lex/HeaderSearch.h
  clang/lib/Lex/HeaderSearch.cpp

Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -116,6 +116,7 @@
   NoCurDirSearch = noCurDirSearch;
   SearchDirToHSEntry = std::move(searchDirToHSEntry);
   //LookupFileCache.clear();
+  indexInitialHeaderMaps();
 }
 
 void HeaderSearch::AddSearchPath(const DirectoryLookup &dir, bool isAngled) {
@@ -372,6 +373,29 @@
   return Module;
 }
 
+void HeaderSearch::indexInitialHeaderMaps() {
+  llvm::StringMap Index(SearchDirs.size());
+
+  // Iterate over all filename keys and associate them with the index i.
+  unsigned i = 0;
+  for (; i != SearchDirs.size(); ++i) {
+auto &Dir = SearchDirs[i];
+
+// We're concerned with only the initial contiguous run of header
+// maps within SearchDirs, which can be 99% of SearchDirs when
+// SearchDirs.size() is ~1.
+if (!Dir.isHeaderMap())
+  break;
+
+// Give earlier keys precedence over identical later keys.
+auto Callback = [&](StringRef Filename) { Index.try_emplace(Filename, i); };
+Dir.getHeaderMap()->forEachKey(Callback);
+  }
+
+  SearchDirHeaderMapIndex = std::move(Index);
+  FirstNonHeaderMapSearchDirIdx = i;
+}
+
 //===--===//
 // File lookup within a DirectoryLookup scope
 //===--===//
@@ -977,24 +1001,37 @@
 
   ConstSearchDirIterator NextIt = std::next(It);
 
-  // If the entry has been previously looked up, the first value will be
-  // non-zero.  If the value is equal to i (the start point of our search), then
-  // this is a matching hit.
-  if (!SkipCache && CacheLookup.StartIt == NextIt) {
-// Skip querying potentially lots of directories for this lookup.
-if (CacheLookup.HitIt)
-  It = CacheLookup.HitIt;
-if (CacheLookup.MappedName) {
-  Filename = CacheLookup.MappedName;
-  if (IsMapped)
-*IsMapped = true;
+  if (!SkipCache) {
+if (CacheLookup.StartIt == NextIt) {
+  // HIT: Skip querying potentially lots of directories for this lookup.
+  if (CacheLookup.HitIt)
+It = CacheLookup.HitIt;
+  if (CacheLookup.MappedName) {
+Filename = CacheLookup.MappedName;
+if (IsMapped)
+  *IsMapped = true;
+  }
+} else {
+  // MISS: This is the first query, or the previous query didn't match
+  // our search start.  We will fill in our found location below, so prime
+  // the start point value.
+  CacheLookup.reset(/*NewStartIt=*/NextIt);
+
+  if (It == search_dir_begin() && FirstNonHeaderMapSearchDirIdx > 0) {
+// Handle cold misses of user includes in the presence of many header
+// maps.  We avoid searching perhaps thousands of header maps by
+// jumping directly to the correct one or jumping beyond all of them.
+auto Iter = SearchDirHeaderMapIndex.find(Filename);
+if (Iter == SearchDirHeaderMapIndex.end())
+  // Not in index => Skip to first SearchDir after initial header maps
+  It = search_dir_nth(FirstNonHeaderMapSearchDirIdx);
+else
+  // In index => Start with a specific header map
+  It = search_dir_nth(Iter->second);
+  }
 }
-  } else {
-// Otherwise, this is the first query, or the previous query didn't match
-// our search start.  We will fill in our found location below, so prime the
-// start point value.
+  } else
 CacheLookup.reset(/*NewStartIt=*/NextIt);
-  }
 
   SmallString<64> MappedName;
 
Index: clang/include/clang/Lex/HeaderSearch.h
===
--- clang/include/clang/Lex/HeaderSearch.h
+++ clang/include/clang/Lex/HeaderSearch.h
@@ -249,6 +249,14 @@
   unsigned SystemDirIdx = 0;
   bool NoCurDirSearch = false;
 
+  /// Maps HeaderMap keys to SearchDir indices. When HeaderMaps are used
+  /// heavily, SearchDirs can start with thousands of HeaderMaps, so this Index
+  /// lets us avoid scanning them all to find a match.
+  llvm::StringMap SearchDirHeaderMapIndex;
+
+  /// The index of the first SearchDir that isn't a header map.
+  unsigned FirstNonHeaderMapSearchDirIdx = 0;
+
   /// \#include prefixes for which the 'system header' property is
   /// overridden.
   ///
@@ -330,6 +338,10 @@
   /// Entity used to look up stored header file information.
   ExternalHeaderFileInfoSource *ExternalSource = nullptr;
 
+  /// Scan all of the header maps at the beginning of SearchDirs and
+  /// map their keys to the SearchDir index of the

[PATCH] D135801: [clang][Lexer] Speedup HeaderSearch when there are many HeaderMaps

2022-10-13 Thread Troy Johnson via Phabricator via cfe-commits

troyj added inline comments.



Comment at: clang/include/clang/Lex/HeaderSearch.h:255
+  /// lets us avoid scanning them all to find a match.
+  llvm::StringMap SearchDirHeaderMapIndex;
+

bruno wrote:
> Did you end up getting data on the memory footprint of this map? It seems to 
> me that it should be fine if header maps aren't much populated.
Right, with ~15000 header maps, this map has ~92000 entries, so each map 
contributes about 6 unique keys. The patch does not increase the peak memory 
usage of Clang.



Comment at: clang/lib/Lex/HeaderSearch.cpp:392
+
+Dir.getHeaderMap()->forEachKey(Callback);
+  }

bruno wrote:
> What happens if different header maps contain the same header name?
> 
The first one sticks because try_emplace doesn't insert if the key already 
exists. I'll add a comment.



Comment at: clang/lib/Lex/HeaderSearch.cpp:1035
   } else {
-// Otherwise, this is the first query, or the previous query didn't match
-// our search start.  We will fill in our found location below, so prime 
the
-// start point value.
 CacheLookup.reset(/*NewStartIt=*/NextIt);
   }

bruno wrote:
> Looks like we could have only one call to `CacheLookup.reset` instead?
The reset call has a side effect of setting CacheLookup.MappedName to nullptr, 
so we can't call it unconditionally because the Hit case should not call it. 
The unpatched code combined the !SkipCache check and the Hit check, so it still 
called reset when SkipCache == true. I preserved that behavior because I think 
it's necessary. SkipCache seems to imply "don't consult the cache for this 
call" instead of "don't cache anything" so it still caches the result for a 
future LookupFile call that passes SkipCache == false.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135801

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

[PATCH] D129755: Thread safety analysis: Support copy-elided production of scoped capabilities through arbitrary calls

2022-10-13 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added a comment.

In D129755#3853904 , @thesamesam 
wrote:

> In D129755#3853809 , @aaronpuchert 
> wrote:
>
>> @aaron.ballman, would like some feedback on the release notes. Should I 
>> additionally write something under "Potentially Breaking Changes", or is it 
>> enough to mention this under "Improvements to Clang's diagnostics"? Though I 
>> guess we could also add this later on if we get more complaints that this 
>> breaks things.
>
> I think it's sufficient in "Improvements to Clang's diagnostics". We're not 
> looking to document every externally observable change in "Potentially 
> Breaking Changes", IMO. Thanks for asking. We could definitely shift it later 
> if needed.

+1 to this -- I think it's defensible either way. It's not that this is a 
*breaking* change, but it is a potentially disruptive one. If we get many more 
folks raising questions about code changes, we can move the release note then. 
I think what you have now is fine by me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129755

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

[PATCH] D135885: [clang][LTO][NFC] Adding More Tests for AIX Options

2022-10-13 Thread Qiongsi Wu via Phabricator via cfe-commits

qiongsiwu1 created this revision.
qiongsiwu1 added reviewers: MaskRay, w2yehia, daltenty.
qiongsiwu1 added a project: clang.
Herald added subscribers: ormris, StephenFan, steven_wu, hiraditya, inglorion.
Herald added a project: All.
qiongsiwu1 requested review of this revision.
Herald added a subscriber: cfe-commits.

This patch adds more tests for AIX. It follows https://reviews.llvm.org/D134820 
which added a minimal set of tests for the newly added AIX options. These new 
tests were originally created by https://reviews.llvm.org/D119109. Since we do 
not plan to land https://reviews.llvm.org/D119109 in its current shape to add 
the AIX specific options, we incorporate the relevant tests developed.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D135885

Files:
  clang/test/Driver/lto-aix.c


Index: clang/test/Driver/lto-aix.c
===
--- clang/test/Driver/lto-aix.c
+++ clang/test/Driver/lto-aix.c
@@ -4,3 +4,32 @@
 //
 // LTOPATH: "-bplugin:{{.*}}libLTO.{{so|dll|dylib}}"
 // MCPUOPTLEVEL: "-bplugin_opt:-mcpu={{.*}}" "-bplugin_opt:-O3"
+//
+// More opt level option tests
+// RUN: %clang --target=powerpc-ibm-aix --sysroot %S/Inputs/aix_ppc_tree %s \
+// RUN:   -fuse-ld=ld -flto -O -### 2>&1 | FileCheck --check-prefix=O1 %s
+// RUN: %clang --target=powerpc-ibm-aix --sysroot %S/Inputs/aix_ppc_tree %s \
+// RUN:   -fuse-ld=ld -flto -O1 -### 2>&1 | FileCheck --check-prefix=O1 %s
+// RUN: %clang --target=powerpc-ibm-aix --sysroot %S/Inputs/aix_ppc_tree %s \
+// RUN:   -fuse-ld=ld -flto -Og -### 2>&1 | FileCheck --check-prefix=O1 %s
+// RUN: %clang --target=powerpc-ibm-aix --sysroot %S/Inputs/aix_ppc_tree %s \
+// RUN:   -fuse-ld=ld -flto -O2 -### 2>&1 | FileCheck --check-prefix=O2 %s
+// RUN: %clang --target=powerpc-ibm-aix --sysroot %S/Inputs/aix_ppc_tree %s \
+// RUN:   -fuse-ld=ld -flto -Os -### 2>&1 | FileCheck --check-prefix=O2 %s
+// RUN: %clang --target=powerpc-ibm-aix --sysroot %S/Inputs/aix_ppc_tree %s \
+// RUN:   -fuse-ld=ld -flto -Oz -### 2>&1 | FileCheck --check-prefix=O2 %s
+// RUN: %clang --target=powerpc-ibm-aix --sysroot %S/Inputs/aix_ppc_tree %s \
+// RUN:   -fuse-ld=ld -flto -O3 -### 2>&1 | FileCheck --check-prefix=O3 %s
+// RUN: %clang --target=powerpc-ibm-aix --sysroot %S/Inputs/aix_ppc_tree %s \
+// RUN:   -fuse-ld=ld -flto -Ofast -### 2>&1 | FileCheck --check-prefix=O3 %s
+// O1: "-bplugin_opt:-O1"
+// O2: "-bplugin_opt:-O2"
+// O3: "-bplugin_opt:-O3"
+//
+// Test debugging options
+// RUN: %clang --target=powerpc-ibm-aix -### %s -flto -fuse-ld=ld -gdbx 2> %t
+// RUN: FileCheck -check-prefix=CHECK-TUNING-DBX < %t %s
+// RUN: %clang --target=powerpc-ibm-aix -### %s -flto -fuse-ld=ld -g 2> %t
+// RUN: FileCheck -check-prefix=CHECK-NO-TUNING < %t %s
+// CHECK-TUNING-DBX:"-bplugin_opt:-debugger-tune=dbx"
+// CHECK-NO-TUNING-NOT: "{{-plugin-opt=|-bplugin_opt:}}-debugger-tune"


Index: clang/test/Driver/lto-aix.c
===
--- clang/test/Driver/lto-aix.c
+++ clang/test/Driver/lto-aix.c
@@ -4,3 +4,32 @@
 //
 // LTOPATH: "-bplugin:{{.*}}libLTO.{{so|dll|dylib}}"
 // MCPUOPTLEVEL: "-bplugin_opt:-mcpu={{.*}}" "-bplugin_opt:-O3"
+//
+// More opt level option tests
+// RUN: %clang --target=powerpc-ibm-aix --sysroot %S/Inputs/aix_ppc_tree %s \
+// RUN:   -fuse-ld=ld -flto -O -### 2>&1 | FileCheck --check-prefix=O1 %s
+// RUN: %clang --target=powerpc-ibm-aix --sysroot %S/Inputs/aix_ppc_tree %s \
+// RUN:   -fuse-ld=ld -flto -O1 -### 2>&1 | FileCheck --check-prefix=O1 %s
+// RUN: %clang --target=powerpc-ibm-aix --sysroot %S/Inputs/aix_ppc_tree %s \
+// RUN:   -fuse-ld=ld -flto -Og -### 2>&1 | FileCheck --check-prefix=O1 %s
+// RUN: %clang --target=powerpc-ibm-aix --sysroot %S/Inputs/aix_ppc_tree %s \
+// RUN:   -fuse-ld=ld -flto -O2 -### 2>&1 | FileCheck --check-prefix=O2 %s
+// RUN: %clang --target=powerpc-ibm-aix --sysroot %S/Inputs/aix_ppc_tree %s \
+// RUN:   -fuse-ld=ld -flto -Os -### 2>&1 | FileCheck --check-prefix=O2 %s
+// RUN: %clang --target=powerpc-ibm-aix --sysroot %S/Inputs/aix_ppc_tree %s \
+// RUN:   -fuse-ld=ld -flto -Oz -### 2>&1 | FileCheck --check-prefix=O2 %s
+// RUN: %clang --target=powerpc-ibm-aix --sysroot %S/Inputs/aix_ppc_tree %s \
+// RUN:   -fuse-ld=ld -flto -O3 -### 2>&1 | FileCheck --check-prefix=O3 %s
+// RUN: %clang --target=powerpc-ibm-aix --sysroot %S/Inputs/aix_ppc_tree %s \
+// RUN:   -fuse-ld=ld -flto -Ofast -### 2>&1 | FileCheck --check-prefix=O3 %s
+// O1: "-bplugin_opt:-O1"
+// O2: "-bplugin_opt:-O2"
+// O3: "-bplugin_opt:-O3"
+//
+// Test debugging options
+// RUN: %clang --target=powerpc-ibm-aix -### %s -flto -fuse-ld=ld -gdbx 2> %t
+// RUN: FileCheck -check-prefix=CHECK-TUNING-DBX < %t %s
+// RUN: %clang --target=powerpc-ibm-aix -### %s -flto -fuse-ld=ld -g 2> %t
+// RUN: FileCheck -check-prefix=CHECK-NO-TUNING < %t %s
+// CHECK-TUNING-DBX:"-bplugin_opt:-debugger-tune=dbx"
+// CHECK-NO-TUNING-NOT: "{{-plugin-opt=|-bplugin

[PATCH] D135384: [AIX] Enable the use of the -pg flag

2022-10-13 Thread Michael Francis via Phabricator via cfe-commits

francii updated this revision to Diff 467491.
francii added a comment.

Updated check for Func name


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135384

Files:
  clang/lib/Basic/Targets/OSTargets.h
  clang/test/CodeGen/mcount-aix.c
  llvm/lib/Transforms/Utils/EntryExitInstrumenter.cpp


Index: llvm/lib/Transforms/Utils/EntryExitInstrumenter.cpp
===
--- llvm/lib/Transforms/Utils/EntryExitInstrumenter.cpp
+++ llvm/lib/Transforms/Utils/EntryExitInstrumenter.cpp
@@ -7,6 +7,7 @@
 
//===--===//
 
 #include "llvm/Transforms/Utils/EntryExitInstrumenter.h"
+#include "llvm/ADT/Triple.h"
 #include "llvm/Analysis/GlobalsModRef.h"
 #include "llvm/IR/DebugInfoMetadata.h"
 #include "llvm/IR/Dominators.h"
@@ -34,9 +35,24 @@
   Func == "__mcount" ||
   Func == "_mcount" ||
   Func == "__cyg_profile_func_enter_bare") {
-FunctionCallee Fn = M.getOrInsertFunction(Func, Type::getVoidTy(C));
-CallInst *Call = CallInst::Create(Fn, "", InsertionPt);
-Call->setDebugLoc(DL);
+Triple TargetTriple(M.getTargetTriple());
+if (TargetTriple.isOSAIX() && Func == "__mcount") {
+  Type *SizeTy = M.getDataLayout().getIntPtrType(C);
+  Type *SizePtrTy = SizeTy->getPointerTo();
+  GlobalVariable *GV = new GlobalVariable(M, SizeTy, /*isConstant=*/false,
+  GlobalValue::InternalLinkage,
+  ConstantInt::get(SizeTy, 0));
+  CallInst *Call = CallInst::Create(
+  M.getOrInsertFunction(Func,
+FunctionType::get(Type::getVoidTy(C), 
{SizePtrTy},
+  /*isVarArg=*/false)),
+  {GV}, "", InsertionPt);
+  Call->setDebugLoc(DL);
+} else {
+  FunctionCallee Fn = M.getOrInsertFunction(Func, Type::getVoidTy(C));
+  CallInst *Call = CallInst::Create(Fn, "", InsertionPt);
+  Call->setDebugLoc(DL);
+}
 return;
   }
 
Index: clang/test/CodeGen/mcount-aix.c
===
--- /dev/null
+++ clang/test/CodeGen/mcount-aix.c
@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 -pg -triple powerpc-ibm-aix7.2.0.0 -S -emit-llvm %s -o - | 
FileCheck %s
+// RUN: %clang_cc1 -pg -triple powerpc64-ibm-aix7.2.0.0 -S -emit-llvm %s -o - 
| FileCheck %s -check-prefix=CHECK64
+
+void foo() {
+}
+
+void bar() {
+foo();
+}
+// CHECK: @[[GLOB0:[0-9]+]] = internal global i32 0
+// CHECK: @[[GLOB1:[0-9]+]] = internal global i32 0
+// CHECK64: @[[GLOB0:[0-9]+]] = internal global i64 0
+// CHECK64: @[[GLOB1:[0-9]+]] = internal global i64 0
+// CHECK-LABEL: @foo(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:call void @__mcount(ptr @[[GLOB0]])
+// CHECK64-LABEL: @foo(
+// CHECK64-NEXT:  entry:
+// CHECK64-NEXT:call void @__mcount(ptr @[[GLOB0]])
+// CHECK-LABEL: @bar(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:call void @__mcount(ptr @[[GLOB1]])
+// CHECK64-LABEL: @bar(
+// CHECK64-NEXT:  entry:
+// CHECK64-NEXT:call void @__mcount(ptr @[[GLOB1]])
Index: clang/lib/Basic/Targets/OSTargets.h
===
--- clang/lib/Basic/Targets/OSTargets.h
+++ clang/lib/Basic/Targets/OSTargets.h
@@ -751,6 +751,7 @@
 public:
   AIXTargetInfo(const llvm::Triple &Triple, const TargetOptions &Opts)
   : OSTargetInfo(Triple, Opts) {
+this->MCountName = "__mcount";
 this->TheCXXABI.set(TargetCXXABI::XL);
 
 if (this->PointerWidth == 64) {


Index: llvm/lib/Transforms/Utils/EntryExitInstrumenter.cpp
===
--- llvm/lib/Transforms/Utils/EntryExitInstrumenter.cpp
+++ llvm/lib/Transforms/Utils/EntryExitInstrumenter.cpp
@@ -7,6 +7,7 @@
 //===--===//
 
 #include "llvm/Transforms/Utils/EntryExitInstrumenter.h"
+#include "llvm/ADT/Triple.h"
 #include "llvm/Analysis/GlobalsModRef.h"
 #include "llvm/IR/DebugInfoMetadata.h"
 #include "llvm/IR/Dominators.h"
@@ -34,9 +35,24 @@
   Func == "__mcount" ||
   Func == "_mcount" ||
   Func == "__cyg_profile_func_enter_bare") {
-FunctionCallee Fn = M.getOrInsertFunction(Func, Type::getVoidTy(C));
-CallInst *Call = CallInst::Create(Fn, "", InsertionPt);
-Call->setDebugLoc(DL);
+Triple TargetTriple(M.getTargetTriple());
+if (TargetTriple.isOSAIX() && Func == "__mcount") {
+  Type *SizeTy = M.getDataLayout().getIntPtrType(C);
+  Type *SizePtrTy = SizeTy->getPointerTo();
+  GlobalVariable *GV = new GlobalVariable(M, SizeTy, /*isConstant=*/false,
+  GlobalValue::InternalLinkage,
+  ConstantInt::get(SizeTy, 0));
+  CallInst *Call = CallInst::

[PATCH] D135472: [ODRHash] Hash attributes on declarations.

2022-10-13 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added inline comments.



Comment at: clang/lib/AST/ODRHash.cpp:479-480
+
+  llvm::copy_if(D->attrs(), std::back_inserter(HashableAttrs),
+[](const Attr *A) { return !A->isImplicit(); });
+}

ChuanqiXu wrote:
> aaron.ballman wrote:
> > ChuanqiXu wrote:
> > > vsapsai wrote:
> > > > vsapsai wrote:
> > > > > erichkeane wrote:
> > > > > > aaron.ballman wrote:
> > > > > > > ChuanqiXu wrote:
> > > > > > > > vsapsai wrote:
> > > > > > > > > I'm not sure `isImplicit` is the best indicator of attributes 
> > > > > > > > > to check, so suggestions in this area are welcome. I think we 
> > > > > > > > > can start strict and relax some of the checks if needed.
> > > > > > > > > 
> > > > > > > > > If people have strong opinions that some attributes shouldn't 
> > > > > > > > > be ignored, we can add them to the tests to avoid 
> > > > > > > > > regressions. Personally, I believe that alignment and packed 
> > > > > > > > > attributes should never be silently ignored.
> > > > > > > > Agreed. I feel `isImplicit` is enough for now.
> > > > > > > The tricky part is -- sometimes certain attributes add additional 
> > > > > > > implicit attributes and those implicit attributes matter 
> > > > > > > (https://github.com/llvm/llvm-project/blob/main/clang/lib/Sema/SemaDeclAttr.cpp#L9380).
> > > > > > >  And some attributes seem to just do the wrong thing entirely: 
> > > > > > > https://github.com/llvm/llvm-project/blob/main/clang/lib/Sema/SemaDeclAttr.cpp#L7344
> > > > > > > 
> > > > > > > So I think `isImplicit()` is a good approximation, but I'm more 
> > > > > > > wondering what the principle is for whether an attribute should 
> > > > > > > or should not be considered part of the ODR hash. Type 
> > > > > > > attributes, attributes that impact object layout, etc all seem 
> > > > > > > like they should almost certainly be part of ODR hashing. Others 
> > > > > > > are a bit more questionable though.
> > > > > > > 
> > > > > > > I think this is something that may need a per-attribute flag in 
> > > > > > > Attr.td so attributes can opt in or out of it because I'm not 
> > > > > > > certain what ODR issues could stem from `[[maybe_unused]]` or 
> > > > > > > `[[deprecated]]` disagreements across module boundaries.
> > > > > > I don't think 'isImplicit' is particularly good.  I think the idea 
> > > > > > of auto-adding 'type' attributes and having the 'rest' be analyzed 
> > > > > > to figure out which are important.
> > > > > > 
> > > > > > Alternatively, I wonder if we're better off just adding ALL 
> > > > > > attributes and seeing what the fallout is. We can later decide when 
> > > > > > we don't want them to be ODR significant (which, might be OTHERWISE 
> > > > > > meaningful later!).
> > > > > One criteria to decide which attributes should be hashed is if they 
> > > > > affect IRGen. But that's not exhaustive and I'm not sure how 
> > > > > practical it is.
> > > > > 
> > > > > The rule I'm trying to follow right now is if declarations with 
> > > > > different attributes can be substituted instead of each other. For 
> > > > > example, structs with different memory layout cannot be interchanged, 
> > > > > so it is reasonable to reject them.
> > > > > 
> > > > > But maybe we should review attributes on case-by-case basis. For 
> > > > > example, for `[[deprecated]]` I think the best for developers is not 
> > > > > to complain about it but merge the attributes and then have regular 
> > > > > diagnostic about using a deprecated entity.
> > > > One option was to hash `isa` attributes as they are 
> > > > "sticky" and should be more significant, so shouldn't be ignored. But I 
> > > > don't think this argument is particularly convincing.
> > > > 
> > > > At this stage I think we can still afford to add all attributes and 
> > > > exclude some as-needed because modules adoption is still limited. 
> > > > Though I don't have strong feelings about this approach, just getting 
> > > > more restrictive later can be hard.
> > > It looks not a bad idea to add all attributes as experiments, @vsapsai 
> > > how do you feel about this?
> > My intuition is that we're going to want this to be controlled from Attr.td 
> > on a case-by-case basis and to automatically generate the ODR hash code for 
> > attribute arguments.
> > 
> > We can either force every attribute to decide explicitly (this seems pretty 
> > disruptive as a final design but would be a really good way to ensure we 
> > audit all attributes if done as an experiment), or we can pick a default 
> > behavior to ODR hash/not. I suspect we're going to want to pick a default 
> > behavior based on whatever the audit tells us the most common situation is.
> > 
> > I think we're going to need something a bit more nuanced than "this 
> > attribute matters for ODR" because there are attribute arguments that don't 
> > always contribute to the entity (for example, we have "fake" arguments that 
> > are used to giv

[PATCH] D134878: Update developer policy on potentially breaking changes

2022-10-13 Thread Aaron Ballman via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG8673444598be: Update developer policy on potentially 
breaking changes (authored by aaron.ballman).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134878

Files:
  llvm/docs/DeveloperPolicy.rst


Index: llvm/docs/DeveloperPolicy.rst
===
--- llvm/docs/DeveloperPolicy.rst
+++ llvm/docs/DeveloperPolicy.rst
@@ -104,6 +104,51 @@
 software. Please see :doc:`CodeReview` for more information on LLVM's 
code-review
 process.
 
+.. _breaking:
+
+Making Potentially Breaking Changes
+---
+
+Please help notify users and vendors of potential disruptions when upgrading to
+a newer version of a tool. For example, deprecating a feature that is expected
+to be removed in the future, removing an already-deprecated feature, upgrading 
a
+diagnostic from a warning to an error, switching important default behavior, or
+any other potentially disruptive situation thought to be worth raising
+awareness of. For such changes, the following should be done:
+
+* When performing the code review for the change, please add any applicable
+  "vendors" group to the review for their awareness. The purpose of these
+  groups is to give vendors early notice that potentially disruptive changes
+  are being considered but have not yet been accepted. Vendors can give early
+  testing feedback on the changes to alert us to unacceptable breakages. The
+  current list of vendor groups is:
+
+  * `Clang vendors `_
+  * `libc++ vendors `_
+
+  People interested in joining the vendors group can do so by clicking the
+  "Join Project" link on the vendor's "Members" page in Phabricator.
+
+* When committing the change to the repository, add appropriate information
+  about the potentially breaking changes to the ``Potentially Breaking 
Changes``
+  section of the project's release notes. The release note should have
+  information about what the change is, what is potentially disruptive about
+  it, as well as any code examples, links, and motivation that is appropriate
+  to share with users. This helps users to learn about potential issues with
+  upgrading to that release.
+
+* After the change has been committed to the repository, the potentially
+  disruptive changes described in the release notes should be posted to the
+  `Announcements `_ channel on
+  Discourse. The post should be tagged with the ``potentially-breaking`` label
+  and a label specific to the project (such as ``clang``, ``llvm``, etc). This
+  is another mechanism by which we can give pre-release notice to users about
+  potentially disruptive changes. It is a lower-traffic alternative to the
+  joining "vendors" group. To automatically be notified of new announcements
+  with the ``potentially-breaking`` label, go to your user preferences page in
+  Discourse, and add the label to one of the watch categories under
+  ``Notifications->Tags``.
+
 .. _code owners:
 
 Code Owners
@@ -181,7 +226,12 @@
   programming paradigms.
 * Modifying a C stable API.
 * Notifying users about a potentially disruptive change expected to be made in
-  a future release, such as removal of a deprecated feature.
+  a future release, such as removal of a deprecated feature. In this case, the
+  release note should be added to a ``Potentially Breaking Changes`` section of
+  the notes with sufficient information and examples to demonstrate the
+  potential disruption. Additionally, any new entries to this section should be
+  announced in the `Announcements `_
+  channel on Discourse. See :ref:`breaking` for more details.
 
 Code reviewers are encouraged to request a release note if they think one is
 warranted when performing a code review.


Index: llvm/docs/DeveloperPolicy.rst
===
--- llvm/docs/DeveloperPolicy.rst
+++ llvm/docs/DeveloperPolicy.rst
@@ -104,6 +104,51 @@
 software. Please see :doc:`CodeReview` for more information on LLVM's code-review
 process.
 
+.. _breaking:
+
+Making Potentially Breaking Changes
+---
+
+Please help notify users and vendors of potential disruptions when upgrading to
+a newer version of a tool. For example, deprecating a feature that is expected
+to be removed in the future, removing an already-deprecated feature, upgrading a
+diagnostic from a warning to an error, switching important default behavior, or
+any other potentially disruptive situation thought to be worth raising
+awareness of. For such changes, the following should be done:
+
+* When performing the code review for the change, please add any applicable
+  "vendors" grou

[PATCH] D135820: [clang-tblgen] renames Diagnostic.Text to Diagnostic.Summary

2022-10-13 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added a comment.

Please be sure to add `NFC` to the commit message so it's clear why there's no 
tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135820

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

[PATCH] D135820: [clang-tblgen] renames Diagnostic.Text to Diagnostic.Summary

2022-10-13 Thread Aaron Ballman via Phabricator via cfe-commits

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

LGTM as well!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135820

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

[PATCH] D135097: Remove redundant option '-menable-unsafe-fp-math'.

2022-10-13 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added inline comments.



Comment at: clang/docs/ReleaseNotes.rst:410-412
+- The driver option ``-menable-unsafe-fp-math`` has been removed. Passing it, 
will
+result in a hard error. To enable unsafe floating-point optimizations, the 
compiler
+options ``-funsafe-math-optimizations`` and ``-ffast-math`` are used instead.

zahiraam wrote:
> aaron.ballman wrote:
> > Because we diagnose unknown driver flags as an error 
> > (https://godbolt.org/z/4xjzKh4Ej) and there's no deprecation period, I 
> > think we should put this under the potentially breaking changes section. In 
> > this case, I'm specifically worried about proprietary projects using the 
> > flag for optimization purposes (a lot of numerical analysis code is behind 
> > closed doors).
> > 
> > CC @MaskRay just to make sure there's agreement (we're still trying to 
> > figure out what constitutes a breaking change we want to be loud about in 
> > terms of driver flags).
> > 
> > Assuming Fangrui doesn't disagree, once this lands, please post an 
> > announcement about it into https://discourse.llvm.org/c/announce/46 with 
> > the `clang` and `potentially-breaking` tags (an example of such a post is: 
> > https://discourse.llvm.org/t/clang-16-notice-of-potentially-breaking-changes/65562/
> >  though you wouldn't need all that lead-in text). 
> @aaron.ballman would it be worth adding a diagnostic for the option we are 
> removing?
If we're going to deprecate rather than remove, sure. But I think we're okay 
removing, and I think the default error diagnostic behavior will be sufficient.


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

https://reviews.llvm.org/D135097

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

[PATCH] D135097: Remove redundant option '-menable-unsafe-fp-math'.

2022-10-13 Thread Zahira Ammarguellat via Phabricator via cfe-commits

zahiraam added inline comments.



Comment at: clang/docs/ReleaseNotes.rst:410-412
+- The driver option ``-menable-unsafe-fp-math`` has been removed. Passing it, 
will
+result in a hard error. To enable unsafe floating-point optimizations, the 
compiler
+options ``-funsafe-math-optimizations`` and ``-ffast-math`` are used instead.

aaron.ballman wrote:
> Because we diagnose unknown driver flags as an error 
> (https://godbolt.org/z/4xjzKh4Ej) and there's no deprecation period, I think 
> we should put this under the potentially breaking changes section. In this 
> case, I'm specifically worried about proprietary projects using the flag for 
> optimization purposes (a lot of numerical analysis code is behind closed 
> doors).
> 
> CC @MaskRay just to make sure there's agreement (we're still trying to figure 
> out what constitutes a breaking change we want to be loud about in terms of 
> driver flags).
> 
> Assuming Fangrui doesn't disagree, once this lands, please post an 
> announcement about it into https://discourse.llvm.org/c/announce/46 with the 
> `clang` and `potentially-breaking` tags (an example of such a post is: 
> https://discourse.llvm.org/t/clang-16-notice-of-potentially-breaking-changes/65562/
>  though you wouldn't need all that lead-in text). 
@aaron.ballman would it be worth adding a diagnostic for the option we are 
removing?


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

https://reviews.llvm.org/D135097

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

[PATCH] D135680: [clang][ARM] follow GCC behavior for defining SOFTFP

2022-10-13 Thread Lucas Prates via Phabricator via cfe-commits

pratlucas added inline comments.



Comment at: clang/test/Preprocessor/init-arm.c:404
+//   is specified
+// RUN: %clang_cc1 -E -dM -ffreestanding -triple=arm-none-linux-gnueabi 
-target-feature +soft-float -target-feature +soft-float-abi < /dev/null | 
FileCheck -match-full-lines -check-prefix ARMEABISOFTFP_NOFP %s
 //

I believe this RUN line won't covered the new condition added to 
`clang/lib/Basic/Targets/ARM.cpp` above, as it sets `+soft-float`.
Can you add an extra test without that option enabled?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135680

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

[PATCH] D135097: Remove redundant option '-menable-unsafe-fp-math'.

2022-10-13 Thread Paul Robinson via Phabricator via cfe-commits

probinson added subscribers: wristow, probinson.
probinson added a comment.

+wristow who has been tracking this kind of stuff for Sony.


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

https://reviews.llvm.org/D135097

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

[PATCH] D135551: [clang] replace `assert(0)` with `llvm_unreachable` NFC

2022-10-13 Thread Paul Robinson via Phabricator via cfe-commits

probinson added a comment.

dblaikie:

> In the same way that an assert says "This condition should never be false" - 
> I use "should" in the same sense in both unreachable and assert, and I 
> believe that's the prevailing opinion of LLVM developers/the LLVM style guide.

aaronballman:

> I believe our code base says something different than our "prevailing 
> opinion" and we should not be discounting the reality of how our code is 
> written today.

So, there are lots of ways and places where "the reality of how our code is 
written today" fails to conform to the style guide.
One conclusion is that the style guide is wrong and should be changed to 
reflect reality.
Another conclusion is that conforming to the style guide is the goal and one 
aspect of development is to strive to reach the goal. The imperfection of the 
state of the code today merely reflects an imperfect understanding of the goal.

I suggest a Round Table at the (less than one month away) Dev Meeting, assuming 
that the active parties in this discussion will be there. Put off any RFC until 
after that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135551

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

[PATCH] D135097: Remove redundant option '-menable-unsafe-fp-math'.

2022-10-13 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added a reviewer: clang-vendors.
aaron.ballman added a comment.

Adding clang-vendors because of the potential for disruption from the changes.




Comment at: clang/docs/ReleaseNotes.rst:410-412
+- The driver option ``-menable-unsafe-fp-math`` has been removed. Passing it, 
will
+result in a hard error. To enable unsafe floating-point optimizations, the 
compiler
+options ``-funsafe-math-optimizations`` and ``-ffast-math`` are used instead.

Because we diagnose unknown driver flags as an error 
(https://godbolt.org/z/4xjzKh4Ej) and there's no deprecation period, I think we 
should put this under the potentially breaking changes section. In this case, 
I'm specifically worried about proprietary projects using the flag for 
optimization purposes (a lot of numerical analysis code is behind closed doors).

CC @MaskRay just to make sure there's agreement (we're still trying to figure 
out what constitutes a breaking change we want to be loud about in terms of 
driver flags).

Assuming Fangrui doesn't disagree, once this lands, please post an announcement 
about it into https://discourse.llvm.org/c/announce/46 with the `clang` and 
`potentially-breaking` tags (an example of such a post is: 
https://discourse.llvm.org/t/clang-16-notice-of-potentially-breaking-changes/65562/
 though you wouldn't need all that lead-in text). 


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

https://reviews.llvm.org/D135097

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

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

2022-10-13 Thread Erich Keane via Phabricator via cfe-commits

erichkeane added a comment.

In D126907#382 , @erichkeane 
wrote:

> In D126907#3854983 , @cor3ntin 
> wrote:
>
>> @erichkeane I was looking at Daisy meta programming shenanigans and found 
>> this crashes https://cute.godbolt.org/z/6fWoEheKc - seems related to your 
>> work, probably? It used to work in Clang 15. I thought you might want to 
>> know!
>
> Thanks!  I'll take a look.  That DOES look like some other failures I've had 
> with this patch.

I've reduced it a bit to remove the STL stuff, but it looks like the 
template-lambda is a part of it: https://cute.godbolt.org/z/Prh6xsesz

It manages to hit quite a few things that I suspect the 
`getTemplateInstantiationArgs` doesn't get right, so I'm guessing the 'fix' is 
going to be in there somewhere.  I'll start taking a look this week.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126907

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

[PATCH] D134128: Resubmit an implemention for constrained template template parameters [P0857R0 Part B]

2022-10-13 Thread Erich Keane via Phabricator via cfe-commits

erichkeane added inline comments.



Comment at: clang/lib/Sema/SemaConcept.cpp:590
 
+// FIXME: may be incomplete
+static unsigned CalculateTemplateDepthForConstraints(const Expr *Constr) {

lime wrote:
> erichkeane wrote:
> > I'd like some sort of assert in the case where you don't know what to do 
> > here.  We need to collect all of these cases best we can in advance.
> > 
> > I'd suggest ALSO running libcxx tests against this once you have the assert 
> > in place, which should help.
> I'm afraid that assertions would abort Clang on many code. This function 
> serves as a check for `AdjustConstraintDepth`, and this class looks like just 
> handling `TemplateTypeParmType`. The function has already handled 
> `TemplateTypeParmType`. And determining cases that should trigger the 
> assertions also involves the subsumption of atomic constraints. It is needed 
> to check the depth according to `TemplateTypeParmType`, because the identity 
> of `QualType` involves the depth, and then effects the subsumption. There 
> will be some works to determine which cases are the same, and leave 
> assertions for the cases. This kind of works is highly related to the depth 
> and the subsumption, while I think it is less related to template template 
> parameters.
The point of an assertion would be to do just that!  We WANT to make sure we 
get all those assertions.  That said, I'm unconvinced that this code here is 
necessary.



Comment at: clang/lib/Sema/SemaConcept.cpp:706
+  std::tie(OldConstr, NewConstr) =
+  AdjustConstraintDepth::UnifyConstraintDepthFromDecl(*this, Old, 
OldConstr,
+  New, NewConstr);

lime wrote:
> erichkeane wrote:
> > lime wrote:
> > > erichkeane wrote:
> > > > Unless I'm missing something, I don't think this idea of 'unify 
> > > > constraint depth' is correct.  We should be able, from the decl itself, 
> > > > to determine the depths?  What is the difference here that I'm not 
> > > > getting?
> > > 3. I extracted the calls to the original 
> > > `CalculateTemplateDepthForConstraints` as `UnifyConstraintDepthFromDecl`. 
> > > Calculating from a declaration is indeed not accurate, as it returns 1 
> > > for `template class>`, but the depth in the constraint is 
> > > actually 0. This is one reason why a previous version of patch breaks 
> > > some unit tests. But I leaves it to keep the code unchanged functionally.
> > Hmm... SO it seems based on debugging that the issue is exclusive with 
> > template template parameters.  It DOES make sense that the inner-template 
> > is 'deeper', but I think the problem is that `getTemplateInstantiationArgs` 
> > isn't correctly handling a ClassTemplateDecl, so the "D2" in 
> > `IsAtLeastAsConstrainedAs` is returning 0, instead of 1.
> > 
> > The "Depth" of something is the 'number of layers of template arguments'.  
> > So for: 'template class>', the answer SHOULD be 1.
> > 
> > So I think the problem is basically that for X, the answer should ALSO be 1 
> > (since it has template parameters), but we aren't handling it.  So I think 
> > we just need to correctly just call the 
> > `ClassTemplateDecl::getTemplatedDecl` at one point on that one.
> I guess there could be another patch improving the calculation about depth. 
> In this patch, I just adjust the calculation to pass unit tests about 
> template template parameters.
Right, but I'm saying how you're doing it is incorrect and will likely result 
in incorrect compilations


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134128

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

[PATCH] D132131: [clang-format] Adds a formatter for aligning trailing comments over empty lines

2022-10-13 Thread Yusuke Kadowaki via Phabricator via cfe-commits

yusuke-kadowaki updated this revision to Diff 467458.
yusuke-kadowaki marked 3 inline comments as done.
yusuke-kadowaki added a comment.

Added more tests
Removed braces


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132131

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/WhitespaceManager.cpp
  clang/unittests/Format/FormatTest.cpp
  clang/unittests/Format/FormatTestComments.cpp

Index: clang/unittests/Format/FormatTestComments.cpp
===
--- clang/unittests/Format/FormatTestComments.cpp
+++ clang/unittests/Format/FormatTestComments.cpp
@@ -2858,6 +2858,224 @@
"int a; //\n");
 }
 
+TEST_F(FormatTestComments, AlignTrailingCommentsAcrossEmptyLines) {
+  FormatStyle Style = getLLVMStyle();
+  Style.AlignTrailingComments.Kind = FormatStyle::TCAS_Always;
+  Style.AlignTrailingComments.OverEmptyLines = 1;
+  verifyFormat("#include \"a.h\"  // simple\n"
+   "\n"
+   "#include \"aa.h\" // example case\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"   // align across\n"
+   "\n"
+   "#include \"aa.h\"  // two empty lines\n"
+   "\n"
+   "#include \"aaa.h\" // in a row\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"  // align\n"
+   "#include \"aa.h\" // comment\n"
+   "#include \"aaa.h\"// blocks\n"
+   "\n"
+   "#include \".h\"   // across\n"
+   "#include \"a.h\"  // one\n"
+   "#include \"aa.h\" // empty line\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"  // align trailing comments\n"
+   "#include \"a.h\"\n"
+   "#include \"aa.h\" // across a line without comment\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"   // align across\n"
+   "#include \"a.h\"\n"
+   "#include \"aa.h\"  // two lines without comment\n"
+   "#include \"a.h\"\n"
+   "#include \"aaa.h\" // in a row\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"  // align\n"
+   "#include \"aa.h\" // comment\n"
+   "#include \"aaa.h\"// blocks\n"
+   "#include \"a.h\"\n"
+   "#include \".h\"   // across\n"
+   "#include \"a.h\"  // a line without\n"
+   "#include \"aa.h\" // comment\n",
+   Style);
+
+  // Start of testing OverEmptyLines
+  Style.MaxEmptyLinesToKeep = 3;
+  Style.AlignTrailingComments.OverEmptyLines = 2;
+  // Cannot use verifyFormat here
+  // test::messUp removes all new lines which changes the logic
+  EXPECT_EQ("#include \"a.h\" // comment\n"
+"\n"
+"\n"
+"\n"
+"#include \"ab.h\"  // comment\n"
+"\n"
+"\n"
+"#include \"abcdefg.h\" // comment\n",
+format("#include \"a.h\" // comment\n"
+   "\n"
+   "\n"
+   "\n"
+   "#include \"ab.h\" // comment\n"
+   "\n"
+   "\n"
+   "#include \"abcdefg.h\" // comment\n",
+   Style));
+
+  Style.MaxEmptyLinesToKeep = 1;
+  Style.AlignTrailingComments.OverEmptyLines = 1;
+  // End of testing OverEmptyLines
+
+  Style.ColumnLimit = 15;
+  EXPECT_EQ("int ab; // line\n"
+"int a;  // long\n"
+"// long\n"
+"\n"
+"// long",
+format("int ab; // line\n"
+   "int a; // long long\n"
+   "\n"
+   "// long",
+   Style));
+
+  Style.ColumnLimit = 15;
+  EXPECT_EQ("int ab; // line\n"
+"\n"
+"int a;  // long\n"
+"// long\n",
+format("int ab; // line\n"
+   "\n"
+   "int a; // long long\n",
+   Style));
+
+  Style.ColumnLimit = 30;
+  EXPECT_EQ("int foo = 12345; // comment\n"
+"int bar =\n"
+"1234;  // This is a very\n"
+"   // long comment\n"
+"   // which is wrapped\n"
+"   // arround.\n"
+"\n"
+"int x = 2; // Is this still\n"
+"   // aligned?\n",
+format("int foo = 12345; // comment\n"
+   "int bar = 1234; // This is a very long comment\n"
+   "// which is wrapped arround.\n"
+   "\n"
+   "int x = 2; // Is this still aligned?\n",
+   Style));
+
+  Style.ColumnLimit = 35;
+  EXPECT_EQ("int foo = 12345; // comment\n"
+"int bar =\n"
+

[PATCH] D135820: [clang-tblgen] renames Diagnostic.Text to Diagnostic.Summary

2022-10-13 Thread Erich Keane via Phabricator via cfe-commits

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

I'm unopposed.  I think the justification isn't particularly clear to me and 
doesn't seem discussed in that RFC that I can tell, but I don't really have a 
concern here.  Give folks a few days to complain if they are going to


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135820

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

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

2022-10-13 Thread Erich Keane via Phabricator via cfe-commits

erichkeane added a comment.

In D126907#3854983 , @cor3ntin wrote:

> @erichkeane I was looking at Daisy meta programming shenanigans and found 
> this crashes https://cute.godbolt.org/z/6fWoEheKc - seems related to your 
> work, probably? It used to work in Clang 15. I thought you might want to know!

Thanks!  I'll take a look.  That DOES look like some other failures I've had 
with this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126907

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

[PATCH] D135872: [clang-format] Handle unions like structs and classes

2022-10-13 Thread Björn Schäpers via Phabricator via cfe-commits

HazardyKnusperkeks created this revision.
HazardyKnusperkeks added reviewers: MyDeveloperDay, curdeius, owenpan, rymiel.
HazardyKnusperkeks added a project: clang-format.
Herald added a project: All.
HazardyKnusperkeks requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

There is no reason why unions should be handled differently, I think they are 
just forgotten since they are not used that often.

No test case added, since that would be complicated to produce.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D135872

Files:
  clang/lib/Format/TokenAnnotator.cpp


Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -3157,7 +3157,7 @@
 return 160;
   if (Left.is(TT_CastRParen))
 return 100;
-  if (Left.isOneOf(tok::kw_class, tok::kw_struct))
+  if (Left.isOneOf(tok::kw_class, tok::kw_struct, tok::kw_union))
 return 5000;
   if (Left.is(tok::comment))
 return 1000;


Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -3157,7 +3157,7 @@
 return 160;
   if (Left.is(TT_CastRParen))
 return 100;
-  if (Left.isOneOf(tok::kw_class, tok::kw_struct))
+  if (Left.isOneOf(tok::kw_class, tok::kw_struct, tok::kw_union))
 return 5000;
   if (Left.is(tok::comment))
 return 1000;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D135871: [clang-format][NFC] Handle language specific stuff at the top...

2022-10-13 Thread Björn Schäpers via Phabricator via cfe-commits

HazardyKnusperkeks created this revision.
HazardyKnusperkeks added reviewers: owenpan, MyDeveloperDay, curdeius, rymiel.
HazardyKnusperkeks added a project: clang-format.
Herald added a project: All.
HazardyKnusperkeks requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

... of TokenAnnotator::splitPenalty. That is in my eyes a bit clearer in the 
workflow.

As a drive-by introduce (but not adapt anywhere else) isProto().


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D135871

Files:
  clang/include/clang/Format/Format.h
  clang/lib/Format/TokenAnnotator.cpp


Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -3095,6 +3095,7 @@
   if (Left.is(tok::semi))
 return 0;
 
+  // Language specific handling.
   if (Style.Language == FormatStyle::LK_Java) {
 if (Right.isOneOf(Keywords.kw_extends, Keywords.kw_throws))
   return 1;
@@ -3114,13 +3115,16 @@
 // Prefer breaking call chains (".foo") over empty "{}", "[]" or "()".
 if (Left.opensScope() && Right.closesScope())
   return 200;
+  } else if (Style.isProto()) {
+if (Right.is(tok::l_square))
+  return 1;
+if (Right.is(tok::period))
+  return 500;
   }
 
   if (Right.is(tok::identifier) && Right.Next && 
Right.Next->is(TT_DictLiteral))
 return 1;
   if (Right.is(tok::l_square)) {
-if (Style.Language == FormatStyle::LK_Proto)
-  return 1;
 if (Left.is(tok::r_square))
   return 200;
 // Slightly prefer formatting local lambda definitions like functions.
@@ -3133,10 +3137,8 @@
 }
   }
 
-  if (Left.is(tok::coloncolon) ||
-  (Right.is(tok::period) && Style.Language == FormatStyle::LK_Proto)) {
+  if (Left.is(tok::coloncolon))
 return 500;
-  }
   if (Right.isOneOf(TT_StartOfName, TT_FunctionDeclarationName) ||
   Right.is(tok::kw_operator)) {
 if (Line.startsWith(tok::kw_for) && Right.PartOfMultiVariableDeclStmt)
Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -2617,6 +2617,7 @@
   bool isJson() const { return Language == LK_Json; }
   bool isJavaScript() const { return Language == LK_JavaScript; }
   bool isVerilog() const { return Language == LK_Verilog; }
+  bool isProto() const { return Language == LK_Proto; }
 
   /// Language, this format style is targeted at.
   /// \version 3.5


Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -3095,6 +3095,7 @@
   if (Left.is(tok::semi))
 return 0;
 
+  // Language specific handling.
   if (Style.Language == FormatStyle::LK_Java) {
 if (Right.isOneOf(Keywords.kw_extends, Keywords.kw_throws))
   return 1;
@@ -3114,13 +3115,16 @@
 // Prefer breaking call chains (".foo") over empty "{}", "[]" or "()".
 if (Left.opensScope() && Right.closesScope())
   return 200;
+  } else if (Style.isProto()) {
+if (Right.is(tok::l_square))
+  return 1;
+if (Right.is(tok::period))
+  return 500;
   }
 
   if (Right.is(tok::identifier) && Right.Next && Right.Next->is(TT_DictLiteral))
 return 1;
   if (Right.is(tok::l_square)) {
-if (Style.Language == FormatStyle::LK_Proto)
-  return 1;
 if (Left.is(tok::r_square))
   return 200;
 // Slightly prefer formatting local lambda definitions like functions.
@@ -3133,10 +3137,8 @@
 }
   }
 
-  if (Left.is(tok::coloncolon) ||
-  (Right.is(tok::period) && Style.Language == FormatStyle::LK_Proto)) {
+  if (Left.is(tok::coloncolon))
 return 500;
-  }
   if (Right.isOneOf(TT_StartOfName, TT_FunctionDeclarationName) ||
   Right.is(tok::kw_operator)) {
 if (Line.startsWith(tok::kw_for) && Right.PartOfMultiVariableDeclStmt)
Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -2617,6 +2617,7 @@
   bool isJson() const { return Language == LK_Json; }
   bool isJavaScript() const { return Language == LK_JavaScript; }
   bool isVerilog() const { return Language == LK_Verilog; }
+  bool isProto() const { return Language == LK_Proto; }
 
   /// Language, this format style is targeted at.
   /// \version 3.5
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] fcce256 - [Clang][LoongArch] Pass "f" and "d" features to cc1 to enable hard float

2022-10-13 Thread Weining Lu via cfe-commits


Author: Weining Lu
Date: 2022-10-13T20:00:29+08:00
New Revision: fcce2562c177f0ad2aa03620be8675bef5c631d1

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

LOG: [Clang][LoongArch] Pass "f" and "d" features to cc1 to enable hard float

On LoongArch, currently neither of "f" and "d" feature is passed from
clang driver to cc1 by default. This means the backend generates code
for soft float.

In order to run programs in current LoongArch machines (hard float
environment) this patch temporarily enables "f" and "d" features.

In future, we should conditionally turn on these features depend on
various clang options, e.g. -mdouble-float, -msingle-float,
-msoft-float and -mfpu.

Added: 
clang/test/Driver/loongarch-default-features.c

Modified: 
clang/lib/Driver/ToolChains/Arch/LoongArch.cpp
clang/lib/Driver/ToolChains/Arch/LoongArch.h
clang/lib/Driver/ToolChains/Clang.cpp

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Arch/LoongArch.cpp 
b/clang/lib/Driver/ToolChains/Arch/LoongArch.cpp
index f8cda26ad3f8e..d364a2e8f7a8e 100644
--- a/clang/lib/Driver/ToolChains/Arch/LoongArch.cpp
+++ b/clang/lib/Driver/ToolChains/Arch/LoongArch.cpp
@@ -26,3 +26,14 @@ StringRef loongarch::getLoongArchABI(const ArgList &Args,
   // TODO: select appropiate ABI.
   return Triple.getArch() == llvm::Triple::loongarch32 ? "ilp32d" : "lp64d";
 }
+
+void loongarch::getLoongArchTargetFeatures(const Driver &D,
+   const llvm::Triple &Triple,
+   const ArgList &Args,
+   std::vector &Features) {
+  // FIXME: hornor various clang options that may affect target features, e.g.
+  // -march/-mtune/-mdouble-float/-msingle-float/-msoft-float/-mfpu. See:
+  // 
https://loongson.github.io/LoongArch-Documentation/LoongArch-toolchain-conventions-EN.html
+  Features.push_back("+f");
+  Features.push_back("+d");
+}

diff  --git a/clang/lib/Driver/ToolChains/Arch/LoongArch.h 
b/clang/lib/Driver/ToolChains/Arch/LoongArch.h
index a128e611f8c3c..eb09419c2c1ba 100644
--- a/clang/lib/Driver/ToolChains/Arch/LoongArch.h
+++ b/clang/lib/Driver/ToolChains/Arch/LoongArch.h
@@ -19,6 +19,10 @@ namespace tools {
 namespace loongarch {
 StringRef getLoongArchABI(const llvm::opt::ArgList &Args,
   const llvm::Triple &Triple);
+
+void getLoongArchTargetFeatures(const Driver &D, const llvm::Triple &Triple,
+const llvm::opt::ArgList &Args,
+std::vector &Features);
 } // end namespace loongarch
 } // end namespace tools
 } // end namespace driver

diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index 83023a155dee9..15f45275b76ab 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -372,6 +372,10 @@ static void getTargetFeatures(const Driver &D, const 
llvm::Triple &Triple,
   case llvm::Triple::csky:
 csky::getCSKYTargetFeatures(D, Triple, Args, CmdArgs, Features);
 break;
+  case llvm::Triple::loongarch32:
+  case llvm::Triple::loongarch64:
+loongarch::getLoongArchTargetFeatures(D, Triple, Args, Features);
+break;
   }
 
   for (auto Feature : unifyTargetFeatures(Features)) {

diff  --git a/clang/test/Driver/loongarch-default-features.c 
b/clang/test/Driver/loongarch-default-features.c
new file mode 100644
index 0..833ee4fd3fafa
--- /dev/null
+++ b/clang/test/Driver/loongarch-default-features.c
@@ -0,0 +1,10 @@
+// RUN: %clang --target=loongarch32 -S -emit-llvm %s -o - | FileCheck %s 
--check-prefix=LA32
+// RUN: %clang --target=loongarch64 -S -emit-llvm %s -o - | FileCheck %s 
--check-prefix=LA64
+
+// LA32: "target-features"="+d,+f"
+// LA64: "target-features"="+d,+f"
+
+/// Dummy function
+int foo(void) {
+  return  3;
+}



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

[PATCH] D135866: [clang-format][NFC] Fix comment grammer in ContinuationIndenter

2022-10-13 Thread Björn Schäpers via Phabricator via cfe-commits

HazardyKnusperkeks created this revision.
HazardyKnusperkeks added reviewers: MyDeveloperDay, OwenBed, curdeius, rymiel.
HazardyKnusperkeks added a project: clang-format.
Herald added a project: All.
HazardyKnusperkeks requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Now all comments (for which id makes sense) end with a punctuation.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D135866

Files:
  clang/lib/Format/ContinuationIndenter.cpp


Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -444,7 +444,7 @@
   }
 
   // If the template declaration spans multiple lines, force wrap before the
-  // function/class declaration
+  // function/class declaration.
   if (Previous.ClosesTemplateDeclaration && CurrentState.BreakBeforeParameter 
&&
   Current.CanBreakBefore) {
 return true;
@@ -552,7 +552,7 @@
 
   // If the return type spans multiple lines, wrap before the function name.
   if (((Current.is(TT_FunctionDeclarationName) &&
-// Don't break before a C# function when no break after return type
+// Don't break before a C# function when no break after return type.
 (!Style.isCSharp() ||
  Style.AlwaysBreakAfterReturnType != FormatStyle::RTBS_None) &&
 // Don't always break between a JavaScript `function` and the function
@@ -1305,7 +1305,7 @@
   if (Previous->ParameterCount > 1)
 return true;
 
-  // Also a nested block if contains a lambda inside function with 1 parameter
+  // Also a nested block if contains a lambda inside function with 1 parameter.
   return Style.BraceWrapping.BeforeLambdaBody && Current.is(TT_LambdaLSquare);
 }
 
@@ -1525,7 +1525,7 @@
   Previous->is(TT_ConditionalExpr))) &&
 !Newline) {
   // If BreakBeforeBinaryOperators is set, un-indent a bit to account for
-  // the operator and keep the operands aligned
+  // the operator and keep the operands aligned.
   if (Style.AlignOperands == FormatStyle::OAS_AlignAfterOperator)
 NewParenState.UnindentOperator = true;
   // Mark indentation as alignment if the expression is aligned.
@@ -1717,7 +1717,7 @@
 
   if (Style.BraceWrapping.BeforeLambdaBody && Current.Next != nullptr &&
   Current.is(tok::l_paren)) {
-// Search for any parameter that is a lambda
+// Search for any parameter that is a lambda.
 FormatToken const *next = Current.Next;
 while (next != nullptr) {
   if (next->is(TT_LambdaLSquare)) {
@@ -2537,7 +2537,7 @@
 }
 
 unsigned ContinuationIndenter::getColumnLimit(const LineState &State) const {
-  // In preprocessor directives reserve two chars for trailing " \"
+  // In preprocessor directives reserve two chars for trailing " \".
   return Style.ColumnLimit - (State.Line->InPPDirective ? 2 : 0);
 }
 


Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -444,7 +444,7 @@
   }
 
   // If the template declaration spans multiple lines, force wrap before the
-  // function/class declaration
+  // function/class declaration.
   if (Previous.ClosesTemplateDeclaration && CurrentState.BreakBeforeParameter &&
   Current.CanBreakBefore) {
 return true;
@@ -552,7 +552,7 @@
 
   // If the return type spans multiple lines, wrap before the function name.
   if (((Current.is(TT_FunctionDeclarationName) &&
-// Don't break before a C# function when no break after return type
+// Don't break before a C# function when no break after return type.
 (!Style.isCSharp() ||
  Style.AlwaysBreakAfterReturnType != FormatStyle::RTBS_None) &&
 // Don't always break between a JavaScript `function` and the function
@@ -1305,7 +1305,7 @@
   if (Previous->ParameterCount > 1)
 return true;
 
-  // Also a nested block if contains a lambda inside function with 1 parameter
+  // Also a nested block if contains a lambda inside function with 1 parameter.
   return Style.BraceWrapping.BeforeLambdaBody && Current.is(TT_LambdaLSquare);
 }
 
@@ -1525,7 +1525,7 @@
   Previous->is(TT_ConditionalExpr))) &&
 !Newline) {
   // If BreakBeforeBinaryOperators is set, un-indent a bit to account for
-  // the operator and keep the operands aligned
+  // the operator and keep the operands aligned.
   if (Style.AlignOperands == FormatStyle::OAS_AlignAfterOperator)
 NewParenState.UnindentOperator = true;
   // Mark indentation as alignment if the expression is aligned.
@@ -1717,7 +1717,7 @@
 
   if (Style.BraceWrapping.BeforeLambdaBody && Current.Next != nullptr &&
   Current.is(tok::l_paren)) {
-// Search for any parameter that is a lambda
+// Search for any paramete

[PATCH] D129443: [clang-format] Add option for aligning requires clause body

2022-10-13 Thread Björn Schäpers via Phabricator via cfe-commits

HazardyKnusperkeks added inline comments.



Comment at: clang/lib/Format/Format.cpp:1304
   LLVMStyle.RequiresClausePosition = FormatStyle::RCPS_OwnLine;
+  LLVMStyle.RequiresExpressionIndentation = FormatStyle::REI_Keyword;
   LLVMStyle.SeparateDefinitionBlocks = FormatStyle::SDS_Leave;

owenpan wrote:
> HazardyKnusperkeks wrote:
> > owenpan wrote:
> > > Delete it.
> > Why delete?
> > Set it to OuterScope.
> > Why delete?
> > Set it to OuterScope.
> 
> Cuz OuterScope is the default. :)
The default is uninitialized. Not setting anything here is undefined behavior.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129443

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

[PATCH] D135696: [pseudo] Document disambiguation design progress

2022-10-13 Thread Sam McCall via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGb13f7f9c0660: [pseudo] Document disambiguation design 
progress (authored by sammccall).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135696

Files:
  clang-tools-extra/pseudo/Disambiguation.md

Index: clang-tools-extra/pseudo/Disambiguation.md
===
--- /dev/null
+++ clang-tools-extra/pseudo/Disambiguation.md
@@ -0,0 +1,367 @@
+# Disambiguation
+
+The C++ grammar is highly ambiguous, so the GLR parser produces a forest of
+parses, represented compactly by a DAG.
+A real C++ parser finds the correct parse through semantic analysis: mostly
+resolving names. But we can't do that, as we don't parse the headers.
+
+Our disambiguation phase should take the parse forest, and choose a single parse
+tree that is most likely.
+It might **optionally** use some other hints (e.g. coding style, or what
+specific names tend to mean in this codebase).
+
+There are some grammatical ambiguities that can be resolved without semantic
+analysis, e.g. whether `int {}` is a function-definition.
+We eliminate these earlier e.g., with rule guards. By "disambiguation" we mean
+choosing between interpretations that we can't reject confidently and locally.
+
+## Types of evidence
+
+We have limited information to go on, and strive to use similar heuristics a
+human reader might.
+
+### Likely and unlikely structure
+
+In some cases, the shape of a particular interpretation is unlikely but not
+impossible. For example, the statement `x(a);` might:
+
+- call a function `x` (likely) 
+- construct a temporary of class type `x` (less likely)
+- define a variable `a` of type `x`, which is an alias for e.g. `int`
+  (unlikely!)
+
+We model this as a bonus/penalty for including a particular forest node in the
+chosen parse. For each rule we want to apply, we can write some code to
+recognize the corresponding pattern in the parse tree, and run these recognizers
+at each node to assign the bonuses.
+
+### Interpreting names
+
+Just as resolving names allows a C++ parser to choose the right parse (rejecting
+others), chunks of a parse tree imply things about how names resolve.
+
+Because the same name often means the same thing in different contexts, we can
+apply knowledge from elsewhere. This can be as simple as knowing "`vector` is
+usually a type", and adding bonuses to nodes that include that interpretation.
+
+However we can also transfer knowlegde across the same file we're parsing:
+
+```cpp
+// Is `Builder` a class or a namespace?
+void Builder::build() { ... }
+// ...
+// `Builder` is a type.
+Builder b;
+```
+
+We can use this to understand more-ambiguous code based on names in a section
+we're more sure about. It also pushes us to provide a consistent parse, rather
+than interpreting each occurrence of an unclear name differently.
+
+Again, we can define bonuses/penalties for forest nodes that interpret names,
+but this time those bonuses change as we disambiguate. Specifically:
+
+- we can group identifiers into **classes**, most importantly "all identifiers
+  with text 'foo'" but also "all snake_case identifiers".
+- clusters of nodes immediately above the identifiers in the parse forest are
+  **interpretations**, they bind the identifier to a **kind** such "type",
+  "value", "namespace", "other".
+- for each class we can query information once from an external source (such as
+  an index or hard-coded list), yielding a vector of weights (one per kind)
+- at each point we can compute metrics based on interpretations in the forest:
+   - the number of identifiers in the class that are interpreted as each kind
+ (e.g. *all* remaining interpretations of 'foo' at 3:7 are 'type')
+   - the number of identifiers in the class that *may* be interpereted as each
+ kind (e.g. 'foo' at 3:7 might be a 'type').
+- we can mash these metrics together into a vector of bonuses for a class (e.g.
+  for identifiers with text 'bar', 'type'=>+5, 'namespace'=>+1, 'value'=>-2).
+- these bonuses are assigned to corresponding interpretations in the graph
+
+### Templates
+
+Another aspect of a name is whether it names a template (type or value). This 
+is ambiguous in many more cases since CTAD allowed template arguments to be
+omitted.
+
+A fairly simple heuristic appears sufficient here: things that look like
+templates usually are, so if a node for certain rules exists in the forest
+(e.g. `template-id := template-name < template-argument-list >`) then we treat
+the template name as a probable template, and apply a bonus to every node that
+interprets it that way. We do this even if alternate parses are possible
+(`a < b > :: c` might be a comparison, but is still evidence `a` is a template).
+
+## Algorithm sketch
+
+With static node scores, finding the best tree is a very tractable problem
+with an

[clang-tools-extra] b13f7f9 - [pseudo] Document disambiguation design progress

2022-10-13 Thread Sam McCall via cfe-commits


Author: Sam McCall
Date: 2022-10-13T13:05:24+02:00
New Revision: b13f7f9c06604110709a968a2fece4b8d5192708

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

LOG: [pseudo] Document disambiguation design progress

Need to take a break from this, so write down where we got to.

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

Added: 
clang-tools-extra/pseudo/Disambiguation.md

Modified: 


Removed: 




diff  --git a/clang-tools-extra/pseudo/Disambiguation.md 
b/clang-tools-extra/pseudo/Disambiguation.md
new file mode 100644
index 0..39e246a523beb
--- /dev/null
+++ b/clang-tools-extra/pseudo/Disambiguation.md
@@ -0,0 +1,367 @@
+# Disambiguation
+
+The C++ grammar is highly ambiguous, so the GLR parser produces a forest of
+parses, represented compactly by a DAG.
+A real C++ parser finds the correct parse through semantic analysis: mostly
+resolving names. But we can't do that, as we don't parse the headers.
+
+Our disambiguation phase should take the parse forest, and choose a single 
parse
+tree that is most likely.
+It might **optionally** use some other hints (e.g. coding style, or what
+specific names tend to mean in this codebase).
+
+There are some grammatical ambiguities that can be resolved without semantic
+analysis, e.g. whether `int {}` is a function-definition.
+We eliminate these earlier e.g., with rule guards. By "disambiguation" we mean
+choosing between interpretations that we can't reject confidently and locally.
+
+## Types of evidence
+
+We have limited information to go on, and strive to use similar heuristics a
+human reader might.
+
+### Likely and unlikely structure
+
+In some cases, the shape of a particular interpretation is unlikely but not
+impossible. For example, the statement `x(a);` might:
+
+- call a function `x` (likely) 
+- construct a temporary of class type `x` (less likely)
+- define a variable `a` of type `x`, which is an alias for e.g. `int`
+  (unlikely!)
+
+We model this as a bonus/penalty for including a particular forest node in the
+chosen parse. For each rule we want to apply, we can write some code to
+recognize the corresponding pattern in the parse tree, and run these 
recognizers
+at each node to assign the bonuses.
+
+### Interpreting names
+
+Just as resolving names allows a C++ parser to choose the right parse 
(rejecting
+others), chunks of a parse tree imply things about how names resolve.
+
+Because the same name often means the same thing in 
diff erent contexts, we can
+apply knowledge from elsewhere. This can be as simple as knowing "`vector` is
+usually a type", and adding bonuses to nodes that include that interpretation.
+
+However we can also transfer knowlegde across the same file we're parsing:
+
+```cpp
+// Is `Builder` a class or a namespace?
+void Builder::build() { ... }
+// ...
+// `Builder` is a type.
+Builder b;
+```
+
+We can use this to understand more-ambiguous code based on names in a section
+we're more sure about. It also pushes us to provide a consistent parse, rather
+than interpreting each occurrence of an unclear name 
diff erently.
+
+Again, we can define bonuses/penalties for forest nodes that interpret names,
+but this time those bonuses change as we disambiguate. Specifically:
+
+- we can group identifiers into **classes**, most importantly "all identifiers
+  with text 'foo'" but also "all snake_case identifiers".
+- clusters of nodes immediately above the identifiers in the parse forest are
+  **interpretations**, they bind the identifier to a **kind** such "type",
+  "value", "namespace", "other".
+- for each class we can query information once from an external source (such as
+  an index or hard-coded list), yielding a vector of weights (one per kind)
+- at each point we can compute metrics based on interpretations in the forest:
+   - the number of identifiers in the class that are interpreted as each kind
+ (e.g. *all* remaining interpretations of 'foo' at 3:7 are 'type')
+   - the number of identifiers in the class that *may* be interpereted as each
+ kind (e.g. 'foo' at 3:7 might be a 'type').
+- we can mash these metrics together into a vector of bonuses for a class (e.g.
+  for identifiers with text 'bar', 'type'=>+5, 'namespace'=>+1, 'value'=>-2).
+- these bonuses are assigned to corresponding interpretations in the graph
+
+### Templates
+
+Another aspect of a name is whether it names a template (type or value). This 
+is ambiguous in many more cases since CTAD allowed template arguments to be
+omitted.
+
+A fairly simple heuristic appears sufficient here: things that look like
+templates usually are, so if a node for certain rules exists in the forest
+(e.g. `template-id := template-name < template-argument-list >`) then we treat
+the template name as a prob

[PATCH] D135696: [pseudo] Document disambiguation design progress

2022-10-13 Thread Haojian Wu via Phabricator via cfe-commits

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

Thanks, the doc looks great, it covers everything we've discussed so far.

In D135696#3850044 , @sammccall wrote:

> The diagrams are rendered by github's markdown view.
> My favorite offline tool to preview them is the "Markdown Preview Mermaid 
> Support" VSCode extension.

I just copy-paste the content to github issue, and use the preview :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135696

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

[clang] b6da16f - [clang][ExtractAPI] Ignore fully anonymous RecordDecls

2022-10-13 Thread Daniel Grumberg via cfe-commits


Author: Daniel Grumberg
Date: 2022-10-13T11:53:53+01:00
New Revision: b6da16ffb9d5a47c16fa377097809c6592132d34

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

LOG: [clang][ExtractAPI] Ignore fully anonymous RecordDecls

ExtractAPI was emitting a separate symbol for anonymous record declaration
that define the type of a member of another record declaration. Now
ExtractAPI ignores these declarations and just records the existence of
the actual member.

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

Added: 
clang/test/ExtractAPI/anonymous_record_no_typedef.c

Modified: 
clang/lib/ExtractAPI/ExtractAPIConsumer.cpp

Removed: 




diff  --git a/clang/lib/ExtractAPI/ExtractAPIConsumer.cpp 
b/clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
index 4a97ee9922ddd..2333f81fd3a36 100644
--- a/clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
+++ b/clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
@@ -401,6 +401,9 @@ class ExtractAPIVisitor : public 
RecursiveASTVisitor {
 StringRef Name = Decl->getName();
 if (Name.empty())
   Name = getTypedefName(Decl);
+if (Name.empty())
+  return true;
+
 StringRef USR = API.recordUSR(Decl);
 PresumedLoc Loc =
 Context.getSourceManager().getPresumedLoc(Decl->getLocation());

diff  --git a/clang/test/ExtractAPI/anonymous_record_no_typedef.c 
b/clang/test/ExtractAPI/anonymous_record_no_typedef.c
new file mode 100644
index 0..6f7156ff1decb
--- /dev/null
+++ b/clang/test/ExtractAPI/anonymous_record_no_typedef.c
@@ -0,0 +1,396 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: sed -e "s@INPUT_DIR@%{/t:regex_replacement}@g" \
+// RUN: %t/reference.output.json.in >> %t/reference.output.json
+// RUN: %clang_cc1 -extract-api -triple arm64-apple-macosx \
+// RUN:   -x c-header %t/input.h -o %t/output.json -verify
+
+// Generator version is not consistent across test runs, normalize it.
+// RUN: sed -e "s@\"generator\": \".*\"@\"generator\": \"?\"@g" \
+// RUN: %t/output.json >> %t/output-normalized.json
+// RUN: 
diff  %t/reference.output.json %t/output-normalized.json
+
+//--- input.h
+/// A Vehicle
+struct Vehicle {
+/// The type of vehicle.
+enum {
+Bicycle,
+Car
+} type;
+
+/// The information about the vehicle.
+struct {
+int wheels;
+char *name;
+} information;
+};
+// expected-no-diagnostics
+
+//--- reference.output.json.in
+{
+  "metadata": {
+"formatVersion": {
+  "major": 0,
+  "minor": 5,
+  "patch": 3
+},
+"generator": "?"
+  },
+  "module": {
+"name": "",
+"platform": {
+  "architecture": "arm64",
+  "operatingSystem": {
+"minimumVersion": {
+  "major": 11,
+  "minor": 0,
+  "patch": 0
+},
+"name": "macosx"
+  },
+  "vendor": "apple"
+}
+  },
+  "relationships": [
+{
+  "kind": "memberOf",
+  "source": "c:@S@Vehicle@E@input.h@64@Bicycle",
+  "target": "c:@S@Vehicle@E@input.h@64"
+},
+{
+  "kind": "memberOf",
+  "source": "c:@S@Vehicle@E@input.h@64@Car",
+  "target": "c:@S@Vehicle@E@input.h@64"
+},
+{
+  "kind": "memberOf",
+  "source": "c:@S@Vehicle@FI@type",
+  "target": "c:@S@Vehicle"
+},
+{
+  "kind": "memberOf",
+  "source": "c:@S@Vehicle@FI@information",
+  "target": "c:@S@Vehicle"
+}
+  ],
+  "symbols": [
+{
+  "accessLevel": "public",
+  "declarationFragments": [
+{
+  "kind": "keyword",
+  "spelling": "enum"
+},
+{
+  "kind": "text",
+  "spelling": ": "
+},
+{
+  "kind": "typeIdentifier",
+  "preciseIdentifier": "c:i",
+  "spelling": "unsigned int"
+}
+  ],
+  "docComment": {
+"lines": [
+  {
+"range": {
+  "end": {
+"character": 29,
+"line": 3
+  },
+  "start": {
+"character": 9,
+"line": 3
+  }
+},
+"text": "The type of vehicle."
+  }
+]
+  },
+  "identifier": {
+"interfaceLanguage": "c",
+"precise": "c:@S@Vehicle@E@input.h@64"
+  },
+  "kind": {
+"displayName": "Enumeration",
+"identifier": "c.enum"
+  },
+  "location": {
+"position": {
+  "character": 5,
+  "line": 4
+},
+"uri": "file://INPUT_DIR/input.h"
+  },
+  "names": {
+"navigator": [
+  {
+"kind": "identifier",
+"spelling": "Vehicle::(anonymous)"
+  }
+],
+"title": "Vehicle::(anonymous)"
+  },
+  "pathComponents": [
+"Vehi

[PATCH] D135804: [clang][ExtractAPI] Ignore fully anonymous RecordDecls

2022-10-13 Thread Daniel Grumberg via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGb6da16ffb9d5: [clang][ExtractAPI] Ignore fully anonymous 
RecordDecls (authored by dang).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135804

Files:
  clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
  clang/test/ExtractAPI/anonymous_record_no_typedef.c

Index: clang/test/ExtractAPI/anonymous_record_no_typedef.c
===
--- /dev/null
+++ clang/test/ExtractAPI/anonymous_record_no_typedef.c
@@ -0,0 +1,396 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: sed -e "s@INPUT_DIR@%{/t:regex_replacement}@g" \
+// RUN: %t/reference.output.json.in >> %t/reference.output.json
+// RUN: %clang_cc1 -extract-api -triple arm64-apple-macosx \
+// RUN:   -x c-header %t/input.h -o %t/output.json -verify
+
+// Generator version is not consistent across test runs, normalize it.
+// RUN: sed -e "s@\"generator\": \".*\"@\"generator\": \"?\"@g" \
+// RUN: %t/output.json >> %t/output-normalized.json
+// RUN: diff %t/reference.output.json %t/output-normalized.json
+
+//--- input.h
+/// A Vehicle
+struct Vehicle {
+/// The type of vehicle.
+enum {
+Bicycle,
+Car
+} type;
+
+/// The information about the vehicle.
+struct {
+int wheels;
+char *name;
+} information;
+};
+// expected-no-diagnostics
+
+//--- reference.output.json.in
+{
+  "metadata": {
+"formatVersion": {
+  "major": 0,
+  "minor": 5,
+  "patch": 3
+},
+"generator": "?"
+  },
+  "module": {
+"name": "",
+"platform": {
+  "architecture": "arm64",
+  "operatingSystem": {
+"minimumVersion": {
+  "major": 11,
+  "minor": 0,
+  "patch": 0
+},
+"name": "macosx"
+  },
+  "vendor": "apple"
+}
+  },
+  "relationships": [
+{
+  "kind": "memberOf",
+  "source": "c:@S@Vehicle@E@input.h@64@Bicycle",
+  "target": "c:@S@Vehicle@E@input.h@64"
+},
+{
+  "kind": "memberOf",
+  "source": "c:@S@Vehicle@E@input.h@64@Car",
+  "target": "c:@S@Vehicle@E@input.h@64"
+},
+{
+  "kind": "memberOf",
+  "source": "c:@S@Vehicle@FI@type",
+  "target": "c:@S@Vehicle"
+},
+{
+  "kind": "memberOf",
+  "source": "c:@S@Vehicle@FI@information",
+  "target": "c:@S@Vehicle"
+}
+  ],
+  "symbols": [
+{
+  "accessLevel": "public",
+  "declarationFragments": [
+{
+  "kind": "keyword",
+  "spelling": "enum"
+},
+{
+  "kind": "text",
+  "spelling": ": "
+},
+{
+  "kind": "typeIdentifier",
+  "preciseIdentifier": "c:i",
+  "spelling": "unsigned int"
+}
+  ],
+  "docComment": {
+"lines": [
+  {
+"range": {
+  "end": {
+"character": 29,
+"line": 3
+  },
+  "start": {
+"character": 9,
+"line": 3
+  }
+},
+"text": "The type of vehicle."
+  }
+]
+  },
+  "identifier": {
+"interfaceLanguage": "c",
+"precise": "c:@S@Vehicle@E@input.h@64"
+  },
+  "kind": {
+"displayName": "Enumeration",
+"identifier": "c.enum"
+  },
+  "location": {
+"position": {
+  "character": 5,
+  "line": 4
+},
+"uri": "file://INPUT_DIR/input.h"
+  },
+  "names": {
+"navigator": [
+  {
+"kind": "identifier",
+"spelling": "Vehicle::(anonymous)"
+  }
+],
+"title": "Vehicle::(anonymous)"
+  },
+  "pathComponents": [
+"Vehicle::(anonymous)"
+  ]
+},
+{
+  "accessLevel": "public",
+  "declarationFragments": [
+{
+  "kind": "identifier",
+  "spelling": "Bicycle"
+}
+  ],
+  "identifier": {
+"interfaceLanguage": "c",
+"precise": "c:@S@Vehicle@E@input.h@64@Bicycle"
+  },
+  "kind": {
+"displayName": "Enumeration Case",
+"identifier": "c.enum.case"
+  },
+  "location": {
+"position": {
+  "character": 9,
+  "line": 5
+},
+"uri": "file://INPUT_DIR/input.h"
+  },
+  "names": {
+"navigator": [
+  {
+"kind": "identifier",
+"spelling": "Bicycle"
+  }
+],
+"subHeading": [
+  {
+"kind": "identifier",
+"spelling": "Bicycle"
+  }
+],
+"title": "Bicycle"
+  },
+  "pathComponents": [
+"Vehicle::(anonymous)",
+"Bicycle"
+  ]
+},
+{
+  "accessLevel": "public",
+  "declarationFragments": [
+{
+

[PATCH] D135829: [clangd] Block clang-tidy misc-const-correctness check

2022-10-13 Thread Sam McCall via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGe78165f0ba1e: [clangd] Block clang-tidy 
misc-const-correctness check (authored by sammccall).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135829

Files:
  clang-tools-extra/clangd/TidyProvider.cpp


Index: clang-tools-extra/clangd/TidyProvider.cpp
===
--- clang-tools-extra/clangd/TidyProvider.cpp
+++ clang-tools-extra/clangd/TidyProvider.cpp
@@ -212,8 +212,14 @@
// code, which is often the case when clangd
// tries to build an AST.
"-bugprone-use-after-move",
-   // Alias for bugprone-use-after-moe.
-   "-hicpp-invalid-access-moved");
+   // Alias for bugprone-use-after-move.
+   "-hicpp-invalid-access-moved",
+
+   // - Performance problems -
+
+   // This check runs expensive analysis for each variable.
+   // It has been observed to increase reparse time by 10x.
+   "-misc-const-correctness");
 
   size_t Size = BadChecks.size();
   for (const std::string &Str : ExtraBadChecks) {


Index: clang-tools-extra/clangd/TidyProvider.cpp
===
--- clang-tools-extra/clangd/TidyProvider.cpp
+++ clang-tools-extra/clangd/TidyProvider.cpp
@@ -212,8 +212,14 @@
// code, which is often the case when clangd
// tries to build an AST.
"-bugprone-use-after-move",
-   // Alias for bugprone-use-after-moe.
-   "-hicpp-invalid-access-moved");
+   // Alias for bugprone-use-after-move.
+   "-hicpp-invalid-access-moved",
+
+   // - Performance problems -
+
+   // This check runs expensive analysis for each variable.
+   // It has been observed to increase reparse time by 10x.
+   "-misc-const-correctness");
 
   size_t Size = BadChecks.size();
   for (const std::string &Str : ExtraBadChecks) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] e78165f - [clangd] Block clang-tidy misc-const-correctness check

2022-10-13 Thread Sam McCall via cfe-commits


Author: Sam McCall
Date: 2022-10-13T12:07:31+02:00
New Revision: e78165f0ba1e2fbf72b36a36c8560645b69a168a

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

LOG: [clangd] Block clang-tidy misc-const-correctness check

This check performs an extremely large amount of work (for each variable, it
runs very many full matcher-driven traversals of the whole scope the variable
is defined in).

When (inadvertently) enabled for Fuchsia, it regressed BuildAST times by >10x
(400ms -> 7s on my machine).

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

Added: 


Modified: 
clang-tools-extra/clangd/TidyProvider.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/TidyProvider.cpp 
b/clang-tools-extra/clangd/TidyProvider.cpp
index 32a4d6a30653..a0a37e86ba01 100644
--- a/clang-tools-extra/clangd/TidyProvider.cpp
+++ b/clang-tools-extra/clangd/TidyProvider.cpp
@@ -212,8 +212,14 @@ TidyProvider 
disableUnusableChecks(llvm::ArrayRef ExtraBadChecks) {
// code, which is often the case when clangd
// tries to build an AST.
"-bugprone-use-after-move",
-   // Alias for bugprone-use-after-moe.
-   "-hicpp-invalid-access-moved");
+   // Alias for bugprone-use-after-move.
+   "-hicpp-invalid-access-moved",
+
+   // - Performance problems -
+
+   // This check runs expensive analysis for each variable.
+   // It has been observed to increase reparse time by 10x.
+   "-misc-const-correctness");
 
   size_t Size = BadChecks.size();
   for (const std::string &Str : ExtraBadChecks) {



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

[PATCH] D135857: [clangd] Fix a crash in ExtractFunction tweak.

2022-10-13 Thread Haojian Wu via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG7ac10e5b347f: [clangd] Fix a crash in ExtractFunction tweak. 
(authored by hokein).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135857

Files:
  clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
  clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp


Index: clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp
===
--- clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp
+++ clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp
@@ -182,6 +182,7 @@
 
   // Shouldn't crash.
   EXPECT_EQ(apply("void f([[int a]]);"), "unavailable");
+  EXPECT_EQ(apply("void f(int a = [[1]]);"), "unavailable");
   // Don't extract if we select the entire function body (CompoundStmt).
   std::string CompoundFailInput = R"cpp(
 void f() [[{
Index: clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
@@ -248,6 +248,8 @@
   // FIXME: Support extraction from templated functions.
   if (Func->isTemplated())
 return nullptr;
+  if (!Func->getBody())
+return nullptr;
   for (const auto *S : Func->getBody()->children()) {
 // During apply phase, we perform semantic analysis (e.g. figure out
 // what variables requires hoisting). We cannot perform those when the


Index: clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp
===
--- clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp
+++ clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp
@@ -182,6 +182,7 @@
 
   // Shouldn't crash.
   EXPECT_EQ(apply("void f([[int a]]);"), "unavailable");
+  EXPECT_EQ(apply("void f(int a = [[1]]);"), "unavailable");
   // Don't extract if we select the entire function body (CompoundStmt).
   std::string CompoundFailInput = R"cpp(
 void f() [[{
Index: clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
@@ -248,6 +248,8 @@
   // FIXME: Support extraction from templated functions.
   if (Func->isTemplated())
 return nullptr;
+  if (!Func->getBody())
+return nullptr;
   for (const auto *S : Func->getBody()->children()) {
 // During apply phase, we perform semantic analysis (e.g. figure out
 // what variables requires hoisting). We cannot perform those when the
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] 7ac10e5 - [clangd] Fix a crash in ExtractFunction tweak.

2022-10-13 Thread Haojian Wu via cfe-commits


Author: Haojian Wu
Date: 2022-10-13T11:12:28+02:00
New Revision: 7ac10e5b347f3e22ae1dad5937de667e57c472be

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

LOG: [clangd] Fix a crash in ExtractFunction tweak.

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

Added: 


Modified: 
clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp 
b/clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
index 4231de8cc45d6..3b2e7d2bb86a7 100644
--- a/clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
+++ b/clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
@@ -248,6 +248,8 @@ const FunctionDecl *findEnclosingFunction(const Node 
*CommonAnc) {
   // FIXME: Support extraction from templated functions.
   if (Func->isTemplated())
 return nullptr;
+  if (!Func->getBody())
+return nullptr;
   for (const auto *S : Func->getBody()->children()) {
 // During apply phase, we perform semantic analysis (e.g. figure out
 // what variables requires hoisting). We cannot perform those when the

diff  --git 
a/clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp 
b/clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp
index 8e1da884bbd7d..dec63d454d52c 100644
--- a/clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp
+++ b/clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp
@@ -182,6 +182,7 @@ F (extracted();)
 
   // Shouldn't crash.
   EXPECT_EQ(apply("void f([[int a]]);"), "unavailable");
+  EXPECT_EQ(apply("void f(int a = [[1]]);"), "unavailable");
   // Don't extract if we select the entire function body (CompoundStmt).
   std::string CompoundFailInput = R"cpp(
 void f() [[{



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

[PATCH] D135859: [Includecleaner] Introduce RefType to ast walking

2022-10-13 Thread Kadir Cetinkaya via Phabricator via cfe-commits

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

RefTypes are distinct categories for each reference to a symbol. They
are signals indicating strength of a reference, that'll enable different
decision making based on the finding being provided.

There are 3 kinds of ref types:

- Explicit, the reference is spelled in the code.
- Implicit, the reference is not directly visible in the code.
- Related, the reference exists but can't be proven as used (e.g. overloads 
brought in by a using decl but not used by the code).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D135859

Files:
  clang-tools-extra/include-cleaner/lib/AnalysisInternal.h
  clang-tools-extra/include-cleaner/lib/WalkAST.cpp
  clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp

Index: clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
===
--- clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
@@ -1,16 +1,35 @@
 #include "AnalysisInternal.h"
 #include "clang/AST/ASTContext.h"
+#include "clang/AST/Expr.h"
 #include "clang/Basic/FileManager.h"
 #include "clang/Frontend/TextDiagnostic.h"
 #include "clang/Testing/TestAST.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Error.h"
+#include "llvm/Support/ErrorHandling.h"
 #include "llvm/Testing/Support/Annotations.h"
 #include "gtest/gtest.h"
+#include 
+#include 
+#include 
+#include 
 
 namespace clang {
 namespace include_cleaner {
 namespace {
 
+llvm::StringLiteral to_string(RefType RT) {
+  switch (RT) {
+  case RefType::Explicit:
+return "explicit";
+  case RefType::Implicit:
+return "implicit";
+  case RefType::Related:
+return "related";
+  }
+  llvm_unreachable("Unexpected RefType");
+}
+
 // Specifies a test of which symbols are referenced by a piece of code.
 //
 // Example:
@@ -38,20 +57,21 @@
   llvm::cantFail(AST.fileManager().getFileRef("target.h")));
 
   // Perform the walk, and capture the offsets of the referenced targets.
-  std::vector ReferencedOffsets;
+  std::unordered_map> ReferencedOffsets;
   for (Decl *D : AST.context().getTranslationUnitDecl()->decls()) {
 if (ReferencingFile != SM.getDecomposedExpansionLoc(D->getLocation()).first)
   continue;
-walkAST(*D, [&](SourceLocation Loc, NamedDecl &ND) {
+walkAST(*D, [&](SourceLocation Loc, NamedDecl &ND, RefType RT) {
   if (SM.getFileLoc(Loc) != ReferencingLoc)
 return;
   auto NDLoc = SM.getDecomposedLoc(SM.getFileLoc(ND.getLocation()));
   if (NDLoc.first != TargetFile)
 return;
-  ReferencedOffsets.push_back(NDLoc.second);
+  ReferencedOffsets[RT].push_back(NDLoc.second);
 });
   }
-  llvm::sort(ReferencedOffsets);
+  for (auto &Entry : ReferencedOffsets)
+llvm::sort(Entry.second);
 
   // Compare results to the expected points.
   // For each difference, show the target point in context, like a diagnostic.
@@ -61,17 +81,22 @@
   DiagOpts->ShowLevel = 0;
   DiagOpts->ShowNoteIncludeStack = 0;
   TextDiagnostic Diag(DiagOS, AST.context().getLangOpts(), DiagOpts);
-  auto DiagnosePoint = [&](const char *Message, unsigned Offset) {
+  auto DiagnosePoint = [&](llvm::StringRef Message, unsigned Offset) {
 Diag.emitDiagnostic(
 FullSourceLoc(SM.getComposedLoc(TargetFile, Offset), SM),
 DiagnosticsEngine::Note, Message, {}, {});
   };
-  for (auto Expected : Target.points())
-if (!llvm::is_contained(ReferencedOffsets, Expected))
-  DiagnosePoint("location not marked used", Expected);
-  for (auto Actual : ReferencedOffsets)
-if (!llvm::is_contained(Target.points(), Actual))
-  DiagnosePoint("location unexpectedly used", Actual);
+  for (auto RT : {RefType::Explicit, RefType::Implicit, RefType::Related}) {
+auto RTStr = to_string(RT);
+for (auto Expected : Target.points(RTStr))
+  if (!llvm::is_contained(ReferencedOffsets[RT], Expected))
+DiagnosePoint(("location not marked used with type " + RTStr).str(),
+  Expected);
+for (auto Actual : ReferencedOffsets[RT])
+  if (!llvm::is_contained(Target.points(RTStr), Actual))
+DiagnosePoint(("location unexpectedly used with type " + RTStr).str(),
+  Actual);
+  }
 
   // If there were any differences, we print the entire referencing code once.
   if (!DiagBuf.empty())
@@ -79,35 +104,39 @@
 }
 
 TEST(WalkAST, DeclRef) {
-  testWalk("int ^x;", "int y = ^x;");
-  testWalk("int ^foo();", "int y = ^foo();");
-  testWalk("namespace ns { int ^x; }", "int y = ns::^x;");
-  testWalk("struct S { static int ^x; };", "int y = S::^x;");
+  testWalk("int $explicit^x;", "int y = ^x;");
+  testWalk("int $explicit^foo();", "int y = ^

[PATCH] D135366: [clang][Interp] Implement String- and CharacterLiterals

2022-10-13 Thread Timm Bäder via Phabricator via cfe-commits

tbaeder updated this revision to Diff 467399.

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

https://reviews.llvm.org/D135366

Files:
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/lib/AST/Interp/ByteCodeExprGen.h
  clang/test/AST/Interp/literals.cpp
  clang/test/Lexer/char-escapes.c


Index: clang/test/Lexer/char-escapes.c
===
--- clang/test/Lexer/char-escapes.c
+++ clang/test/Lexer/char-escapes.c
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -fsyntax-only -pedantic -verify %s
+// RUN: %clang_cc1 -fsyntax-only -pedantic 
-fexperimental-new-constant-interpreter -verify %s
 
 int test['\\' == 92 ? 1 : -1];
 int test['\"' == 34 ? 1 : -1];
Index: clang/test/AST/Interp/literals.cpp
===
--- clang/test/AST/Interp/literals.cpp
+++ clang/test/AST/Interp/literals.cpp
@@ -309,3 +309,37 @@
 
   static_assert(0.0f == -0.0f, "");
 };
+
+namespace strings {
+  constexpr const char *S = "abc";
+  static_assert(S[0] == 97, "");
+  static_assert(S[1] == 98, "");
+  static_assert(S[2] == 99, "");
+  static_assert(S[3] == 0, "");
+
+  static_assert("foobar"[2] == 'o', "");
+  static_assert(2["foobar"] == 'o', "");
+
+  constexpr const wchar_t *wide = L"bar";
+  static_assert(wide[0] == L'b', "");
+
+  constexpr const char32_t *u32 = U"abc";
+  static_assert(u32[1] == U'b', "");
+
+  constexpr char32_t c = U'\U0001F60E';
+  static_assert(c == 0x0001F60EL, "");
+
+  constexpr char k = -1;
+  static_assert(k == -1, "");
+
+  static_assert('\N{LATIN CAPITAL LETTER E}' == 'E', "");
+  static_assert('\t' == 9, "");
+
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wmultichar"
+  constexpr int mc = 'abc';
+  static_assert(mc == 'abc', "");
+  __WCHAR_TYPE__ wm = L'abc'; // ref-error{{wide character literals may not 
contain multiple characters}} \
+  // expected-error{{wide character literals may 
not contain multiple characters}}
+#pragma clang diagnostic pop
+};
Index: clang/lib/AST/Interp/ByteCodeExprGen.h
===
--- clang/lib/AST/Interp/ByteCodeExprGen.h
+++ clang/lib/AST/Interp/ByteCodeExprGen.h
@@ -90,6 +90,8 @@
   bool VisitArrayInitIndexExpr(const ArrayInitIndexExpr *E);
   bool VisitOpaqueValueExpr(const OpaqueValueExpr *E);
   bool VisitAbstractConditionalOperator(const AbstractConditionalOperator *E);
+  bool VisitStringLiteral(const StringLiteral *E);
+  bool VisitCharacterLiteral(const CharacterLiteral *E);
 
 protected:
   bool visitExpr(const Expr *E) override;
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -433,6 +433,18 @@
   return true;
 }
 
+template 
+bool ByteCodeExprGen::VisitStringLiteral(const StringLiteral *E) {
+  unsigned StringIndex = P.createGlobalString(E);
+  return this->emitGetPtrGlobal(StringIndex, E);
+}
+
+template 
+bool ByteCodeExprGen::VisitCharacterLiteral(
+const CharacterLiteral *E) {
+  return this->emitConst(E, E->getValue());
+}
+
 template  bool ByteCodeExprGen::discard(const Expr *E) 
{
   OptionScope Scope(this, /*NewDiscardResult=*/true);
   return this->Visit(E);


Index: clang/test/Lexer/char-escapes.c
===
--- clang/test/Lexer/char-escapes.c
+++ clang/test/Lexer/char-escapes.c
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -fsyntax-only -pedantic -verify %s
+// RUN: %clang_cc1 -fsyntax-only -pedantic -fexperimental-new-constant-interpreter -verify %s
 
 int test['\\' == 92 ? 1 : -1];
 int test['\"' == 34 ? 1 : -1];
Index: clang/test/AST/Interp/literals.cpp
===
--- clang/test/AST/Interp/literals.cpp
+++ clang/test/AST/Interp/literals.cpp
@@ -309,3 +309,37 @@
 
   static_assert(0.0f == -0.0f, "");
 };
+
+namespace strings {
+  constexpr const char *S = "abc";
+  static_assert(S[0] == 97, "");
+  static_assert(S[1] == 98, "");
+  static_assert(S[2] == 99, "");
+  static_assert(S[3] == 0, "");
+
+  static_assert("foobar"[2] == 'o', "");
+  static_assert(2["foobar"] == 'o', "");
+
+  constexpr const wchar_t *wide = L"bar";
+  static_assert(wide[0] == L'b', "");
+
+  constexpr const char32_t *u32 = U"abc";
+  static_assert(u32[1] == U'b', "");
+
+  constexpr char32_t c = U'\U0001F60E';
+  static_assert(c == 0x0001F60EL, "");
+
+  constexpr char k = -1;
+  static_assert(k == -1, "");
+
+  static_assert('\N{LATIN CAPITAL LETTER E}' == 'E', "");
+  static_assert('\t' == 9, "");
+
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wmultichar"
+  constexpr int mc = 'abc';
+  static_assert(mc == 'abc', "");
+  __WCHAR_TYPE__ wm = L'abc'; // ref-error{{wide character literals may not contain multiple characters}} \
+  // expec

[PATCH] D135858: [clang][Interp] Support pointer arithmethic in binary operators

2022-10-13 Thread Timm Bäder via Phabricator via cfe-commits

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D135858

Files:
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/test/AST/Interp/arrays.cpp

Index: clang/test/AST/Interp/arrays.cpp
===
--- clang/test/AST/Interp/arrays.cpp
+++ clang/test/AST/Interp/arrays.cpp
@@ -37,6 +37,20 @@
 static_assert(getElement(1) == 4, "");
 static_assert(getElement(5) == 36, "");
 
+constexpr int data[] = {5, 4, 3, 2, 1};
+constexpr int getElement(const int *Arr, int index) {
+  return *(Arr + index);
+}
+
+static_assert(getElement(data, 1) == 4, "");
+static_assert(getElement(data, 4) == 1, "");
+
+constexpr int getElementFromEnd(const int *Arr, int size, int index) {
+  return *(Arr + size - index - 1);
+}
+static_assert(getElementFromEnd(data, 5, 0) == 1, "");
+static_assert(getElementFromEnd(data, 5, 4) == 5, "");
+
 
 template
 constexpr T getElementOf(T* array, int i) {
@@ -52,7 +66,6 @@
 static_assert(getElementOfArray(foo[2], 3) == &m, "");
 
 
-constexpr int data[] = {5, 4, 3, 2, 1};
 static_assert(data[0] == 4, ""); // expected-error{{failed}} \
  // expected-note{{5 == 4}} \
  // ref-error{{failed}} \
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -226,61 +226,79 @@
   // Typecheck the args.
   Optional LT = classify(LHS->getType());
   Optional RT = classify(RHS->getType());
-  if (!LT || !RT) {
+  Optional T = classify(BO->getType());
+  if (!LT || !RT || !T) {
 return this->bail(BO);
   }
 
-  if (Optional T = classify(BO->getType())) {
-if (!visit(LHS))
+  auto Discard = [this, T, BO](bool Result) {
+if (!Result)
   return false;
-if (!visit(RHS))
-  return false;
-
-auto Discard = [this, T, BO](bool Result) {
-  if (!Result)
+return DiscardResult ? this->emitPop(*T, BO) : true;
+  };
+
+  // Pointer arithmethic special case. This is supported for one of
+  // LHS and RHS being a pointer type and the other being an integer type.
+  if (BO->getType()->isPointerType()) {
+PrimType OffsetType;
+if (LHS->getType()->isIntegerType() && RHS->getType()->isPointerType()) {
+  if (!visit(RHS) || !visit(LHS))
 return false;
-  return DiscardResult ? this->emitPop(*T, BO) : true;
-};
-
-switch (BO->getOpcode()) {
-case BO_EQ:
-  return Discard(this->emitEQ(*LT, BO));
-case BO_NE:
-  return Discard(this->emitNE(*LT, BO));
-case BO_LT:
-  return Discard(this->emitLT(*LT, BO));
-case BO_LE:
-  return Discard(this->emitLE(*LT, BO));
-case BO_GT:
-  return Discard(this->emitGT(*LT, BO));
-case BO_GE:
-  return Discard(this->emitGE(*LT, BO));
-case BO_Sub:
-  return Discard(this->emitSub(*T, BO));
-case BO_Add:
-  return Discard(this->emitAdd(*T, BO));
-case BO_Mul:
-  return Discard(this->emitMul(*T, BO));
-case BO_Rem:
-  return Discard(this->emitRem(*T, BO));
-case BO_Div:
-  return Discard(this->emitDiv(*T, BO));
-case BO_Assign:
-  if (!this->emitStore(*T, BO))
+  OffsetType = *LT;
+} else {
+  if (!visit(LHS) || !visit(RHS))
 return false;
-  return DiscardResult ? this->emitPopPtr(BO) : true;
-case BO_And:
-  return Discard(this->emitBitAnd(*T, BO));
-case BO_Or:
-  return Discard(this->emitBitOr(*T, BO));
-case BO_LAnd:
-case BO_LOr:
-default:
-  return this->bail(BO);
+  OffsetType = *RT;
 }
+
+if (BO->getOpcode() == BO_Add)
+  return Discard(this->emitAddOffset(OffsetType, BO));
+else if (BO->getOpcode() == BO_Sub)
+  return Discard(this->emitSubOffset(OffsetType, BO));
+return this->bail(BO);
+  }
+
+  if (!visit(LHS) || !visit(RHS))
+return false;
+
+  switch (BO->getOpcode()) {
+  case BO_EQ:
+return Discard(this->emitEQ(*LT, BO));
+  case BO_NE:
+return Discard(this->emitNE(*LT, BO));
+  case BO_LT:
+return Discard(this->emitLT(*LT, BO));
+  case BO_LE:
+return Discard(this->emitLE(*LT, BO));
+  case BO_GT:
+return Discard(this->emitGT(*LT, BO));
+  case BO_GE:
+return Discard(this->emitGE(*LT, BO));
+  case BO_Sub:
+return Discard(this->emitSub(*T, BO));
+  case BO_Add:
+return Discard(this->emitAdd(*T, BO));
+  case BO_Mul:
+return Discard(this->emitMul(*T, BO));
+  case BO_Rem:
+return Discard(this->emitRem(*T, BO));
+  case BO_Div:
+return Discard(this->emitDiv(*T, BO));
+  case BO_Assign:
+if (!this->emitStore(*T, BO))
+  return false;
+return DiscardResult ? this->e

[PATCH] D135472: [ODRHash] Hash attributes on declarations.

2022-10-13 Thread Chuanqi Xu via Phabricator via cfe-commits

ChuanqiXu added inline comments.



Comment at: clang/lib/AST/ODRHash.cpp:479-480
+
+  llvm::copy_if(D->attrs(), std::back_inserter(HashableAttrs),
+[](const Attr *A) { return !A->isImplicit(); });
+}

aaron.ballman wrote:
> ChuanqiXu wrote:
> > vsapsai wrote:
> > > vsapsai wrote:
> > > > erichkeane wrote:
> > > > > aaron.ballman wrote:
> > > > > > ChuanqiXu wrote:
> > > > > > > vsapsai wrote:
> > > > > > > > I'm not sure `isImplicit` is the best indicator of attributes 
> > > > > > > > to check, so suggestions in this area are welcome. I think we 
> > > > > > > > can start strict and relax some of the checks if needed.
> > > > > > > > 
> > > > > > > > If people have strong opinions that some attributes shouldn't 
> > > > > > > > be ignored, we can add them to the tests to avoid regressions. 
> > > > > > > > Personally, I believe that alignment and packed attributes 
> > > > > > > > should never be silently ignored.
> > > > > > > Agreed. I feel `isImplicit` is enough for now.
> > > > > > The tricky part is -- sometimes certain attributes add additional 
> > > > > > implicit attributes and those implicit attributes matter 
> > > > > > (https://github.com/llvm/llvm-project/blob/main/clang/lib/Sema/SemaDeclAttr.cpp#L9380).
> > > > > >  And some attributes seem to just do the wrong thing entirely: 
> > > > > > https://github.com/llvm/llvm-project/blob/main/clang/lib/Sema/SemaDeclAttr.cpp#L7344
> > > > > > 
> > > > > > So I think `isImplicit()` is a good approximation, but I'm more 
> > > > > > wondering what the principle is for whether an attribute should or 
> > > > > > should not be considered part of the ODR hash. Type attributes, 
> > > > > > attributes that impact object layout, etc all seem like they should 
> > > > > > almost certainly be part of ODR hashing. Others are a bit more 
> > > > > > questionable though.
> > > > > > 
> > > > > > I think this is something that may need a per-attribute flag in 
> > > > > > Attr.td so attributes can opt in or out of it because I'm not 
> > > > > > certain what ODR issues could stem from `[[maybe_unused]]` or 
> > > > > > `[[deprecated]]` disagreements across module boundaries.
> > > > > I don't think 'isImplicit' is particularly good.  I think the idea of 
> > > > > auto-adding 'type' attributes and having the 'rest' be analyzed to 
> > > > > figure out which are important.
> > > > > 
> > > > > Alternatively, I wonder if we're better off just adding ALL 
> > > > > attributes and seeing what the fallout is. We can later decide when 
> > > > > we don't want them to be ODR significant (which, might be OTHERWISE 
> > > > > meaningful later!).
> > > > One criteria to decide which attributes should be hashed is if they 
> > > > affect IRGen. But that's not exhaustive and I'm not sure how practical 
> > > > it is.
> > > > 
> > > > The rule I'm trying to follow right now is if declarations with 
> > > > different attributes can be substituted instead of each other. For 
> > > > example, structs with different memory layout cannot be interchanged, 
> > > > so it is reasonable to reject them.
> > > > 
> > > > But maybe we should review attributes on case-by-case basis. For 
> > > > example, for `[[deprecated]]` I think the best for developers is not to 
> > > > complain about it but merge the attributes and then have regular 
> > > > diagnostic about using a deprecated entity.
> > > One option was to hash `isa` attributes as they are 
> > > "sticky" and should be more significant, so shouldn't be ignored. But I 
> > > don't think this argument is particularly convincing.
> > > 
> > > At this stage I think we can still afford to add all attributes and 
> > > exclude some as-needed because modules adoption is still limited. Though 
> > > I don't have strong feelings about this approach, just getting more 
> > > restrictive later can be hard.
> > It looks not a bad idea to add all attributes as experiments, @vsapsai how 
> > do you feel about this?
> My intuition is that we're going to want this to be controlled from Attr.td 
> on a case-by-case basis and to automatically generate the ODR hash code for 
> attribute arguments.
> 
> We can either force every attribute to decide explicitly (this seems pretty 
> disruptive as a final design but would be a really good way to ensure we 
> audit all attributes if done as an experiment), or we can pick a default 
> behavior to ODR hash/not. I suspect we're going to want to pick a default 
> behavior based on whatever the audit tells us the most common situation is.
> 
> I think we're going to need something a bit more nuanced than "this attribute 
> matters for ODR" because there are attribute arguments that don't always 
> contribute to the entity (for example, we have "fake" arguments that are used 
> to give the semantic attribute more information, there may also be cases 
> where one argument matter and another argument does not such as `enable_if` 
> where the condition matters gr

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

2022-10-13 Thread Iain Sandoe via Phabricator via cfe-commits

iains added a comment.

In D134267#3854980 , @ChuanqiXu wrote:

> In D134267#3852136 , @boris wrote:
>
>>> For example, my experimental support for P1689 
>>>  is at: [...]. The implementation looks 
>>> relatively trivial to me. The simplicity is pretty important.



>> 2. I haven't looked at the code, but there are some complex problems in this 
>> area as highlighted by this MSVC bug: 
>> https://developercommunity.visualstudio.com/t/scanDependencies-does-not-take-into-acc/10029154
>>  I believe you may have it easier because reportedly Richard & friends have 
>> already implemented the necessary header import isolation semantics.
>
> Yeah, it looks like the header unit brings new complexity. And my demo only 
> works for named modules. I need to reconsider it.

Header Units are "special", they defeat automatic processing in my view.

- The compiler cannot determine that a source is a HU (from the header source); 
it has to be told.
- A build system can determine that a source **depends** on a header (of 
course, that is already done for years), however, it is not at all clear that 
the build system is able to determine if that dependent header is an 
"importable header" and therefore suitable for building as a HU.  Maybe the 
build system folks have some ideas, of course (and a sub-set of headers are 
pre-defined as importable).

So, I suspect, that we will be expecting (at least in the short to medium term) 
the user's project to define which headers should be converted to HUs;  we 
already have specific driver options to help with the actual production.


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

https://reviews.llvm.org/D134267

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

[PATCH] D135857: [clangd] Fix a crash in ExtractFunction tweak.

2022-10-13 Thread Haojian Wu via Phabricator via cfe-commits

hokein created this revision.
hokein added a reviewer: kadircet.
Herald added a subscriber: arphaman.
Herald added a project: All.
hokein requested review of this revision.
Herald added subscribers: MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D135857

Files:
  clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
  clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp


Index: clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp
===
--- clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp
+++ clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp
@@ -182,6 +182,7 @@
 
   // Shouldn't crash.
   EXPECT_EQ(apply("void f([[int a]]);"), "unavailable");
+  EXPECT_EQ(apply("void f(int a = [[1]]);"), "unavailable");
   // Don't extract if we select the entire function body (CompoundStmt).
   std::string CompoundFailInput = R"cpp(
 void f() [[{
Index: clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
@@ -248,6 +248,8 @@
   // FIXME: Support extraction from templated functions.
   if (Func->isTemplated())
 return nullptr;
+  if (!Func->getBody())
+return nullptr;
   for (const auto *S : Func->getBody()->children()) {
 // During apply phase, we perform semantic analysis (e.g. figure out
 // what variables requires hoisting). We cannot perform those when the


Index: clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp
===
--- clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp
+++ clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp
@@ -182,6 +182,7 @@
 
   // Shouldn't crash.
   EXPECT_EQ(apply("void f([[int a]]);"), "unavailable");
+  EXPECT_EQ(apply("void f(int a = [[1]]);"), "unavailable");
   // Don't extract if we select the entire function body (CompoundStmt).
   std::string CompoundFailInput = R"cpp(
 void f() [[{
Index: clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
@@ -248,6 +248,8 @@
   // FIXME: Support extraction from templated functions.
   if (Func->isTemplated())
 return nullptr;
+  if (!Func->getBody())
+return nullptr;
   for (const auto *S : Func->getBody()->children()) {
 // During apply phase, we perform semantic analysis (e.g. figure out
 // what variables requires hoisting). We cannot perform those when the
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

2022-10-13 Thread Corentin Jabot via Phabricator via cfe-commits

cor3ntin added a comment.

@erichkeane I was looking at Daisy meta programming shenanigans and found this 
crashes https://cute.godbolt.org/z/6fWoEheKc - seems related to your work, 
probably? It used to work in Clang 15. I thought you might want to know!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126907

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

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

2022-10-13 Thread Chuanqi Xu via Phabricator via cfe-commits

ChuanqiXu added a comment.

In D134267#3852136 , @boris wrote:

>> For example, my experimental support for P1689 
>>  is at: [...]. The implementation looks 
>> relatively trivial to me. The simplicity is pretty important.
>
> Two points:
>
> 1. It's worth considering the simplicity of the overall system (compiler + 
> build system + user project) rather than just the compiler. I hope my 
> previous comment highlighted some of the complexity that the overall system 
> must deal with in the pre-scan approach with a lot of it potentially ending 
> up on the user's plate.
>
> 2. I haven't looked at the code, but there are some complex problems in this 
> area as highlighted by this MSVC bug: 
> https://developercommunity.visualstudio.com/t/scanDependencies-does-not-take-into-acc/10029154
>  I believe you may have it easier because reportedly Richard & friends have 
> already implemented the necessary header import isolation semantics.

Yeah, it looks like the header unit brings new complexity. And my demo only 
works for named modules. I need to reconsider it.

My original thought for these two modes was: the compiler could avoid to make 
the choice. I mean, the compiler could support both modes and let the build 
system writer and end user to make the choice.

And I believe there will be a discuss for client-server model vs pre-scan model 
in clang. I think we can discuss more then.

In D134267#3852883 , @iains wrote:

> To avoid more tangential discussion here - I've opened 
> https://discourse.llvm.org/t/generating-pcm-module-interfaces-and-regular-object-files-from-the-same-compiler-invocation/65918
>   ... it would be great if the major stakeholders here especially  @rsmith 
> could comment on the design intentions.

Yeah, this is about the deserialization part. I believe there'll be 2 other 
discussions about filename suffixes and client-module model. Although I feel 
none of them are blocker or even related to this patch : )

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

https://reviews.llvm.org/D134267

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

[PATCH] D135060: [HLSL] Add groupshare address space.

2022-10-13 Thread Xiang Li via Phabricator via cfe-commits

python3kgae updated this revision to Diff 467383.
python3kgae marked an inline comment as done.
python3kgae added a comment.

Add hlsl202x test for C++ 11 features like lambda.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135060

Files:
  clang/include/clang/Basic/AddressSpaces.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/TokenKinds.def
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/ParsedAttr.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/Basic/Targets/AMDGPU.cpp
  clang/lib/Basic/Targets/DirectX.h
  clang/lib/Basic/Targets/NVPTX.h
  clang/lib/Basic/Targets/SPIR.h
  clang/lib/Basic/Targets/TCE.h
  clang/lib/Basic/Targets/X86.h
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseExprCXX.cpp
  clang/lib/Parse/ParseTentative.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaLambda.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/AST/HLSL/group_shared.hlsl
  clang/test/CodeGenHLSL/group_shared.hlsl
  clang/test/Parser/opencl-cxx-keywords.cl
  clang/test/ParserHLSL/group_shared.hlsl
  clang/test/ParserHLSL/group_shared_202x.hlsl
  clang/test/SemaHLSL/group_shared.hlsl
  clang/test/SemaHLSL/group_shared_202x.hlsl
  clang/test/SemaOpenCL/address-spaces.cl
  clang/test/SemaOpenCL/invalid-kernel.cl
  clang/test/SemaTemplate/address_space-dependent.cpp

Index: clang/test/SemaTemplate/address_space-dependent.cpp
===
--- clang/test/SemaTemplate/address_space-dependent.cpp
+++ clang/test/SemaTemplate/address_space-dependent.cpp
@@ -43,7 +43,7 @@
 
 template 
 void tooBig() {
-  __attribute__((address_space(I))) int *bounds; // expected-error {{address space is larger than the maximum supported (8388588)}}
+  __attribute__((address_space(I))) int *bounds; // expected-error {{address space is larger than the maximum supported (8388587)}}
 }
 
 template 
Index: clang/test/SemaOpenCL/invalid-kernel.cl
===
--- clang/test/SemaOpenCL/invalid-kernel.cl
+++ clang/test/SemaOpenCL/invalid-kernel.cl
@@ -13,14 +13,14 @@
   return 0;
 }
 
-int* global x(int* x) { // expected-error {{return value cannot be qualified with address space}}
+int* global x(int* x) { // expected-error {{return type cannot be qualified with address space}}
   return x + 1;
 }
 
-int* local x(int* x) { // expected-error {{return value cannot be qualified with address space}}
+int* local x(int* x) { // expected-error {{return type cannot be qualified with address space}}
   return x + 1;
 }
 
-int* constant x(int* x) { // expected-error {{return value cannot be qualified with address space}}
+int* constant x(int* x) { // expected-error {{return type cannot be qualified with address space}}
   return x + 1;
 }
Index: clang/test/SemaOpenCL/address-spaces.cl
===
--- clang/test/SemaOpenCL/address-spaces.cl
+++ clang/test/SemaOpenCL/address-spaces.cl
@@ -231,12 +231,12 @@
 }
 #endif
 
-__private int func_return_priv(void);   //expected-error {{return value cannot be qualified with address space}}
-__global int func_return_global(void);  //expected-error {{return value cannot be qualified with address space}}
-__local int func_return_local(void);//expected-error {{return value cannot be qualified with address space}}
-__constant int func_return_constant(void);  //expected-error {{return value cannot be qualified with address space}}
+__private int func_return_priv(void);   //expected-error {{return type cannot be qualified with address space}}
+__global int func_return_global(void);  //expected-error {{return type cannot be qualified with address space}}
+__local int func_return_local(void);//expected-error {{return type cannot be qualified with address space}}
+__constant int func_return_constant(void);  //expected-error {{return type cannot be qualified with address space}}
 #if __OPENCL_C_VERSION__ >= 200
-__generic int func_return_generic(void);//expected-error {{return value cannot be qualified with address space}}
+__generic int func_return_generic(void);//expected-error {{return type cannot be qualified with address space}}
 #endif
 
 void func_multiple_addr(void) {
Index: clang/test/SemaHLSL/group_shared_202x.hlsl
===
--- /dev/null
+++ clang/test/SemaHLSL/group_shared_202x.hlsl
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -x hlsl -std=hlsl202x  -o - -fsyntax-only %s -verify
+
+// expected-error@+1 {{return type cannot be qualified with address space}}
+auto func() -> groupshared void;
+
+// expected-error@+1 {{parameter may not be qualified with an address space}}
+auto func(float group

[clang] c5d950f - [HLSL] Simplify code and fix unused variable warnings. NFC.

2022-10-13 Thread Benjamin Kramer via cfe-commits


Author: Benjamin Kramer
Date: 2022-10-13T09:46:32+02:00
New Revision: c5d950f4699ff0d9eee20fa144a2ef5f2deffa7b

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

LOG: [HLSL] Simplify code and fix unused variable warnings. NFC.

Added: 


Modified: 
clang/lib/CodeGen/CGHLSLRuntime.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGHLSLRuntime.cpp 
b/clang/lib/CodeGen/CGHLSLRuntime.cpp
index 245fe88c170f..e1011db758d6 100644
--- a/clang/lib/CodeGen/CGHLSLRuntime.cpp
+++ b/clang/lib/CodeGen/CGHLSLRuntime.cpp
@@ -98,16 +98,12 @@ GlobalVariable *replaceBuffer(CGHLSLRuntime::Buffer &Buf) {
   IRBuilder<> B(CBGV->getContext());
   Value *ZeroIdx = B.getInt32(0);
   // Replace Const use with CB use.
-  for (auto &Const : Buf.Constants) {
-llvm::Type *EltTy = Buf.LayoutStruct->getElementType(Const.second);
-GlobalVariable *GV = Const.first;
-unsigned Offset = Const.second;
-
+  for (auto &[GV, Offset]: Buf.Constants) {
 Value *GEP =
 B.CreateGEP(Buf.LayoutStruct, CBGV, {ZeroIdx, B.getInt32(Offset)});
 
-llvm::Type *GVTy = GV->getValueType();
-assert(EltTy == GVTy && "constant type mismatch");
+assert(Buf.LayoutStruct->getElementType(Offset) == GV->getValueType() &&
+   "constant type mismatch");
 
 // Replace.
 GV->replaceAllUsesWith(GEP);



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

< 1 2

101 - 189 of 189 matches

Mail list logo