[PATCH] D123319: Change how we handle auto return types for lambda operator() to be consistent with gcc

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D123319#3506745 , @shafik wrote:

>> Any update/further details here?
>
> David, apologies for not getting back to you sooner. The context is D105564 
>  which I started working on again recently. 
> I was having difficulties finding a solution that also worked for local 
> lambdas.

But what I don't understand is what makes local lambdas special. You can 
produce the same DWARF (so far as I can think of - maybe there's some 
distinction I'm missing?) without lambdas:

  void f1() {
auto x = [](){ return 3; };
x();
  }
  void f2() {
struct t1 {
  auto operator()() {
return 3;
  }
};
t1 v1;
v1();
  }

  $ clang++-tot test.cpp -g -c && llvm-dwarfdump-tot test.o | grep 
"DW_TAG\|DW_AT_type\|DW_AT_name" | sed -e "s/^...//"
  ...
 DW_TAG_subprogram
   DW_AT_name ("f1")
   DW_TAG_variable
 DW_AT_name   ("x")
 DW_AT_type   (0x003a "class ")
   DW_TAG_class_type
 DW_TAG_subprogram
   DW_AT_name ("operator()")
   DW_AT_type (0x0050 "int")
   DW_TAG_formal_parameter
 DW_AT_type   (0x0054 "const class  *")
  ...
 DW_TAG_subprogram
   DW_AT_specification(0x003f "operator()")
   DW_TAG_formal_parameter
 DW_AT_name   ("this")
 DW_AT_type   (0x00cc "const class  *")
 DW_TAG_subprogram
   DW_AT_name ("f2")
   DW_TAG_variable
 DW_AT_name   ("v1")
 DW_AT_type   (0x0090 "t1")
   DW_TAG_structure_type
 DW_AT_name   ("t1")
 DW_TAG_subprogram
   DW_AT_name ("operator()")
   DW_AT_type (0x00a6 "auto")
   DW_TAG_formal_parameter
 DW_AT_type   (0x00a8 "t1 *")
 DW_TAG_subprogram
   DW_AT_type (0x0050 "int")
   DW_AT_specification(0x0096 "operator()")
   DW_TAG_formal_parameter
 DW_AT_name   ("this")
 DW_AT_type   (0x00d1 "t1 *")
 DW_TAG_pointer_type
   DW_AT_type (0x0059 "const class ")
 DW_TAG_pointer_type
   DW_AT_type (0x0090 "t1")

So if there's particularly an issue with lambdas, it's probably also with the 
above local type example.

> I discovered eventually that what I had worked with debug-info that gcc 
> generated

Yep, at least testing GCC 11 just now - it seems GCC doesn't use "auto" for 
local types, be they named or unnamed.

> and realized that gcc was not emitting `auto` for lambdas and decided to 
> match gcc's behavior here since we often do that.

I think that's only a really sound argument when it's GDB 
compatibility/somewhat outside our control on the consumer side. I'm not sure 
there's a good argument for doing this for lldb when we can fix lldb to handle 
the right DWARF instead.

> I think an alternative approach would have been to emit `DW_AT_linkage_name` 
> for local lambdas but when I dug into that it looked like we were dropping 
> the linkage name several step before where would emit `DW_AT_linkage_name` 
> and it was not clear to me how easy a fix that was.

What does the linkage name do for your use case? Which cases are missing 
linkage names/where do they go missing?

> I am happy to consider other approaches as well to solving lookup for local 
> lambdas for D105564 . Let me know what you 
> think.

Why does the return type help perform lookup? What kind of lookup?

(again, my take is that "auto" return types probably shouldn't be described at 
all - we should just not describe functions where we don't know their return 
types because they're not very useful to know about (overload resolution, yes, 
but then you can't call them anyway) - but that's a broader argument to 
make/change to make)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123319

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

[PATCH] D125622: [clang-tidy] Reject invalid enum initializers in C files

[Richard via Phabricator via cfe-commits]

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



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/modernize-macro-to-enum.c:9
+
+// C forbids comma operator in initializing expressions.
+#define BAD_OP 1, 2

aaron.ballman wrote:
> LegalizeAdulthood wrote:
> > aaron.ballman wrote:
> > > It's also forbidden in C++.
> > C++ just talks about a constant expression and doesn't explicitly disallow 
> > `operator,` in such expressions that I could find.  Can you point me to 
> > where it is disallowed?
> > 
> > If it is disallowed universally, then I don't see why you asked me to parse 
> > it `:)`
> > C++ just talks about a constant expression and doesn't explicitly disallow 
> > operator, in such expressions that I could find. Can you point me to where 
> > it is disallowed?
> 
> It falls out from the grammar:
> 
> http://eel.is/c++draft/enum#nt:enumerator-definition
> http://eel.is/c++draft/expr.const#nt:constant-expression
> http://eel.is/c++draft/expr.cond#nt:conditional-expression
> 
> > If it is disallowed universally, then I don't see why you asked me to parse 
> > it :)
> 
> It can show up in paren expressions and the interface is generally about 
> integer literal expressions.
OK, yeah, I was looking at the PDF and it doesn't have those nice navigable 
links, I'll have to keep that site in mind.

So we have to disallow top-level comma but allow it as a parenthesized 
expression.  Tricky!



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/modernize-macro-to-enum.c:10
+// C forbids comma operator in initializing expressions.
+#define BAD_OP 1, 2
+

aaron.ballman wrote:
> Can you add a test:
> ```
> #define GOOD_OP (1, 2)
> ```
> to make sure it still gets converted to an enum?
Yeah, another one I was thinking of is when someone does [[ 
https://godbolt.org/z/c677je8s1 | something as disgusting as this. ]]

```
#define ARGS 1, 2
#define OTHER_ARGS (1, 2)

int f(int x, int y) { return x + y; }

int main()
{
return f(ARGS) + f OTHER_ARGS;
}
```

However, the only real way to handle avoiding conversion of the 2nd case is to 
examine the context of macro expansion.  This is another edge case that will 
have to be handled subsequently.

This gets tricky because AFAIK there is no way to select expressions in the AST 
that result from macro expansion.  You have to match the macro expansion 
locations against AST nodes to identify the node(s) that match the expansion 
location yourself.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125622

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

[PATCH] D125742: Minor refactor of CanonicalIncludes::addSystemHeadersMapping.

[Paul Pluzhnikov via Phabricator via cfe-commits]

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

Before commit b3a991df3cd6a SystemHeaderMap used to be a vector.

Commit b3a991df3cd6a changed it into a map, but neglected to remove
duplicate keys (e.g. "bits/typesizes.h", "include/stdint.h", etc.).

To prevent confusion, remove all duplicates, build HeaderMapping
one pair at a time and assert() that no duplicates are found.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125742

Files:
  clang-tools-extra/clangd/index/CanonicalIncludes.cpp

Index: clang-tools-extra/clangd/index/CanonicalIncludes.cpp
===
--- clang-tools-extra/clangd/index/CanonicalIncludes.cpp
+++ clang-tools-extra/clangd/index/CanonicalIncludes.cpp
@@ -18,6 +18,655 @@
 namespace clangd {
 namespace {
 const char IWYUPragma[] = "// IWYU pragma: private, include ";
+
+const std::pair IncludeMappings[] = {
+{"include/__stddef_max_align_t.h", ""},
+{"include/__wmmintrin_aes.h", ""},
+{"include/__wmmintrin_pclmul.h", ""},
+{"include/adxintrin.h", ""},
+{"include/ammintrin.h", ""},
+{"include/avx2intrin.h", ""},
+{"include/avx512bwintrin.h", ""},
+{"include/avx512cdintrin.h", ""},
+{"include/avx512dqintrin.h", ""},
+{"include/avx512erintrin.h", ""},
+{"include/avx512fintrin.h", ""},
+{"include/avx512ifmaintrin.h", ""},
+{"include/avx512ifmavlintrin.h", ""},
+{"include/avx512pfintrin.h", ""},
+{"include/avx512vbmiintrin.h", ""},
+{"include/avx512vbmivlintrin.h", ""},
+{"include/avx512vlbwintrin.h", ""},
+{"include/avx512vlcdintrin.h", ""},
+{"include/avx512vldqintrin.h", ""},
+{"include/avx512vlintrin.h", ""},
+{"include/avxintrin.h", ""},
+{"include/bmi2intrin.h", ""},
+{"include/bmiintrin.h", ""},
+{"include/emmintrin.h", ""},
+{"include/f16cintrin.h", ""},
+{"include/float.h", ""},
+{"include/fma4intrin.h", ""},
+{"include/fmaintrin.h", ""},
+{"include/fxsrintrin.h", ""},
+{"include/ia32intrin.h", ""},
+{"include/immintrin.h", ""},
+{"include/inttypes.h", ""},
+{"include/limits.h", ""},
+{"include/lzcntintrin.h", ""},
+{"include/mm3dnow.h", ""},
+{"include/mm_malloc.h", ""},
+{"include/mmintrin.h", ""},
+{"include/mwaitxintrin.h", ""},
+{"include/pkuintrin.h", ""},
+{"include/pmmintrin.h", ""},
+{"include/popcntintrin.h", ""},
+{"include/prfchwintrin.h", ""},
+{"include/rdseedintrin.h", ""},
+{"include/rtmintrin.h", ""},
+{"include/shaintrin.h", ""},
+{"include/smmintrin.h", ""},
+{"include/stdalign.h", ""},
+{"include/stdarg.h", ""},
+{"include/stdbool.h", ""},
+{"include/stddef.h", ""},
+{"include/stdint.h", ""},
+{"include/tbmintrin.h", ""},
+{"include/tmmintrin.h", ""},
+{"include/wmmintrin.h", ""},
+{"include/x86intrin.h", ""},
+{"include/xmmintrin.h", ""},
+{"include/xopintrin.h", ""},
+{"include/xsavecintrin.h", ""},
+{"include/xsaveintrin.h", ""},
+{"include/xsaveoptintrin.h", ""},
+{"include/xsavesintrin.h", ""},
+{"include/xtestintrin.h", ""},
+{"include/_G_config.h", ""},
+{"include/assert.h", ""},
+{"algorithm", ""},
+{"valarray", ""},
+{"array", ""},
+{"atomic", ""},
+{"backward/auto_ptr.h", ""},
+{"backward/binders.h", ""},
+{"bits/algorithmfwd.h", ""},
+{"bits/alloc_traits.h", ""},
+{"bits/allocated_ptr.h", ""},
+{"bits/allocator.h", ""},
+{"bits/atomic_base.h", ""},
+{"bits/atomic_lockfree_defines.h", ""},
+{"bits/atomic_futex.h", ""},
+{"bits/basic_ios.h", ""},
+{"bits/basic_ios.tcc", ""},
+{"bits/basic_string.h", ""},
+{"bits/basic_string.tcc", ""},
+{"bits/char_traits.h", ""},
+{"bits/codecvt.h", ""},
+{"bits/concept_check.h", ""},
+{"bits/cpp_type_traits.h", ""},
+{"bits/cxxabi_forced.h", ""},
+{"bits/deque.tcc", ""},
+{"bits/exception.h", ""},
+{"bits/exception_defines.h", ""},
+{"bits/exception_ptr.h", ""},
+{"bits/forward_list.h", ""},
+{"bits/forward_list.tcc", ""},
+{"bits/fstream.tcc", ""},
+{"bits/functexcept.h", ""},
+{"bits/functional_hash.h", ""},
+{"bits/gslice.h", ""},
+{"bits/gslice_array.h", ""},
+{"bits/hash_bytes.h", ""},
+{"bits/hashtable.h", ""},
+{"bits/hashtable_policy.h", ""},
+{"bits/indirect_array.h", ""},
+{"bits/invoke.h", ""},
+{"bits/ios_base.h", ""},
+{"bits/istream.tcc", ""},
+{"bits/list.tcc", ""},
+{"bits/locale_classes.h", ""},
+{"bits/locale_classes.tcc", ""},
+{"bits/locale_conv.h", ""},
+{"bits/locale_facets.h", ""},
+{"bits/locale_facets.tcc", ""},
+

[PATCH] D125402: [clang][diag] warn if function returns class type by-const-value

[Namgoo Lee via Phabricator via cfe-commits]

nlee added a comment.

In D125402#3508845 , @aaron.ballman 
wrote:

> Thank you for working on this new diagnostic! We don't typically add new 
> diagnostics that are off by default unless they're for pedantic diagnostics 
> or are reasonably expected to be enabled by default in the future. This is 
> because we've had evidence that suggests such diagnostics aren't enabled 
> often enough to warrant the maintenance cost for them.
>
> I'm not convinced this diagnostic can be enabled by default. Yes, it prevents 
> move semantics, but that's not typically an issue with the correctness of the 
> code. IIRC, we decided to put this diagnostic into clang-tidy because we 
> thought it would be too chatty in practice, especially on older code bases.
>
> Perhaps there are ways we can improve the diagnostic behavior to allow it to 
> be enabled by default though. One possibility would be to look at the return 
> type to see if there's a user-provided move constructor (either `= default` 
> or explicitly written) and only diagnose if one is found. But I think we'd 
> want some evidence that this actually reduces the false positive rate in 
> practice, which means trying to compile some large C++ projects (of various 
> ages/code quality) to see. Would you be willing to run your patch against 
> some large C++ projects to see what kind of true and false positives it 
> generates? From there, we can decide whether we need additional heuristics or 
> not.

How about adding CXXRecordDecl::hasMoveConstructor() to ensure the type is 
movable? I ran a test with OpenCV(c++11), and it returned some useful warnings:

  In file included from 
/Users/nlee/Documents/opencv/modules/core/include/opencv2/core/base.hpp:662:
  
/Users/nlee/Documents/opencv/modules/core/include/opencv2/core/check.hpp:16:12: 
warning: returning by const value prevents move semantics 
[-Wreturn-by-const-value]
  CV_EXPORTS const String typeToString(int type);
 ^~
  
/Users/nlee/Documents/opencv/modules/core/include/opencv2/core/check.hpp:26:12: 
warning: returning by const value prevents move semantics 
[-Wreturn-by-const-value]
  CV_EXPORTS const cv::String typeToString_(int type);
 ^~

@aaron.ballman Can you suggest some other projects to test? I'm thinking of 
these: protobuf, pytorch

> I wonder if this is same concern applies to Unions?  Should this just be 
> `T->isRecordType()`?  Should we do more analysis to ensure that this is a 
> movable type? I THINK `CXXRecordDecl::defaultedMoveConstructorIsDeleted()` 
> should be enough to test for t his.

@erichkeane Does adding `CXXRecordDecl::hasMoveConstructor()` suffices to 
ensure the type is movable? I left out unions because they may alert false 
positives. An example of such a case is:

  union U { int i; std::string s; };
  const U f() { return U{239}; }




Comment at: clang/lib/Sema/SemaType.cpp:5203
+  T.isConstQualified() &&
+  T->isStructureOrClassType()) {
+const SourceLocation ConstLoc = D.getDeclSpec().getConstSpecLoc();

erichkeane wrote:
> I wonder if this is same concern applies to Unions?  Should this just be 
> `T->isRecordType()`?  Should we do more analysis to ensure that this is a 
> movable type? I THINK `CXXRecordDecl::defaultedMoveConstructorIsDeleted()` 
> should be enough to test for t his.
How about adding `CXXRecordDecl::hasMoveConstructor()` to ensure the type is 
movable? I left out unions because they may alert false positives. An example 
of such a case is:
```
union U { int i; std::string s; };
const U f() { return U{239}; }
```



Comment at: clang/lib/Sema/SemaType.cpp:5203
+  T.isConstQualified() &&
+  T->isStructureOrClassType()) {
+const SourceLocation ConstLoc = D.getDeclSpec().getConstSpecLoc();

nlee wrote:
> erichkeane wrote:
> > I wonder if this is same concern applies to Unions?  Should this just be 
> > `T->isRecordType()`?  Should we do more analysis to ensure that this is a 
> > movable type? I THINK `CXXRecordDecl::defaultedMoveConstructorIsDeleted()` 
> > should be enough to test for t his.
> How about adding `CXXRecordDecl::hasMoveConstructor()` to ensure the type is 
> movable? I left out unions because they may alert false positives. An example 
> of such a case is:
> ```
> union U { int i; std::string s; };
> const U f() { return U{239}; }
> ```
> I wonder if this is same concern applies to Unions?  Should this just be 
> `T->isRecordType()`?  Should we do more analysis to ensure that this is a 
> movable type? I THINK `CXXRecordDecl::defaultedMoveConstructorIsDeleted()` 
> should be enough to test for t his.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125402

___
cfe-commits mailing list
cfe-commits@lists.llvm.org

[PATCH] D125517: [Frontend] [Coroutines] Emit error when we found incompatible allocation function in promise_type

[Chuanqi Xu 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 rG452fac9534c0: [Frontend] [Coroutines] Emit error when we 
found incompatible allocation (authored by ChuanqiXu).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125517

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaCoroutine.cpp
  clang/test/SemaCXX/coroutine-allocs.cpp

Index: clang/test/SemaCXX/coroutine-allocs.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/coroutine-allocs.cpp
@@ -0,0 +1,61 @@
+// RUN: %clang_cc1 %s -std=c++20 -fsyntax-only -verify
+#include "Inputs/std-coroutine.h"
+
+namespace std {
+typedef decltype(sizeof(int)) size_t;
+}
+
+struct Allocator {};
+
+struct resumable {
+  struct promise_type {
+void *operator new(std::size_t sz, Allocator &);
+
+resumable get_return_object() { return {}; }
+auto initial_suspend() { return std::suspend_always(); }
+auto final_suspend() noexcept { return std::suspend_always(); }
+void unhandled_exception() {}
+void return_void(){};
+  };
+};
+
+resumable f1() { // expected-error {{'operator new' provided by 'std::coroutine_traits::promise_type' (aka 'resumable::promise_type') is not usable}}
+  co_return;
+}
+
+// NOTE: Although the argument here is a rvalue reference and the corresponding
+// allocation function in resumable::promise_type have lvalue references, it looks
+// the signature of f2 is invalid. But according to [dcl.fct.def.coroutine]p4:
+//
+//   In the following, pi is an lvalue of type Pi, where p1 denotes the object 
+//   parameter and pi+1 denotes the ith non-object function parameter for a
+//   non-static member function.
+//
+// And [dcl.fct.def.coroutine]p9.1
+//
+//   overload resolution is performed on a function call created by assembling an argument list.
+//   The first argument is the amount of space requested, and has type std::size_­t.
+//   The lvalues p1…pn are the succeeding arguments.
+//
+// So the acctual type passed to resumable::promise_type::operator new is lvalue
+// Allocator. It is allowed  to convert a lvalue to a lvalue reference. So the 
+// following one is valid.
+resumable f2(Allocator &&) {
+  co_return;
+}
+
+resumable f3(Allocator &) {
+  co_return;
+}
+
+resumable f4(Allocator) {
+  co_return;
+}
+
+resumable f5(const Allocator) { // expected-error {{operator new' provided by 'std::coroutine_traits::promise_type' (aka 'resumable::promise_type') is not usable}}
+  co_return;
+}
+
+resumable f6(const Allocator &) { // expected-error {{operator new' provided by 'std::coroutine_traits::promise_type' (aka 'resumable::promise_type') is not usable}}
+  co_return;
+}
Index: clang/lib/Sema/SemaCoroutine.cpp
===
--- clang/lib/Sema/SemaCoroutine.cpp
+++ clang/lib/Sema/SemaCoroutine.cpp
@@ -1308,10 +1308,33 @@
 
 PlacementArgs.push_back(PDRefExpr.get());
   }
-  S.FindAllocationFunctions(Loc, SourceRange(), /*NewScope*/ Sema::AFS_Class,
-/*DeleteScope*/ Sema::AFS_Both, PromiseType,
-/*isArray*/ false, PassAlignment, PlacementArgs,
-OperatorNew, UnusedResult, /*Diagnose*/ false);
+
+  bool PromiseContainNew = [this, ]() -> bool {
+DeclarationName NewName =
+S.getASTContext().DeclarationNames.getCXXOperatorName(OO_New);
+LookupResult R(S, NewName, Loc, Sema::LookupOrdinaryName);
+
+if (PromiseType->isRecordType())
+  S.LookupQualifiedName(R, PromiseType->getAsCXXRecordDecl());
+
+return !R.empty() && !R.isAmbiguous();
+  }();
+
+  auto LookupAllocationFunction = [&]() {
+// [dcl.fct.def.coroutine]p9
+//   The allocation function's name is looked up by searching for it in the
+// scope of the promise type.
+// - If any declarations are found, ...
+// - Otherwise, a search is performed in the global scope.
+Sema::AllocationFunctionScope NewScope = PromiseContainNew ? Sema::AFS_Class : Sema::AFS_Global;
+S.FindAllocationFunctions(Loc, SourceRange(),
+  NewScope,
+  /*DeleteScope*/ Sema::AFS_Both, PromiseType,
+  /*isArray*/ false, PassAlignment, PlacementArgs,
+  OperatorNew, UnusedResult, /*Diagnose*/ false);
+  };
+
+  LookupAllocationFunction();
 
   // [dcl.fct.def.coroutine]p9
   //   If no viable function is found ([over.match.viable]), overload resolution
@@ -1319,22 +1342,7 @@
   // space required as an argument of type std::size_t.
   if (!OperatorNew && !PlacementArgs.empty()) {
 PlacementArgs.clear();
-S.FindAllocationFunctions(Loc, SourceRange(), /*NewScope*/ Sema::AFS_Class,
- 

[clang] 452fac9 - [Frontend] [Coroutines] Emit error when we found incompatible allocation

[Chuanqi Xu via cfe-commits]

Author: Chuanqi Xu
Date: 2022-05-17T10:36:21+08:00
New Revision: 452fac9534c00290e92819202d445810e33d0444

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

LOG: [Frontend] [Coroutines] Emit error when we found incompatible allocation
function in promise_type

According to https://cplusplus.github.io/CWG/issues/2585.html, this
fixes https://github.com/llvm/llvm-project/issues/54881

Simply, the clang tried to found (do lookup and overload resolution. Is
there any better word to use than found?) allocation function in
promise_type and global scope. However, this is not consistent with the
standard. The standard behavior would be that the compiler shouldn't
lookup in global scope in case we lookup the allocation function name in
promise_type. In other words, the program is ill-formed if there is
incompatible allocation function in promise type.

Reviewed By: erichkeane

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

Added: 
clang/test/SemaCXX/coroutine-allocs.cpp

Modified: 
clang/docs/ReleaseNotes.rst
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaCoroutine.cpp

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 59c42384c4ed7..8058df16bc4c3 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -149,6 +149,9 @@ Bug Fixes
   because there is no way to fully qualify the enumerator name, so this
   "extension" was unintentional and useless. This fixes
   `Issue 42372 `_.
+- Clang shouldn't lookup allocation function in global scope for coroutines
+  in case it found the allocation function name in the promise_type body.
+  This fixes Issue `Issue 54881 
`_.
 
 Improvements to Clang's diagnostics
 ^^^

diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 6dcf2eafbf210..e03c583c63fe4 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -11245,6 +11245,9 @@ def warn_always_inline_coroutine : Warning<
   "this coroutine may be split into pieces; not every piece is guaranteed to 
be inlined"
   >,
   InGroup;
+def err_coroutine_unusable_new : Error<
+  "'operator new' provided by %0 is not usable with the function signature of 
%1"
+>;
 } // end of coroutines issue category
 
 let CategoryName = "Documentation Issue" in {

diff  --git a/clang/lib/Sema/SemaCoroutine.cpp 
b/clang/lib/Sema/SemaCoroutine.cpp
index 44a7271910cf1..8709e7bed207a 100644
--- a/clang/lib/Sema/SemaCoroutine.cpp
+++ b/clang/lib/Sema/SemaCoroutine.cpp
@@ -1308,10 +1308,33 @@ bool CoroutineStmtBuilder::makeNewAndDeleteExpr() {
 
 PlacementArgs.push_back(PDRefExpr.get());
   }
-  S.FindAllocationFunctions(Loc, SourceRange(), /*NewScope*/ Sema::AFS_Class,
-/*DeleteScope*/ Sema::AFS_Both, PromiseType,
-/*isArray*/ false, PassAlignment, PlacementArgs,
-OperatorNew, UnusedResult, /*Diagnose*/ false);
+
+  bool PromiseContainNew = [this, ]() -> bool {
+DeclarationName NewName =
+S.getASTContext().DeclarationNames.getCXXOperatorName(OO_New);
+LookupResult R(S, NewName, Loc, Sema::LookupOrdinaryName);
+
+if (PromiseType->isRecordType())
+  S.LookupQualifiedName(R, PromiseType->getAsCXXRecordDecl());
+
+return !R.empty() && !R.isAmbiguous();
+  }();
+
+  auto LookupAllocationFunction = [&]() {
+// [dcl.fct.def.coroutine]p9
+//   The allocation function's name is looked up by searching for it in the
+// scope of the promise type.
+// - If any declarations are found, ...
+// - Otherwise, a search is performed in the global scope.
+Sema::AllocationFunctionScope NewScope = PromiseContainNew ? 
Sema::AFS_Class : Sema::AFS_Global;
+S.FindAllocationFunctions(Loc, SourceRange(),
+  NewScope,
+  /*DeleteScope*/ Sema::AFS_Both, PromiseType,
+  /*isArray*/ false, PassAlignment, PlacementArgs,
+  OperatorNew, UnusedResult, /*Diagnose*/ false);
+  };
+
+  LookupAllocationFunction();
 
   // [dcl.fct.def.coroutine]p9
   //   If no viable function is found ([over.match.viable]), overload 
resolution
@@ -1319,22 +1342,7 @@ bool CoroutineStmtBuilder::makeNewAndDeleteExpr() {
   // space required as an argument of type std::size_t.
   if (!OperatorNew && !PlacementArgs.empty()) {
 PlacementArgs.clear();
-S.FindAllocationFunctions(Loc, SourceRange(), /*NewScope*/ Sema::AFS_Class,
-  

[PATCH] D124977: [NFC][Clang] Modify expect of fail test or XFAIL because CSKY align is different

[Zixuan Wu via Phabricator via cfe-commits]

zixuan-wu added a comment.

Gentle pin..


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

https://reviews.llvm.org/D124977

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

[PATCH] D123534: [dwarf] Emit a DIGlobalVariable for constant strings.

[Paul Kirth via Phabricator via cfe-commits]

paulkirth added a comment.

Thanks for the quick response.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123534

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

[PATCH] D123534: [dwarf] Emit a DIGlobalVariable for constant strings.

[Mitch Phillips via Phabricator via cfe-commits]

hctim added a comment.

Done, I'll take a look and resubmit later.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123534

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

[clang] ed2c321 - Revert "[dwarf] Emit a DIGlobalVariable for constant strings."

[Mitch Phillips via cfe-commits]

Author: Mitch Phillips
Date: 2022-05-16T19:07:38-07:00
New Revision: ed2c3218f5badf88cb7897fabf8faa01e8aa2044

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

LOG: Revert "[dwarf] Emit a DIGlobalVariable for constant strings."

This reverts commit 4680982b36a84770a1600fc438be8ec090671724.

Broke a fuchsia windows bot. More details in the review:
https://reviews.llvm.org/D123534

Added: 
llvm/test/Assembler/invalid-diglobalvariable-missing-name.ll

Modified: 
clang/lib/CodeGen/CGDebugInfo.cpp
clang/lib/CodeGen/CGDebugInfo.h
clang/lib/CodeGen/CodeGenModule.cpp
clang/test/VFS/external-names.c
llvm/lib/AsmParser/LLParser.cpp
llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp

Removed: 
clang/test/CodeGen/debug-info-variables.c
llvm/test/DebugInfo/COFF/global-no-strings.ll



diff  --git a/clang/lib/CodeGen/CGDebugInfo.cpp 
b/clang/lib/CodeGen/CGDebugInfo.cpp
index 7534270294415..3d73bfb8ce793 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -5132,7 +5132,7 @@ std::string CGDebugInfo::GetName(const Decl *D, bool 
Qualified) const {
 return Name;
   codegenoptions::DebugTemplateNamesKind TemplateNamesKind =
   CGM.getCodeGenOpts().getDebugSimpleTemplateNames();
-
+  
   if (!CGM.getCodeGenOpts().hasReducedDebugInfo())
 TemplateNamesKind = codegenoptions::DebugTemplateNamesKind::Full;
 
@@ -5459,21 +5459,6 @@ void CGDebugInfo::EmitGlobalAlias(const 
llvm::GlobalValue *GV,
   ImportedDeclCache[GD.getCanonicalDecl().getDecl()].reset(ImportDI);
 }
 
-void CGDebugInfo::AddStringLiteralDebugInfo(llvm::GlobalVariable *GV,
-const StringLiteral *S) {
-  SourceLocation Loc = S->getStrTokenLoc(0);
-  PresumedLoc PLoc = CGM.getContext().getSourceManager().getPresumedLoc(Loc);
-  if (!PLoc.isValid())
-return;
-
-  llvm::DIFile *File = getOrCreateFile(Loc);
-  llvm::DIGlobalVariableExpression *Debug =
-  DBuilder.createGlobalVariableExpression(
-  nullptr, StringRef(), StringRef(), getOrCreateFile(Loc),
-  getLineNumber(Loc), getOrCreateType(S->getType(), File), true);
-  GV->addDebugInfo(Debug);
-}
-
 llvm::DIScope *CGDebugInfo::getCurrentContextDescriptor(const Decl *D) {
   if (!LexicalBlockStack.empty())
 return LexicalBlockStack.back();

diff  --git a/clang/lib/CodeGen/CGDebugInfo.h b/clang/lib/CodeGen/CGDebugInfo.h
index 38e3fa5b2fa96..8984f3eb1d7a0 100644
--- a/clang/lib/CodeGen/CGDebugInfo.h
+++ b/clang/lib/CodeGen/CGDebugInfo.h
@@ -533,14 +533,6 @@ class CGDebugInfo {
   /// Emit an @import declaration.
   void EmitImportDecl(const ImportDecl );
 
-  /// DebugInfo isn't attached to string literals by default. While certain
-  /// aspects of debuginfo aren't useful for string literals (like a name), 
it's
-  /// nice to be able to symbolize the line and column information. This is
-  /// especially useful for sanitizers, as it allows symbolization of
-  /// heap-buffer-overflows on constant strings.
-  void AddStringLiteralDebugInfo(llvm::GlobalVariable *GV,
- const StringLiteral *S);
-
   /// Emit C++ namespace alias.
   llvm::DIImportedEntity *EmitNamespaceAlias(const NamespaceAliasDecl );
 

diff  --git a/clang/lib/CodeGen/CodeGenModule.cpp 
b/clang/lib/CodeGen/CodeGenModule.cpp
index 703cf4edf5f56..f8bf210dc0e21 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -5670,11 +5670,6 @@ CodeGenModule::GetAddrOfConstantStringFromLiteral(const 
StringLiteral *S,
   }
 
   auto GV = GenerateStringLiteral(C, LT, *this, GlobalVariableName, Alignment);
-
-  CGDebugInfo *DI = getModuleDebugInfo();
-  if (DI && getCodeGenOpts().hasReducedDebugInfo())
-DI->AddStringLiteralDebugInfo(GV, S);
-
   if (Entry)
 *Entry = GV;
 

diff  --git a/clang/test/CodeGen/debug-info-variables.c 
b/clang/test/CodeGen/debug-info-variables.c
deleted file mode 100644
index 1e3b4fb979d7a..0
--- a/clang/test/CodeGen/debug-info-variables.c
+++ /dev/null
@@ -1,19 +0,0 @@
-// RUN: %clang_cc1 %s -debug-info-kind=standalone -S -emit-llvm -o - | 
FileCheck %s
-
-// CHECK: DIGlobalVariable(name: "global",{{.*}} line: [[@LINE+1]]
-int global = 42;
-
-// CHECK: DIGlobalVariable({{.*}}line: [[@LINE+4]],{{.*}} type: 
[[TYPEID:![0-9]+]]
-// CHECK: [[TYPEID]] = !DICompositeType(tag: DW_TAG_array_type, baseType: 
[[BASETYPE:![0-9]+]]
-// CHECK: [[BASETYPE]] = !DIBasicType(name: "char"
-const char* s() {
-  return "1234567890";
-}
-
-// CHECK: DILocalVariable(name: "p"
-// CHECK: DILocalVariable(name: "q"
-// CHECK: DILocalVariable(name: "r"
-int sum(int p, int q) {
-  int r = p + q;
-  return r;
-}

diff  --git a/clang/test/VFS/external-names.c 

[PATCH] D123534: [dwarf] Emit a DIGlobalVariable for constant strings.

[Paul Kirth via Phabricator via cfe-commits]

paulkirth added a comment.

Hi, we're seeing a breakage in Fuchsia's clang CI for x64 windows that I think 
is related to this patch.

https://luci-milo.appspot.com/ui/p/fuchsia/builders/toolchain.ci/clang-windows-x64/b8813962058917346337/overview

We see a test failure in Clang :: CodeGen/debug-info-variables.c, which this 
patch modifies.

If this will be hard to fix, would you mind reverting until a fix is ready?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123534

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

[PATCH] D124435: [X86] Always extend the integer parameters in callee

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

I'm fine with alternatives on the names.  I just don't think we want names that 
imply that the caller and callee are parallel here, as if there were some sort 
of requirement that the callee always extends.  In those conventions, the 
callee gets passed 8 or 16 meaningful bits; if it wants an extended value, it's 
responsible for doing that extension, just like it would be responsible for 
doing the opposite extension if it wanted that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124435

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

[PATCH] D124806: [clang-tidy] add support for Demorgan conversions to readability-simplify-bool-expr

[Nathan James via Phabricator via cfe-commits]

njames93 updated this revision to Diff 429907.
njames93 added a comment.

Extend fix-its to also support removing parens where possible.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124806

Files:
  clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
  clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/readability-simplify-boolean-expr.rst
  
clang-tools-extra/test/clang-tidy/checkers/readability-simplify-bool-expr-demorgan.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability-simplify-bool-expr-demorgan.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability-simplify-bool-expr-demorgan.cpp
@@ -0,0 +1,87 @@
+// RUN: %check_clang_tidy %s readability-simplify-boolean-expr %t
+
+void eat(bool);
+
+void foo(bool A1, bool A2, bool A3, bool A4) {
+  bool X;
+  X = !(!A1 || A2);
+  X = !(A1 || !A2);
+  X = !(!A1 || !A2);
+  // CHECK-MESSAGES: :[[@LINE-3]]:7: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-MESSAGES: :[[@LINE-3]]:7: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-MESSAGES: :[[@LINE-3]]:7: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-FIXES: X = A1 && !A2;
+  // CHECK-FIXES-NEXT: X = !A1 && A2;
+  // CHECK-FIXES-NEXT: X = A1 && A2;
+
+  X = !(!A1 && A2);
+  X = !(A1 && !A2);
+  X = !(!A1 && !A2);
+  // CHECK-MESSAGES: :[[@LINE-3]]:7: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-MESSAGES: :[[@LINE-3]]:7: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-MESSAGES: :[[@LINE-3]]:7: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-FIXES: X = A1 || !A2;
+  // CHECK-FIXES-NEXT: X = !A1 || A2;
+  // CHECK-FIXES-NEXT: X = A1 || A2;
+
+  X = !(!A1 && !A2 && !A3);
+  X = !(!A1 && (!A2 && !A3));
+  X = !(!A1 && (A2 && A3));
+  // CHECK-MESSAGES: :[[@LINE-3]]:7: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-MESSAGES: :[[@LINE-3]]:7: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-MESSAGES: :[[@LINE-3]]:7: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-FIXES: X = A1 || A2 || A3;
+  // CHECK-FIXES-NEXT: X = A1 || A2 || A3;
+  // CHECK-FIXES-NEXT: X = A1 || !A2 || !A3;
+
+  X = !(A1 && A2 == A3);
+  X = !(!A1 && A2 > A3);
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-FIXES: X = !A1 || A2 != A3;
+  // CHECK-FIXES-NEXT: X = A1 || A2 <= A3;
+
+  // Ensure the check doesn't try to combine fixes for the inner and outer demorgan simplification.
+  X = !(!A1 && !(!A2 && !A3));
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-MESSAGES: :[[@LINE-2]]:16: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-FIXES: X = A1 || (!A2 && !A3);
+
+  // Testing to see how it handles parens
+  X = !(A1 && !A2 && !A3);
+  X = !(A1 && !A2 || !A3);
+  X = !(!A1 || A2 && !A3);
+  X = !((A1 || !A2) && !A3);
+  X = !((A1 || !A2) || !A3);
+  // CHECK-MESSAGES: :[[@LINE-5]]:7: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-MESSAGES: :[[@LINE-5]]:7: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-MESSAGES: :[[@LINE-5]]:7: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-MESSAGES: :[[@LINE-5]]:7: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-MESSAGES: :[[@LINE-5]]:7: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-FIXES: X = !A1 || A2 || A3;
+  // CHECK-FIXES-NEXT: X = (!A1 || A2) && A3;
+  // CHECK-FIXES-NEXT: X = A1 && (!A2 || A3);
+  // CHECK-FIXES-NEXT: X = (!A1 && A2) || A3;
+  // CHECK-FIXES-NEXT: X = !A1 && A2 && A3;
+  X = !((A1 || A2) && (!A3 || A4));
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-FIXES: X = (!A1 && !A2) || (A3 && !A4);
+
+  eat(!(!A1 && !A2));
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-FIXES: eat(A1 || A2);
+
+  bool Init = !(!A1 || !A2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-FIXES: bool Init = A1 && A2;
+
+  X = A1 && !(!A2 || !A3);
+  X = A1 || !(!A2 || !A3);
+  X = A1 && !(!A2 && !A3);
+  // CHECK-MESSAGES: :[[@LINE-3]]:13: warning: boolean expression can be simplified by 

[PATCH] D123534: [dwarf] Emit a DIGlobalVariable for constant strings.

[Mitch Phillips 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 rG4680982b36a8: [dwarf] Emit a DIGlobalVariable for constant 
strings. (authored by hctim).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123534

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGDebugInfo.h
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGen/debug-info-variables.c
  clang/test/VFS/external-names.c
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
  llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
  llvm/test/Assembler/invalid-diglobalvariable-missing-name.ll
  llvm/test/DebugInfo/COFF/global-no-strings.ll

Index: llvm/test/DebugInfo/COFF/global-no-strings.ll
===
--- /dev/null
+++ llvm/test/DebugInfo/COFF/global-no-strings.ll
@@ -0,0 +1,59 @@
+; RUN: llc < %s | FileCheck %s
+; RUN: llc < %s -filetype=obj | llvm-readobj - --codeview | FileCheck %s
+
+; This test ensures that no debuginfo is emitted for the constant "123456789"
+; string. These global variables have debug expressions because DWARF has the
+; ability to tie them to a file name and line number, but this isn't possible
+; in CodeView, so we make sure it's omitted to save file size.
+;
+; The various CodeView outputs should have a description of "my_string", but not
+; the string contents itself.
+
+; C++ source to regenerate:
+; $ cat a.cpp
+; char* my_string =
+;   "12345679";
+;
+; $ clang-cl a.cpp /GS- /Z7 /GR- /clang:-S /clang:-emit-llvm
+
+; CHECK-NOT: S_LDATA
+; CHECK: S_GDATA
+; CHECK-NOT: S_LDATA
+; CHECK-NOT: S_GDATA
+
+; ModuleID = '/tmp/a.c'
+source_filename = "/tmp/a.c"
+target datalayout = "e-m:w-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-pc-windows-msvc19.20.0"
+
+$"??_C@_08HCBFHPJA@12345679?$AA@" = comdat any
+
+@"??_C@_08HCBFHPJA@12345679?$AA@" = linkonce_odr dso_local unnamed_addr constant [9 x i8] c"12345679\00", comdat, align 1, !dbg !0
+@my_string = dso_local global ptr @"??_C@_08HCBFHPJA@12345679?$AA@", align 8, !dbg !7
+
+!llvm.dbg.cu = !{!9}
+!llvm.linker.options = !{!13, !14}
+!llvm.module.flags = !{!15, !16, !17, !18, !19}
+!llvm.ident = !{!20}
+
+!0 = !DIGlobalVariableExpression(var: !1, expr: !DIExpression())
+!1 = distinct !DIGlobalVariable(scope: null, file: !2, line: 2, type: !3, isLocal: true, isDefinition: true)
+!2 = !DIFile(filename: "/tmp/a.c", directory: "", checksumkind: CSK_MD5, checksum: "b972961d64de3c90497767110ab58eb6")
+!3 = !DICompositeType(tag: DW_TAG_array_type, baseType: !4, size: 72, elements: !5)
+!4 = !DIBasicType(name: "char", size: 8, encoding: DW_ATE_signed_char)
+!5 = !{!6}
+!6 = !DISubrange(count: 9)
+!7 = !DIGlobalVariableExpression(var: !8, expr: !DIExpression())
+!8 = distinct !DIGlobalVariable(name: "my_string", scope: !9, file: !2, line: 1, type: !12, isLocal: false, isDefinition: true)
+!9 = distinct !DICompileUnit(language: DW_LANG_C99, file: !10, producer: "clang version 15.0.0 (https://github.com/llvm/llvm-project.git 7c1c0849f8a1a6f1bf5f5b554484bbf8b0debd0a)", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, globals: !11, splitDebugInlining: false, nameTableKind: None)
+!10 = !DIFile(filename: "/tmp/a.c", directory: "/usr/local/google/home/mitchp/llvm-build/opt", checksumkind: CSK_MD5, checksum: "b972961d64de3c90497767110ab58eb6")
+!11 = !{!0, !7}
+!12 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !4, size: 64)
+!13 = !{!"/DEFAULTLIB:libcmt.lib"}
+!14 = !{!"/DEFAULTLIB:oldnames.lib"}
+!15 = !{i32 2, !"CodeView", i32 1}
+!16 = !{i32 2, !"Debug Info Version", i32 3}
+!17 = !{i32 1, !"wchar_size", i32 2}
+!18 = !{i32 7, !"PIC Level", i32 2}
+!19 = !{i32 7, !"uwtable", i32 2}
+!20 = !{!"clang version 15.0.0 (https://github.com/llvm/llvm-project.git 7c1c0849f8a1a6f1bf5f5b554484bbf8b0debd0a)"}
Index: llvm/test/Assembler/invalid-diglobalvariable-missing-name.ll
===
--- llvm/test/Assembler/invalid-diglobalvariable-missing-name.ll
+++ /dev/null
@@ -1,4 +0,0 @@
-; RUN: not llvm-as < %s -disable-output 2>&1 | FileCheck %s
-
-; CHECK: :[[@LINE+1]]:24: error: missing required field 'name'
-!0 = !DIGlobalVariable()
Index: llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
===
--- llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
+++ llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
@@ -165,7 +165,9 @@
   } else {
 DeclContext = GV->getScope();
 // Add name and type.
-addString(*VariableDIE, dwarf::DW_AT_name, GV->getDisplayName());
+StringRef DisplayName = GV->getDisplayName();
+if (!DisplayName.empty())
+  addString(*VariableDIE, dwarf::DW_AT_name, GV->getDisplayName());
 if (GTy)
   addType(*VariableDIE, 

[clang] 4680982 - [dwarf] Emit a DIGlobalVariable for constant strings.

[Mitch Phillips via cfe-commits]

Author: Mitch Phillips
Date: 2022-05-16T16:52:16-07:00
New Revision: 4680982b36a84770a1600fc438be8ec090671724

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

LOG: [dwarf] Emit a DIGlobalVariable for constant strings.

An upcoming patch will extend llvm-symbolizer to provide the source line
information for global variables. The goal is to move AddressSanitizer
off of internal debug info for symbolization onto the DWARF standard
(and doing a clean-up in the process). Currently, ASan reports the line
information for constant strings if a memory safety bug happens around
them. We want to keep this behaviour, so we need to emit debuginfo for
these variables as well.

Reviewed By: dblaikie, rnk, aprantl

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

Added: 
clang/test/CodeGen/debug-info-variables.c
llvm/test/DebugInfo/COFF/global-no-strings.ll

Modified: 
clang/lib/CodeGen/CGDebugInfo.cpp
clang/lib/CodeGen/CGDebugInfo.h
clang/lib/CodeGen/CodeGenModule.cpp
clang/test/VFS/external-names.c
llvm/lib/AsmParser/LLParser.cpp
llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp

Removed: 
llvm/test/Assembler/invalid-diglobalvariable-missing-name.ll



diff  --git a/clang/lib/CodeGen/CGDebugInfo.cpp 
b/clang/lib/CodeGen/CGDebugInfo.cpp
index 3d73bfb8ce793..7534270294415 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -5132,7 +5132,7 @@ std::string CGDebugInfo::GetName(const Decl *D, bool 
Qualified) const {
 return Name;
   codegenoptions::DebugTemplateNamesKind TemplateNamesKind =
   CGM.getCodeGenOpts().getDebugSimpleTemplateNames();
-  
+
   if (!CGM.getCodeGenOpts().hasReducedDebugInfo())
 TemplateNamesKind = codegenoptions::DebugTemplateNamesKind::Full;
 
@@ -5459,6 +5459,21 @@ void CGDebugInfo::EmitGlobalAlias(const 
llvm::GlobalValue *GV,
   ImportedDeclCache[GD.getCanonicalDecl().getDecl()].reset(ImportDI);
 }
 
+void CGDebugInfo::AddStringLiteralDebugInfo(llvm::GlobalVariable *GV,
+const StringLiteral *S) {
+  SourceLocation Loc = S->getStrTokenLoc(0);
+  PresumedLoc PLoc = CGM.getContext().getSourceManager().getPresumedLoc(Loc);
+  if (!PLoc.isValid())
+return;
+
+  llvm::DIFile *File = getOrCreateFile(Loc);
+  llvm::DIGlobalVariableExpression *Debug =
+  DBuilder.createGlobalVariableExpression(
+  nullptr, StringRef(), StringRef(), getOrCreateFile(Loc),
+  getLineNumber(Loc), getOrCreateType(S->getType(), File), true);
+  GV->addDebugInfo(Debug);
+}
+
 llvm::DIScope *CGDebugInfo::getCurrentContextDescriptor(const Decl *D) {
   if (!LexicalBlockStack.empty())
 return LexicalBlockStack.back();

diff  --git a/clang/lib/CodeGen/CGDebugInfo.h b/clang/lib/CodeGen/CGDebugInfo.h
index 8984f3eb1d7a0..38e3fa5b2fa96 100644
--- a/clang/lib/CodeGen/CGDebugInfo.h
+++ b/clang/lib/CodeGen/CGDebugInfo.h
@@ -533,6 +533,14 @@ class CGDebugInfo {
   /// Emit an @import declaration.
   void EmitImportDecl(const ImportDecl );
 
+  /// DebugInfo isn't attached to string literals by default. While certain
+  /// aspects of debuginfo aren't useful for string literals (like a name), 
it's
+  /// nice to be able to symbolize the line and column information. This is
+  /// especially useful for sanitizers, as it allows symbolization of
+  /// heap-buffer-overflows on constant strings.
+  void AddStringLiteralDebugInfo(llvm::GlobalVariable *GV,
+ const StringLiteral *S);
+
   /// Emit C++ namespace alias.
   llvm::DIImportedEntity *EmitNamespaceAlias(const NamespaceAliasDecl );
 

diff  --git a/clang/lib/CodeGen/CodeGenModule.cpp 
b/clang/lib/CodeGen/CodeGenModule.cpp
index f8bf210dc0e21..703cf4edf5f56 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -5670,6 +5670,11 @@ CodeGenModule::GetAddrOfConstantStringFromLiteral(const 
StringLiteral *S,
   }
 
   auto GV = GenerateStringLiteral(C, LT, *this, GlobalVariableName, Alignment);
+
+  CGDebugInfo *DI = getModuleDebugInfo();
+  if (DI && getCodeGenOpts().hasReducedDebugInfo())
+DI->AddStringLiteralDebugInfo(GV, S);
+
   if (Entry)
 *Entry = GV;
 

diff  --git a/clang/test/CodeGen/debug-info-variables.c 
b/clang/test/CodeGen/debug-info-variables.c
new file mode 100644
index 0..1e3b4fb979d7a
--- /dev/null
+++ b/clang/test/CodeGen/debug-info-variables.c
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 %s -debug-info-kind=standalone -S -emit-llvm -o - | 
FileCheck %s
+
+// CHECK: DIGlobalVariable(name: "global",{{.*}} line: [[@LINE+1]]
+int global = 42;
+
+// CHECK: DIGlobalVariable({{.*}}line: [[@LINE+4]],{{.*}} type: 
[[TYPEID:![0-9]+]]
+// CHECK: [[TYPEID]] = 

[PATCH] D125679: [Clang] Added options for integrated backend only used for SPIR-V for now

[Ilia Diachkov via Phabricator via cfe-commits]

iliya-diyachkov added a comment.

I'm not an expert in clang, but overall the patch looks good to me.




Comment at: clang/docs/UsersManual.rst:3640
+Clang also supports integrated generation of SPIR-V without use of 
``llvm-spirv``
+tool as an exerimental feature when ``-fintegrated-objemitter`` flag is passed 
in
+the command line.

Please fix typo in "exerimental".



Comment at: clang/include/clang/Driver/ToolChain.h:392
+
+  /// IsIntegratedBackendSupported - Does this tool chain supports
+  /// -fintegrated-objemitter.

Probably change "supports" to "support".



Comment at: clang/include/clang/Driver/ToolChain.h:396
+
+  /// IsNonIntegratedBackendSupported - Does this tool chain supports
+  /// -fno-integrated-objemitter.

The same issue.


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

https://reviews.llvm.org/D125679

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

[PATCH] D123538: [symbolizer] Parse DW_TAG_variable DIs to show line info for globals

[David Blaikie via Phabricator via cfe-commits]

dblaikie added inline comments.



Comment at: llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp:769-783
+DataExtractor Data(Location.Expr, /*IsLittleEndian=*/true, 8);
+uint64_t DataOffset = 0;
+uint8_t Operation = Data.getU8();
+if (Operation == dwarf::DW_OP_addr) {
+  uint64_t Pointer = Data.getAddress();
+  VariableDieMap[Pointer] = Die;
+  return;

hctim wrote:
> dblaikie wrote:
> > Are there any examples of global merge where this might not be correct?
> > 
> > Like if a global was described as "addrx, 5, add" (eg: 5 beyond this 
> > address) then only looking at the first operation might mis-conclude that 
> > the variable is at this address when it's at 5 bytes past this address - 
> > and some other variable is at this address)
> > 
> > LLVM has a "global merge" optimization that might cause something like 
> > this. Let's see if I can create an example.
> > 
> > Ah, yeah, here we go:
> > ```
> > static int x;
> > static int y;
> > int *f(bool b) { return b ?  :  }
> > ```
> > ```
> > $ clang++-tot -O3 merge.cpp -c -g -target aarch64-linux-gnuabi -mllvm 
> > -global-merge-ignore-single-use && llvm-dwarfdump-tot merge.o | grep 
> > DW_AT_location
> > DW_AT_location  (DW_OP_addrx 0x0)
> > DW_AT_location  (DW_OP_addrx 0x0, DW_OP_plus_uconst 0x4)
> > ```
> > 
> > (the `-global-merge-ignore-single-use` is to simplify the test case - 
> > without that it could still be tickled with a more complicated example - 
> > seems we only enable global merge on ARM 32 and 64 bit due to the higher 
> > register pressure there, by the sounds of it)
> > 
> > I'm guessing this patch would overwrite the VariableDieMap entry for the 
> > first global with the second one so queries for the first address would 
> > result in the second DIE and queries for the second address wouldn't give 
> > any result?
> > 
> > Hmm, also - how does this handle queries that aren't at exactly the 
> > starting address of the variable? Presumably the `DW_AT_type` of the 
> > `DW_TAG_global_variable` would have to be inspected to find the size of the 
> > variable starting at the address, and any query in that range should be 
> > successful?
> I've added some handling for the `DW_OP_plus_uconst`. Unfortunately I didn't 
> find any generic existing handling for the expression evaluation (which 
> surprised me, maybe I just suck at looking), so I'm just making the 
> assumption that global variable addresses aren't described using `DW_OP_plus` 
> or anything else...
> 
> Fixed it up to now provide line info when you provide an address halfway 
> through a symbol. Good thing there wasn't a big impact in D123534 about 
> keeping the string sizes around :). Worst case (if the type info sucks) we 
> just only accept the precise start addresses of the symbols.
Probably worth maybe /only/ matching these expressions, then, rather than 
currently it's matching any expression that starts with addr, or addr+plus? 
Even if that's followed by any other garbage that would be ignored (& so the 
expression would be misinterpreted)

There's probably no expression evaluation logic in LLVM's libDebugInfoDWARF - 
there's no need to evaluate expressions when symbolizing, and dwarfdump just 
wants to dump it, not compute the resulting value. (some similar code exists in 
lldb that would do evaluation).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123538

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

[PATCH] D123534: [dwarf] Emit a DIGlobalVariable for constant strings.

[David Blaikie via Phabricator via cfe-commits]

dblaikie accepted this revision.
dblaikie added a comment.

Seems good - if folks find the growth too much, or it creates weird perf issues 
otherwise, we can discuss that if/when it comes up.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123534

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

[PATCH] D125727: [clang] Avoid suggesting typoed directives in `.S` files

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui added a comment.

Thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125727

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

[PATCH] D125727: [clang] Avoid suggesting typoed directives in `.S` files

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

pushed 45e01ce5fe6a5e4dc25ffdf626caa344fbcb93dd 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125727

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

[PATCH] D125727: [clang] Avoid suggesting typoed directives in `.S` files

[Nick Desaulniers 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 rG45e01ce5fe6a: [clang] Avoid suggesting typoed directives in 
`.S` files (authored by ken-matsui, committed by nickdesaulniers).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125727

Files:
  clang/lib/Lex/PPDirectives.cpp
  clang/test/Preprocessor/suggest-typoed-directive.S


Index: clang/test/Preprocessor/suggest-typoed-directive.S
===
--- /dev/null
+++ clang/test/Preprocessor/suggest-typoed-directive.S
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+// expected-no-diagnostics
+
+#ifdef UNDEFINED
+#id
+#ifd
+#ifde
+#elf
+#elsif
+#elseif
+#elfidef
+#elfindef
+#elfinndef
+#els
+#endi
+#endif
+
+#ifdef UNDEFINED
+# in in order to perform
+#endif
+
+#ifdef UNDEFINED
+#i
+#endif
+
+#if special_compiler
+#special_compiler_directive
+#endif
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -481,6 +481,10 @@
 void Preprocessor::SuggestTypoedDirective(const Token ,
   StringRef Directive,
   const SourceLocation ) const {
+  // If this is a `.S` file, treat unknown # directives as non-preprocessor
+  // directives.
+  if (getLangOpts().AsmPreprocessor) return;
+
   std::vector Candidates = {
   "if", "ifdef", "ifndef", "elif", "else", "endif"
   };


Index: clang/test/Preprocessor/suggest-typoed-directive.S
===
--- /dev/null
+++ clang/test/Preprocessor/suggest-typoed-directive.S
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+// expected-no-diagnostics
+
+#ifdef UNDEFINED
+#id
+#ifd
+#ifde
+#elf
+#elsif
+#elseif
+#elfidef
+#elfindef
+#elfinndef
+#els
+#endi
+#endif
+
+#ifdef UNDEFINED
+# in in order to perform
+#endif
+
+#ifdef UNDEFINED
+#i
+#endif
+
+#if special_compiler
+#special_compiler_directive
+#endif
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -481,6 +481,10 @@
 void Preprocessor::SuggestTypoedDirective(const Token ,
   StringRef Directive,
   const SourceLocation ) const {
+  // If this is a `.S` file, treat unknown # directives as non-preprocessor
+  // directives.
+  if (getLangOpts().AsmPreprocessor) return;
+
   std::vector Candidates = {
   "if", "ifdef", "ifndef", "elif", "else", "endif"
   };
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 45e01ce - [clang] Avoid suggesting typoed directives in `.S` files

[Nick Desaulniers via cfe-commits]

Author: Ken Matsui
Date: 2022-05-16T15:46:59-07:00
New Revision: 45e01ce5fe6a5e4dc25ffdf626caa344fbcb93dd

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

LOG: [clang] Avoid suggesting typoed directives in `.S` files

This patch is itended to avoid suggesting typoed directives in `.S`
files to support the cases of `#` directives treated as comments or
various pseudo-ops. The feature is implemented in
https://reviews.llvm.org/D124726.

Fixes: https://reviews.llvm.org/D124726#3516346.

Reviewed By: nickdesaulniers

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

Added: 
clang/test/Preprocessor/suggest-typoed-directive.S

Modified: 
clang/lib/Lex/PPDirectives.cpp

Removed: 




diff  --git a/clang/lib/Lex/PPDirectives.cpp b/clang/lib/Lex/PPDirectives.cpp
index 55045320fd591..d947c1580f5ca 100644
--- a/clang/lib/Lex/PPDirectives.cpp
+++ b/clang/lib/Lex/PPDirectives.cpp
@@ -481,6 +481,10 @@ Optional 
Preprocessor::getSkippedRangeForExcludedConditionalBlock(
 void Preprocessor::SuggestTypoedDirective(const Token ,
   StringRef Directive,
   const SourceLocation ) const {
+  // If this is a `.S` file, treat unknown # directives as non-preprocessor
+  // directives.
+  if (getLangOpts().AsmPreprocessor) return;
+
   std::vector Candidates = {
   "if", "ifdef", "ifndef", "elif", "else", "endif"
   };

diff  --git a/clang/test/Preprocessor/suggest-typoed-directive.S 
b/clang/test/Preprocessor/suggest-typoed-directive.S
new file mode 100644
index 0..d00c21f621fa8
--- /dev/null
+++ b/clang/test/Preprocessor/suggest-typoed-directive.S
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+// expected-no-diagnostics
+
+#ifdef UNDEFINED
+#id
+#ifd
+#ifde
+#elf
+#elsif
+#elseif
+#elfidef
+#elfindef
+#elfinndef
+#els
+#endi
+#endif
+
+#ifdef UNDEFINED
+# in in order to perform
+#endif
+
+#ifdef UNDEFINED
+#i
+#endif
+
+#if special_compiler
+#special_compiler_directive
+#endif



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

[PATCH] D125727: [clang] Avoid suggesting typoed directives in `.S` files

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

sorted, will commit soon


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125727

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

[PATCH] D125727: [clang] Avoid suggesting typoed directives in `.S` files

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

Sorry, it looks like `arcanist` or my PHP runtime was auto updated on my host 
and has regressed. I need to sort that out first.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125727

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

[PATCH] D125727: [clang] Avoid suggesting typoed directives in `.S` files

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

> I do not have permission to land this patch, so could you please do it on my 
> behalf?

Will do.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125727

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

[PATCH] D125727: [clang] Avoid suggesting typoed directives in `.S` files

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui added a comment.

@nickdesaulniers
Thank you for your review!

I do not have permission to land this patch, so could you please do it on my 
behalf?
Here is my information:
Name: `Ken Matsui`
Email: `v...@kmatsui.me`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125727

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

[PATCH] D125727: [clang] Avoid suggesting typoed directives in `.S` files

[Nick Desaulniers via Phabricator via cfe-commits]

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

thanks for the patch!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125727

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

[PATCH] D125727: [clang] Avoid suggesting typoed directives in `.S` files

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui added a comment.

Sorry for having missed it. Thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125727

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

[PATCH] D125727: [clang] Avoid suggesting typoed directives in `.S` files

[Ken Matsui via Phabricator via cfe-commits]

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

Remove `- < %s`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125727

Files:
  clang/lib/Lex/PPDirectives.cpp
  clang/test/Preprocessor/suggest-typoed-directive.S


Index: clang/test/Preprocessor/suggest-typoed-directive.S
===
--- /dev/null
+++ clang/test/Preprocessor/suggest-typoed-directive.S
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+// expected-no-diagnostics
+
+#ifdef UNDEFINED
+#id
+#ifd
+#ifde
+#elf
+#elsif
+#elseif
+#elfidef
+#elfindef
+#elfinndef
+#els
+#endi
+#endif
+
+#ifdef UNDEFINED
+# in in order to perform
+#endif
+
+#ifdef UNDEFINED
+#i
+#endif
+
+#if special_compiler
+#special_compiler_directive
+#endif
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -481,6 +481,10 @@
 void Preprocessor::SuggestTypoedDirective(const Token ,
   StringRef Directive,
   const SourceLocation ) const {
+  // If this is a `.S` file, treat unknown # directives as non-preprocessor
+  // directives.
+  if (getLangOpts().AsmPreprocessor) return;
+
   std::vector Candidates = {
   "if", "ifdef", "ifndef", "elif", "else", "endif"
   };


Index: clang/test/Preprocessor/suggest-typoed-directive.S
===
--- /dev/null
+++ clang/test/Preprocessor/suggest-typoed-directive.S
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+// expected-no-diagnostics
+
+#ifdef UNDEFINED
+#id
+#ifd
+#ifde
+#elf
+#elsif
+#elseif
+#elfidef
+#elfindef
+#elfinndef
+#els
+#endi
+#endif
+
+#ifdef UNDEFINED
+# in in order to perform
+#endif
+
+#ifdef UNDEFINED
+#i
+#endif
+
+#if special_compiler
+#special_compiler_directive
+#endif
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -481,6 +481,10 @@
 void Preprocessor::SuggestTypoedDirective(const Token ,
   StringRef Directive,
   const SourceLocation ) const {
+  // If this is a `.S` file, treat unknown # directives as non-preprocessor
+  // directives.
+  if (getLangOpts().AsmPreprocessor) return;
+
   std::vector Candidates = {
   "if", "ifdef", "ifndef", "elif", "else", "endif"
   };
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D125727: [clang] Avoid suggesting typoed directives in `.S` files

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added inline comments.



Comment at: clang/test/Preprocessor/suggest-typoed-directive.S:1
+// RUN: %clang_cc1 -fsyntax-only -verify %s - < %s
+

These three can be removed now as well.  Clang will read `%s` as input without 
the need to read from stdin, which is what `- < [filename]` is doing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125727

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

[PATCH] D125705: [OpenMP] Don't build the offloading driver without a source input

[Joseph Huber 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 rGb653b409ff44: [OpenMP] Dont build the offloading 
driver without a source input (authored by jhuber6).

Changed prior to commit:
  https://reviews.llvm.org/D125705?vs=429761=429876#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125705

Files:
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/openmp-offload-gpu-new.c


Index: clang/test/Driver/openmp-offload-gpu-new.c
===
--- clang/test/Driver/openmp-offload-gpu-new.c
+++ clang/test/Driver/openmp-offload-gpu-new.c
@@ -70,6 +70,11 @@
 // CHECK-NVIDIA-AMDGPU: "x86_64-unknown-linux-gnu" - "clang", inputs: 
["[[HOST_BC]]", "[[BINARY]]"], output: "[[HOST_OBJ:.+]]"
 // CHECK-NVIDIA-AMDGPU: "x86_64-unknown-linux-gnu" - "Offload::Linker", 
inputs: ["[[HOST_OBJ]]"], output: "a.out"
 
+// RUN:   %clang -x ir -### --target=x86_64-unknown-linux-gnu 
-ccc-print-bindings -fopenmp --offload-arch=sm_52 -nogpulib %s 2>&1 | FileCheck 
%s --check-prefix=CHECK-IR
+
+// CHECK-IR: "x86_64-unknown-linux-gnu" - "clang", inputs: 
["[[INPUT_IR:.+]]"], output: "[[OBJECT:.+]]"
+// CHECK-IR: "x86_64-unknown-linux-gnu" - "Offload::Linker", inputs: 
["[[OBJECT]]"], output: "a.out"
+
 // RUN:   %clang -### --target=x86_64-unknown-linux-gnu -emit-llvm -S -fopenmp 
-fopenmp-targets=nvptx64-nvidia-cuda -Xopenmp-target=nvptx64-nvidia-cuda 
-march=sm_52 -nogpulib %s 2>&1 | FileCheck %s --check-prefix=CHECK-EMIT-LLVM-IR
 // CHECK-EMIT-LLVM-IR: "-cc1"{{.*}}"-triple" 
"nvptx64-nvidia-cuda"{{.*}}"-emit-llvm"
 
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -4377,9 +4377,11 @@
   Mode && Mode->getOption().matches(options::OPT_offload_device_only);
 
   // Don't build offloading actions if explicitly disabled or we do not have a
-  // compile action to embed it in. If preprocessing only ignore embedding.
-  if (HostOnly || !(isa(HostAction) ||
-getFinalPhase(Args) == phases::Preprocess))
+  // valid source input and compile action to embed it in. If preprocessing 
only
+  // ignore embedding.
+  if (HostOnly || !types::isSrcFile(Input.first) ||
+  !(isa(HostAction) ||
+getFinalPhase(Args) == phases::Preprocess))
 return HostAction;
 
   ActionList OffloadActions;


Index: clang/test/Driver/openmp-offload-gpu-new.c
===
--- clang/test/Driver/openmp-offload-gpu-new.c
+++ clang/test/Driver/openmp-offload-gpu-new.c
@@ -70,6 +70,11 @@
 // CHECK-NVIDIA-AMDGPU: "x86_64-unknown-linux-gnu" - "clang", inputs: ["[[HOST_BC]]", "[[BINARY]]"], output: "[[HOST_OBJ:.+]]"
 // CHECK-NVIDIA-AMDGPU: "x86_64-unknown-linux-gnu" - "Offload::Linker", inputs: ["[[HOST_OBJ]]"], output: "a.out"
 
+// RUN:   %clang -x ir -### --target=x86_64-unknown-linux-gnu -ccc-print-bindings -fopenmp --offload-arch=sm_52 -nogpulib %s 2>&1 | FileCheck %s --check-prefix=CHECK-IR
+
+// CHECK-IR: "x86_64-unknown-linux-gnu" - "clang", inputs: ["[[INPUT_IR:.+]]"], output: "[[OBJECT:.+]]"
+// CHECK-IR: "x86_64-unknown-linux-gnu" - "Offload::Linker", inputs: ["[[OBJECT]]"], output: "a.out"
+
 // RUN:   %clang -### --target=x86_64-unknown-linux-gnu -emit-llvm -S -fopenmp -fopenmp-targets=nvptx64-nvidia-cuda -Xopenmp-target=nvptx64-nvidia-cuda -march=sm_52 -nogpulib %s 2>&1 | FileCheck %s --check-prefix=CHECK-EMIT-LLVM-IR
 // CHECK-EMIT-LLVM-IR: "-cc1"{{.*}}"-triple" "nvptx64-nvidia-cuda"{{.*}}"-emit-llvm"
 
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -4377,9 +4377,11 @@
   Mode && Mode->getOption().matches(options::OPT_offload_device_only);
 
   // Don't build offloading actions if explicitly disabled or we do not have a
-  // compile action to embed it in. If preprocessing only ignore embedding.
-  if (HostOnly || !(isa(HostAction) ||
-getFinalPhase(Args) == phases::Preprocess))
+  // valid source input and compile action to embed it in. If preprocessing only
+  // ignore embedding.
+  if (HostOnly || !types::isSrcFile(Input.first) ||
+  !(isa(HostAction) ||
+getFinalPhase(Args) == phases::Preprocess))
 return HostAction;
 
   ActionList OffloadActions;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] b653b40 - [OpenMP] Don't build the offloading driver without a source input

[Joseph Huber via cfe-commits]

Author: Joseph Huber
Date: 2022-05-16T18:19:02-04:00
New Revision: b653b409ff44b09ade04bb6e579f5f9790424611

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

LOG: [OpenMP] Don't build the offloading driver without a source input

The Clang driver additional stages to build a complete offloading
program for applications using CUDA or OpenMP offloading. This normally
requires either a source file input or a valid object file to be
handled. This would cause problems when trying to compile an assembly or
LLVM IR file through clang with flags that would enable offloading. This
patch simply adds a check to prevent the offloading toolchain from being
used if we don't have a valid source file.

Reviewed By: jdoerfert

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

Added: 


Modified: 
clang/lib/Driver/Driver.cpp
clang/test/Driver/openmp-offload-gpu-new.c

Removed: 




diff  --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index 5884b096faab..3edbaed9ae7f 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -4377,9 +4377,11 @@ Action *Driver::BuildOffloadingActions(Compilation ,
   Mode && Mode->getOption().matches(options::OPT_offload_device_only);
 
   // Don't build offloading actions if explicitly disabled or we do not have a
-  // compile action to embed it in. If preprocessing only ignore embedding.
-  if (HostOnly || !(isa(HostAction) ||
-getFinalPhase(Args) == phases::Preprocess))
+  // valid source input and compile action to embed it in. If preprocessing 
only
+  // ignore embedding.
+  if (HostOnly || !types::isSrcFile(Input.first) ||
+  !(isa(HostAction) ||
+getFinalPhase(Args) == phases::Preprocess))
 return HostAction;
 
   ActionList OffloadActions;

diff  --git a/clang/test/Driver/openmp-offload-gpu-new.c 
b/clang/test/Driver/openmp-offload-gpu-new.c
index cd8dfc452615..5ea73ccadc07 100644
--- a/clang/test/Driver/openmp-offload-gpu-new.c
+++ b/clang/test/Driver/openmp-offload-gpu-new.c
@@ -70,6 +70,11 @@
 // CHECK-NVIDIA-AMDGPU: "x86_64-unknown-linux-gnu" - "clang", inputs: 
["[[HOST_BC]]", "[[BINARY]]"], output: "[[HOST_OBJ:.+]]"
 // CHECK-NVIDIA-AMDGPU: "x86_64-unknown-linux-gnu" - "Offload::Linker", 
inputs: ["[[HOST_OBJ]]"], output: "a.out"
 
+// RUN:   %clang -x ir -### --target=x86_64-unknown-linux-gnu 
-ccc-print-bindings -fopenmp --offload-arch=sm_52 -nogpulib %s 2>&1 | FileCheck 
%s --check-prefix=CHECK-IR
+
+// CHECK-IR: "x86_64-unknown-linux-gnu" - "clang", inputs: 
["[[INPUT_IR:.+]]"], output: "[[OBJECT:.+]]"
+// CHECK-IR: "x86_64-unknown-linux-gnu" - "Offload::Linker", inputs: 
["[[OBJECT]]"], output: "a.out"
+
 // RUN:   %clang -### --target=x86_64-unknown-linux-gnu -emit-llvm -S -fopenmp 
-fopenmp-targets=nvptx64-nvidia-cuda -Xopenmp-target=nvptx64-nvidia-cuda 
-march=sm_52 -nogpulib %s 2>&1 | FileCheck %s --check-prefix=CHECK-EMIT-LLVM-IR
 // CHECK-EMIT-LLVM-IR: "-cc1"{{.*}}"-triple" 
"nvptx64-nvidia-cuda"{{.*}}"-emit-llvm"
 



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

[PATCH] D125719: [Attribute] Add attribute NeverOptimizeNone

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added inline comments.



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:1934
   !F->hasFnAttribute(llvm::Attribute::AlwaysInline)) {
 B.addAttribute(llvm::Attribute::OptimizeNone);
 

I still think adding optnone to everything at -O0 is a bad idea in the first 
place, and the optimization level should not be encoded in the IR


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125719

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

[PATCH] D125727: [clang] Avoid suggesting typoed directives in `.S` files

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui added a comment.

Removed it 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125727

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

[PATCH] D125727: [clang] Avoid suggesting typoed directives in `.S` files

[Ken Matsui via Phabricator via cfe-commits]

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

Remove `-x assembler-with-cpp`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125727

Files:
  clang/lib/Lex/PPDirectives.cpp
  clang/test/Preprocessor/suggest-typoed-directive.S


Index: clang/test/Preprocessor/suggest-typoed-directive.S
===
--- /dev/null
+++ clang/test/Preprocessor/suggest-typoed-directive.S
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s - < %s
+
+// expected-no-diagnostics
+
+#ifdef UNDEFINED
+#id
+#ifd
+#ifde
+#elf
+#elsif
+#elseif
+#elfidef
+#elfindef
+#elfinndef
+#els
+#endi
+#endif
+
+#ifdef UNDEFINED
+# in in order to perform
+#endif
+
+#ifdef UNDEFINED
+#i
+#endif
+
+#if special_compiler
+#special_compiler_directive
+#endif
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -481,6 +481,10 @@
 void Preprocessor::SuggestTypoedDirective(const Token ,
   StringRef Directive,
   const SourceLocation ) const {
+  // If this is a `.S` file, treat unknown # directives as non-preprocessor
+  // directives.
+  if (getLangOpts().AsmPreprocessor) return;
+
   std::vector Candidates = {
   "if", "ifdef", "ifndef", "elif", "else", "endif"
   };


Index: clang/test/Preprocessor/suggest-typoed-directive.S
===
--- /dev/null
+++ clang/test/Preprocessor/suggest-typoed-directive.S
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s - < %s
+
+// expected-no-diagnostics
+
+#ifdef UNDEFINED
+#id
+#ifd
+#ifde
+#elf
+#elsif
+#elseif
+#elfidef
+#elfindef
+#elfinndef
+#els
+#endi
+#endif
+
+#ifdef UNDEFINED
+# in in order to perform
+#endif
+
+#ifdef UNDEFINED
+#i
+#endif
+
+#if special_compiler
+#special_compiler_directive
+#endif
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -481,6 +481,10 @@
 void Preprocessor::SuggestTypoedDirective(const Token ,
   StringRef Directive,
   const SourceLocation ) const {
+  // If this is a `.S` file, treat unknown # directives as non-preprocessor
+  // directives.
+  if (getLangOpts().AsmPreprocessor) return;
+
   std::vector Candidates = {
   "if", "ifdef", "ifndef", "elif", "else", "endif"
   };
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D125727: [clang] Avoid suggesting typoed directives in `.S` files

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added inline comments.



Comment at: clang/test/Preprocessor/suggest-typoed-directive.S:1
+// RUN: %clang_cc1 -fsyntax-only -verify -S %s -o %t
+

nickdesaulniers wrote:
> nickdesaulniers wrote:
> > Consider adding a test:
> > 
> > ```
> > // RUN: %clang_cc1 -fsyntax-only -verify %s -x assembler-with-cpp < %s
> > ```
> > 
> > I don't think you need `-S` here or `-o %t` since there should be nothing 
> > produced from -fsyntax-only.
> Sorry, I missed a `-` for "read from stdin":
> ```
> // RUN: %clang_cc1 -fsyntax-only -verify %s -x assembler-with-cpp - < %s
> ```
Sorry, I meant for you to have two run lines. One that specified `-x 
assembler-with-cpp` and one without. But I think `-x assembler-with-cpp` is not 
necessary, you should/can/may drop it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125727

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

[PATCH] D125727: [clang] Avoid suggesting typoed directives in `.S` files

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui added inline comments.



Comment at: clang/lib/Lex/PPDirectives.cpp:484
   const SourceLocation ) const {
+  // If this is a `.S` file, treat unknown # directives as non-preprocessor
+  // directives.

nickdesaulniers wrote:
> Also, IIRC the token used to start a comment in assembler differs per 
> architecture. This might be the simplest fix, for now.
Ah, I did not know that. Thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125727

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

[PATCH] D125727: [clang] Avoid suggesting typoed directives in `.S` files

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui added a comment.

Thank you for your review! I've fixed it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125727

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

[PATCH] D125727: [clang] Avoid suggesting typoed directives in `.S` files

[Ken Matsui via Phabricator via cfe-commits]

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

Update the code as reviewed


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125727

Files:
  clang/lib/Lex/PPDirectives.cpp
  clang/test/Preprocessor/suggest-typoed-directive.S


Index: clang/test/Preprocessor/suggest-typoed-directive.S
===
--- /dev/null
+++ clang/test/Preprocessor/suggest-typoed-directive.S
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s -x assembler-with-cpp - < %s
+
+// expected-no-diagnostics
+
+#ifdef UNDEFINED
+#id
+#ifd
+#ifde
+#elf
+#elsif
+#elseif
+#elfidef
+#elfindef
+#elfinndef
+#els
+#endi
+#endif
+
+#ifdef UNDEFINED
+# in in order to perform
+#endif
+
+#ifdef UNDEFINED
+#i
+#endif
+
+#if special_compiler
+#special_compiler_directive
+#endif
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -481,6 +481,10 @@
 void Preprocessor::SuggestTypoedDirective(const Token ,
   StringRef Directive,
   const SourceLocation ) const {
+  // If this is a `.S` file, treat unknown # directives as non-preprocessor
+  // directives.
+  if (getLangOpts().AsmPreprocessor) return;
+
   std::vector Candidates = {
   "if", "ifdef", "ifndef", "elif", "else", "endif"
   };


Index: clang/test/Preprocessor/suggest-typoed-directive.S
===
--- /dev/null
+++ clang/test/Preprocessor/suggest-typoed-directive.S
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s -x assembler-with-cpp - < %s
+
+// expected-no-diagnostics
+
+#ifdef UNDEFINED
+#id
+#ifd
+#ifde
+#elf
+#elsif
+#elseif
+#elfidef
+#elfindef
+#elfinndef
+#els
+#endi
+#endif
+
+#ifdef UNDEFINED
+# in in order to perform
+#endif
+
+#ifdef UNDEFINED
+#i
+#endif
+
+#if special_compiler
+#special_compiler_directive
+#endif
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -481,6 +481,10 @@
 void Preprocessor::SuggestTypoedDirective(const Token ,
   StringRef Directive,
   const SourceLocation ) const {
+  // If this is a `.S` file, treat unknown # directives as non-preprocessor
+  // directives.
+  if (getLangOpts().AsmPreprocessor) return;
+
   std::vector Candidates = {
   "if", "ifdef", "ifndef", "elif", "else", "endif"
   };
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D125728: [WebAssembly] Update supported features in -mcpu=generic

[Dan Gohman via Phabricator via cfe-commits]

sunfish created this revision.
Herald added subscribers: pmatos, asb, ecnelises, jgravelle-google, sbc100, 
dschuff.
Herald added a project: All.
sunfish requested review of this revision.
Herald added a subscriber: aheejin.
Herald added a project: clang.

Enable nontrapping-fptoint, sign-ext, bulk-memory, and mutable-globals in 
-mcpu=generic. This makes these features enabled by default.

These features are all [finished proposals], and all major wasm engines support 
them.

[finished proposals]: 
https://github.com/WebAssembly/proposals/blob/main/finished-proposals.md


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125728

Files:
  clang/lib/Basic/Targets/WebAssembly.cpp


Index: clang/lib/Basic/Targets/WebAssembly.cpp
===
--- clang/lib/Basic/Targets/WebAssembly.cpp
+++ clang/lib/Basic/Targets/WebAssembly.cpp
@@ -144,6 +144,11 @@
 Features["mutable-globals"] = true;
 Features["tail-call"] = true;
 setSIMDLevel(Features, SIMD128, true);
+  } else if (CPU == "generic") {
+Features["nontrapping-fptoint"] = true;
+Features["sign-ext"] = true;
+Features["bulk-memory"] = true;
+Features["mutable-globals"] = true;
   }
 
   return TargetInfo::initFeatureMap(Features, Diags, CPU, FeaturesVec);


Index: clang/lib/Basic/Targets/WebAssembly.cpp
===
--- clang/lib/Basic/Targets/WebAssembly.cpp
+++ clang/lib/Basic/Targets/WebAssembly.cpp
@@ -144,6 +144,11 @@
 Features["mutable-globals"] = true;
 Features["tail-call"] = true;
 setSIMDLevel(Features, SIMD128, true);
+  } else if (CPU == "generic") {
+Features["nontrapping-fptoint"] = true;
+Features["sign-ext"] = true;
+Features["bulk-memory"] = true;
+Features["mutable-globals"] = true;
   }
 
   return TargetInfo::initFeatureMap(Features, Diags, CPU, FeaturesVec);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D125727: Avoid suggesting typoed directives in `.S` files

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added inline comments.



Comment at: clang/test/Preprocessor/suggest-typoed-directive.S:1
+// RUN: %clang_cc1 -fsyntax-only -verify -S %s -o %t
+

nickdesaulniers wrote:
> Consider adding a test:
> 
> ```
> // RUN: %clang_cc1 -fsyntax-only -verify %s -x assembler-with-cpp < %s
> ```
> 
> I don't think you need `-S` here or `-o %t` since there should be nothing 
> produced from -fsyntax-only.
Sorry, I missed a `-` for "read from stdin":
```
// RUN: %clang_cc1 -fsyntax-only -verify %s -x assembler-with-cpp - < %s
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125727

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

[PATCH] D125727: Avoid suggesting typoed directives in `.S` files

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added inline comments.



Comment at: clang/lib/Lex/PPDirectives.cpp:484
   const SourceLocation ) const {
+  // If this is a `.S` file, treat unknown # directives as non-preprocessor
+  // directives.

Also, IIRC the token used to start a comment in assembler differs per 
architecture. This might be the simplest fix, for now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125727

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

[PATCH] D125727: Avoid suggesting typoed directives in `.S` files

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added inline comments.



Comment at: clang/test/Preprocessor/suggest-typoed-directive.S:1
+// RUN: %clang_cc1 -fsyntax-only -verify -S %s -o %t
+

Consider adding a test:

```
// RUN: %clang_cc1 -fsyntax-only -verify %s -x assembler-with-cpp < %s
```

I don't think you need `-S` here or `-o %t` since there should be nothing 
produced from -fsyntax-only.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125727

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

[PATCH] D124726: Suggest typoed directives in preprocessor conditionals

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui added a comment.

I see. Thank you. I fixed the issue using its workaround.

https://reviews.llvm.org/D125727


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124726

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

[PATCH] D125727: Avoid suggesting typoed directives in `.S` files

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui created this revision.
ken-matsui added a reviewer: aaron.ballman.
Herald added a project: All.
ken-matsui requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This patch is itended to avoid suggesting typoed directives in `.S` files to 
support the cases of `#` directives treated as comments or various pseudo-ops. 
The feature is implemented in https://reviews.llvm.org/D124726. Fixes: 
https://reviews.llvm.org/D124726#3516346.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125727

Files:
  clang/lib/Lex/PPDirectives.cpp
  clang/test/Preprocessor/suggest-typoed-directive.S


Index: clang/test/Preprocessor/suggest-typoed-directive.S
===
--- /dev/null
+++ clang/test/Preprocessor/suggest-typoed-directive.S
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -S %s -o %t
+
+// expected-no-diagnostics
+
+#ifdef UNDEFINED
+#id
+#ifd
+#ifde
+#elf
+#elsif
+#elseif
+#elfidef
+#elfindef
+#elfinndef
+#els
+#endi
+#endif
+
+#ifdef UNDEFINED
+# in in order to perform
+#endif
+
+#ifdef UNDEFINED
+#i
+#endif
+
+#if special_compiler
+#special_compiler_directive
+#endif
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -481,6 +481,10 @@
 void Preprocessor::SuggestTypoedDirective(const Token ,
   StringRef Directive,
   const SourceLocation ) const {
+  // If this is a `.S` file, treat unknown # directives as non-preprocessor
+  // directives.
+  if (getLangOpts().AsmPreprocessor) return;
+
   std::vector Candidates = {
   "if", "ifdef", "ifndef", "elif", "else", "endif"
   };


Index: clang/test/Preprocessor/suggest-typoed-directive.S
===
--- /dev/null
+++ clang/test/Preprocessor/suggest-typoed-directive.S
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -S %s -o %t
+
+// expected-no-diagnostics
+
+#ifdef UNDEFINED
+#id
+#ifd
+#ifde
+#elf
+#elsif
+#elseif
+#elfidef
+#elfindef
+#elfinndef
+#els
+#endi
+#endif
+
+#ifdef UNDEFINED
+# in in order to perform
+#endif
+
+#ifdef UNDEFINED
+#i
+#endif
+
+#if special_compiler
+#special_compiler_directive
+#endif
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -481,6 +481,10 @@
 void Preprocessor::SuggestTypoedDirective(const Token ,
   StringRef Directive,
   const SourceLocation ) const {
+  // If this is a `.S` file, treat unknown # directives as non-preprocessor
+  // directives.
+  if (getLangOpts().AsmPreprocessor) return;
+
   std::vector Candidates = {
   "if", "ifdef", "ifndef", "elif", "else", "endif"
   };
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D125678: [clang][extract-api] Don't emit symbols prefixed with an underscore

[Zixu Wang via Phabricator via cfe-commits]

zixuw accepted this revision.
zixuw added a comment.

In D125678#3517168 , @QuietMisdreavus 
wrote:

> clang-format failed:
>
>   ---  clang-format
>   
>   changed files:
>   
>   clang/test/ExtractAPI/underscored.c

I think it's fine. clang-format always get tripped on split-file tests because 
of the mixed format and what not.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125678

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

[PATCH] D125678: [clang][extract-api] Don't emit symbols prefixed with an underscore

[Victoria Mitchell via Phabricator via cfe-commits]

QuietMisdreavus added a comment.

clang-format failed:

  ---  clang-format
  
  changed files:
  
  clang/test/ExtractAPI/underscored.c


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125678

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

[PATCH] D125678: [clang][extract-api] Don't emit symbols prefixed with an underscore

[Victoria Mitchell via Phabricator via cfe-commits]

QuietMisdreavus accepted this revision.
QuietMisdreavus 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/D125678/new/

https://reviews.llvm.org/D125678

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

[PATCH] D125723: [MSVC] Add support for MSVC pragma optimize

[Stephen Long via Phabricator via cfe-commits]

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

MSVC's pragma optimize turns optimizations on or off based on the list
passed. Depends on D125722 .

>From MSVC's docs:
+---+-+

| Parameter | Type of optimization |
|

+---+-+

| g | Enable global optimizations. Deprecated |
|

+---+-+

| s or t | Specify short or fast sequences of machine code |
|

+---+-+

| y | Generate frame pointers on the program stack |
|

+---+-+

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125723

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/ParsePragma.cpp
  clang/lib/Sema/SemaAttr.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/test/CodeGen/pragma-msvc-optimize.c
  clang/test/Preprocessor/pragma_microsoft.c

Index: clang/test/Preprocessor/pragma_microsoft.c
===
--- clang/test/Preprocessor/pragma_microsoft.c
+++ clang/test/Preprocessor/pragma_microsoft.c
@@ -228,7 +228,12 @@
 #pragma optimize("g"  // expected-warning{{expected ',' in '#pragma optimize'}}
 #pragma optimize("g", // expected-warning{{missing argument to '#pragma optimize'; expected 'on' or 'off'}}
 #pragma optimize("g",xyz  // expected-warning{{unexpected argument 'xyz' to '#pragma optimize'; expected 'on' or 'off'}}
-#pragma optimize("g",on)  // expected-warning{{#pragma optimize' is not supported}}
+#pragma optimize("g",on)  // no-warning
+#pragma optimize("g", on) asdf // expected-warning{{extra tokens at end of '#pragma optimize'}}
+
+void pragma_optimize_foo() {
+#pragma optimize("g", on) // expected-error {{'#pragma optimize' can only appear at file scope}}
+}
 
 #pragma execution_character_set // expected-warning {{expected '('}}
 #pragma execution_character_set(// expected-warning {{expected 'push' or 'pop'}}
Index: clang/test/CodeGen/pragma-msvc-optimize.c
===
--- /dev/null
+++ clang/test/CodeGen/pragma-msvc-optimize.c
@@ -0,0 +1,48 @@
+// RUN: %clang_cc1 -O2 -emit-llvm -fms-extensions -o - %s | FileCheck %s -check-prefix CHECK-O2
+// RUN: %clang_cc1 -O0 -emit-llvm -fms-extensions -o - %s | FileCheck %s -check-prefix CHECK-O0
+// RUN: %clang_cc1 -Os -emit-llvm -fms-extensions -o - %s | FileCheck %s -check-prefix CHECK-Os
+
+#pragma optimize("s", on)
+
+// CHECK-O2: define{{.*}} void @f0(){{.*}} #[[OPTSIZE:[0-9]+]]
+void f0() {}
+
+#pragma optimize("y", on)
+
+// CHECK-O2: define{{.*}} void @f1(){{.*}} #[[OPTSIZE_FRAMEPTR_NONLEAF:[0-9]+]]
+void f1() {}
+
+#pragma optimize("t", on)
+
+// CHECK-O2: define{{.*}} void @f2(){{.*}} #[[OPTSIZE_FRAMEPTR_NONLEAF]]
+// CHECK-O0: define{{.*}} void @f2(){{.*}} #[[NO_OPTNONE:[0-9]+]]
+void f2() {}
+
+#pragma optimize("s", off)
+
+// CHECK-O2: define{{.*}} void @f3(){{.*}} #[[FRAMEPTR_NONLEAF:[0-9]+]]
+// CHECK-Os: define{{.*}} void @f3(){{.*}} #[[FRAMEPTR_NONLEAF:[0-9]+]]
+void f3() {}
+
+#pragma optimize("y", off)
+
+// CHECK-O2: define{{.*}} void @f4(){{.*}} #[[FRAMEPTR_NONE:[0-9]+]]
+void f4() {}
+
+#pragma optimize("t", off)
+
+// CHECK-O2: define{{.*}} void @f5(){{.*}} #[[OPTNONE_FRAMEPTR_NONE:[0-9]+]]
+void f5() {}
+
+// CHECK-O2: attributes #[[OPTSIZE]] = {{{.*}}optsize{{.*}}}
+// CHECK-O2-NOT: attributes #[[OPTSIZE]] = {{{.*}}optsize{{.*}}"frame-pointer"="non-leaf"{{.*}}}
+// CHECK-O2: attributes #[[OPTSIZE_FRAMEPTR_NONLEAF]] = {{{.*}}optsize{{.*}}"frame-pointer"="non-leaf"{{.*}}}
+// CHECK-O2: attributes #[[FRAMEPTR_NONLEAF]] = {{{.*}}"frame-pointer"="non-leaf"{{.*}}}
+// CHECK-O2-NOT: attributes #[[FRAMEPTR_NONLEAF]] = {{{.*}}optsize{{.*}}"frame-pointer"="non-leaf"{{.*}}}
+// CHECK-O2: attributes #[[FRAMEPTR_NONE]] = {{{.*}}"frame-pointer"="none"{{.*}}}
+// CHECK-O2: attributes #[[OPTNONE_FRAMEPTR_NONE]] = {{{.*}}optnone{{.*}}"frame-pointer"="none"{{.*}}}
+
+// CHECK-O0-NOT: attributes #[[NO_OPTNONE]] = {{{.*}}optnone{{.*}}}
+
+// CHECK-Os: attributes #[[FRAMEPTR_NONLEAF]] = {{{.*}}"frame-pointer"="non-leaf"{{.*}}}
+// CHECK-Os-NOT: attributes #[[FRAMEPTR_NONLEAF]] = {{{.*}}optsize{{.*}}"frame-pointer"="non-leaf"{{.*}}}
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -10220,6 +10220,7 @@
 AddRangeBasedOptnone(NewFD);
 AddImplicitMSFunctionNoBuiltinAttr(NewFD);
 AddSectionMSAllocText(NewFD);

[PATCH] D125722: [Attribute] Add clang optsize attribute

[Stephen Long via Phabricator via cfe-commits]

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

Add the ability to put `__attribute__((optsize))` on functions. Depends on 
D125720 .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125722

Files:
  clang/include/clang/Basic/Attr.td
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Sema/SemaDeclAttr.cpp


Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -6763,6 +6763,19 @@
 checkSwiftAsyncErrorBlock(S, D, ErrorAttr, AsyncAttr);
 }
 
+static void handleOptimizeSizeAttr(Sema , Decl *D, const ParsedAttr ) {
+  if (!AL.isArgIdent(0)) {
+S.Diag(AL.getLoc(), diag::err_attribute_argument_n_type)
+<< AL << 1 << AANT_ArgumentIdentifier;
+return;
+  }
+
+  OptimizeSizeAttr::Kind Kind;
+  IdentifierInfo *II = AL.getArgAsIdent(0)->Ident;
+  OptimizeSizeAttr::ConvertStrToKind(II->getName(), Kind);
+  D->addAttr(::new (S.Context) OptimizeSizeAttr(S.Context, AL, Kind));
+}
+
 static void handleFramePointer(Sema , Decl *D, const ParsedAttr ) {
   if (!AL.isArgIdent(0)) {
 S.Diag(AL.getLoc(), diag::err_attribute_argument_n_type)
@@ -8534,6 +8547,9 @@
   case ParsedAttr::AT_MinSize:
 handleMinSizeAttr(S, D, AL);
 break;
+  case ParsedAttr::AT_OptimizeSize:
+handleOptimizeSizeAttr(S, D, AL);
+break;
   case ParsedAttr::AT_OptimizeNone:
 handleOptimizeNoneAttr(S, D, AL);
 break;
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -1653,6 +1653,19 @@
   if (!GD.getDecl())
 return;
 
+  bool HasOptnone = GD.getDecl()->hasAttr();
+  bool HasOptsizeOn = false;
+  bool HasOptsizeOff = false;
+  if (GD.getDecl()->hasAttr()) {
+OptimizeSizeAttr *Attr = GD.getDecl()->getAttr();
+HasOptsizeOn = Attr->getKind() == OptimizeSizeAttr::On;
+HasOptsizeOff = Attr->getKind() == OptimizeSizeAttr::Off;
+  }
+  if (HasOptsizeOn && !HasOptnone)
+F->addFnAttr(llvm::Attribute::OptimizeForSize);
+  if (HasOptsizeOff)
+F->removeFnAttr(llvm::Attribute::OptimizeForSize);
+
   if (GD.getDecl()->hasAttr()) {
 F->removeFnAttr("frame-pointer");
 FramePointerAttr *Attr = GD.getDecl()->getAttr();
Index: clang/include/clang/Basic/Attr.td
===
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -1451,6 +1451,16 @@
   let Documentation = [Undocumented];
 }
 
+def OptimizeSize: InheritableAttr {
+  // This attribute has no spellings as it is only ever created implicitly.
+  let Spellings = [];
+  let Subjects = SubjectList<[Function]>;
+  let Args = [EnumArgument<"Kind", "Kind",
+["off", "on"],
+["Off", "On"]>];
+  let Documentation = [Undocumented];
+}
+
 def FlagEnum : InheritableAttr {
   let Spellings = [Clang<"flag_enum">];
   let Subjects = SubjectList<[Enum]>;


Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -6763,6 +6763,19 @@
 checkSwiftAsyncErrorBlock(S, D, ErrorAttr, AsyncAttr);
 }
 
+static void handleOptimizeSizeAttr(Sema , Decl *D, const ParsedAttr ) {
+  if (!AL.isArgIdent(0)) {
+S.Diag(AL.getLoc(), diag::err_attribute_argument_n_type)
+<< AL << 1 << AANT_ArgumentIdentifier;
+return;
+  }
+
+  OptimizeSizeAttr::Kind Kind;
+  IdentifierInfo *II = AL.getArgAsIdent(0)->Ident;
+  OptimizeSizeAttr::ConvertStrToKind(II->getName(), Kind);
+  D->addAttr(::new (S.Context) OptimizeSizeAttr(S.Context, AL, Kind));
+}
+
 static void handleFramePointer(Sema , Decl *D, const ParsedAttr ) {
   if (!AL.isArgIdent(0)) {
 S.Diag(AL.getLoc(), diag::err_attribute_argument_n_type)
@@ -8534,6 +8547,9 @@
   case ParsedAttr::AT_MinSize:
 handleMinSizeAttr(S, D, AL);
 break;
+  case ParsedAttr::AT_OptimizeSize:
+handleOptimizeSizeAttr(S, D, AL);
+break;
   case ParsedAttr::AT_OptimizeNone:
 handleOptimizeNoneAttr(S, D, AL);
 break;
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -1653,6 +1653,19 @@
   if (!GD.getDecl())
 return;
 
+  bool HasOptnone = GD.getDecl()->hasAttr();
+  bool HasOptsizeOn = false;
+  bool HasOptsizeOff = false;
+  if (GD.getDecl()->hasAttr()) {
+OptimizeSizeAttr *Attr = GD.getDecl()->getAttr();
+HasOptsizeOn = Attr->getKind() == OptimizeSizeAttr::On;
+HasOptsizeOff = Attr->getKind() == OptimizeSizeAttr::Off;
+  }
+  if 

[PATCH] D125720: [Attribute] Add clang frame_pointer attribute

[Stephen Long via Phabricator via cfe-commits]

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

Add the ability to put __attribute__((frame_pointer(arg))) on functions.
arg is only allowed to be none, non_leaf, or all. Depends on D125719 
.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125720

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Sema/SemaDeclAttr.cpp

Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -6763,6 +6763,24 @@
 checkSwiftAsyncErrorBlock(S, D, ErrorAttr, AsyncAttr);
 }
 
+static void handleFramePointer(Sema , Decl *D, const ParsedAttr ) {
+  if (!AL.isArgIdent(0)) {
+S.Diag(AL.getLoc(), diag::err_attribute_argument_n_type)
+<< AL << 1 << AANT_ArgumentIdentifier;
+return;
+  }
+
+  FramePointerAttr::Kind Kind;
+  IdentifierInfo *II = AL.getArgAsIdent(0)->Ident;
+  if (!FramePointerAttr::ConvertStrToKind(II->getName(), Kind)) {
+S.Diag(AL.getLoc(), diag::warn_frame_pointer_type_not_supported) << AL
+ << II;
+return;
+  }
+
+  D->addAttr(::new (S.Context) FramePointerAttr(S.Context, AL, Kind));
+}
+
 //===--===//
 // Microsoft specific attribute handlers.
 //===--===//
@@ -9031,6 +9049,10 @@
   case ParsedAttr::AT_UsingIfExists:
 handleSimpleAttribute(S, D, AL);
 break;
+
+  case ParsedAttr::AT_FramePointer:
+handleFramePointer(S, D, AL);
+break;
   }
 }
 
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -1650,6 +1650,24 @@
  /*AttrOnCallSite=*/false, IsThunk);
   F->setAttributes(PAL);
   F->setCallingConv(static_cast(CallingConv));
+  if (!GD.getDecl())
+return;
+
+  if (GD.getDecl()->hasAttr()) {
+F->removeFnAttr("frame-pointer");
+FramePointerAttr *Attr = GD.getDecl()->getAttr();
+switch (Attr->getKind()) {
+case FramePointerAttr::None:
+  F->addFnAttr("frame-pointer", "none");
+  break;
+case FramePointerAttr::NonLeaf:
+  F->addFnAttr("frame-pointer", "non-leaf");
+  break;
+case FramePointerAttr::All:
+  F->addFnAttr("frame-pointer", "all");
+  break;
+}
+  }
 }
 
 static void removeImageAccessQualifier(std::string& TyName) {
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -11633,4 +11633,8 @@
   "a randomized struct can only be initialized with a designated initializer">;
 def err_cast_from_randomized_struct : Error<
   "casting from randomized structure pointer type %0 to %1">;
+
+def warn_frame_pointer_type_not_supported : Warning<
+  "%0 frame_pointer argument not supported: %1">,
+  InGroup;
 } // end of sema component.
Index: clang/include/clang/Basic/Attr.td
===
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -1441,6 +1441,16 @@
   let Documentation = [Undocumented];
 }
 
+def FramePointer : InheritableAttr {
+  // This attribute has no spellings as it is only ever created implicitly.
+  let Spellings = [];
+  let Subjects = SubjectList<[Function]>;
+  let Args = [EnumArgument<"Kind", "Kind",
+["none", "non_leaf", "all"],
+["None", "NonLeaf", "All"]>];
+  let Documentation = [Undocumented];
+}
+
 def FlagEnum : InheritableAttr {
   let Spellings = [Clang<"flag_enum">];
   let Subjects = SubjectList<[Enum]>;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123676: [clang-format] Fix WhitespaceSensitiveMacros not being honoured when macro closing parenthesis is followed by a newline.

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added a comment.

In D123676#3515949 , @krasimir wrote:

> It looks like this regressed the following example by adding an unwanted 
> level of indentation to the `#elif B` branch:

Sure, I'll have a look.
It seems that even this:

  MACRO_BEGIN
  #if A
  int f();
  #else
  int f();
  #endif

gets misindented:

  MACRO_BEGIN
  #if A
  int f();
  #else
  int
  f();
  #endif


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123676

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

[PATCH] D125719: [Attribute] Add attribute NeverOptimizeNone

[Stephen Long via Phabricator via cfe-commits]

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

Add NeverOptimizeNone to facilitate removing OptimizeNone. This allows
us to remove OptimizeNone even if -O0 is passed on the cmdline


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125719

Files:
  clang/include/clang/Basic/Attr.td
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Sema/SemaDeclAttr.cpp


Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -8519,6 +8519,9 @@
   case ParsedAttr::AT_OptimizeNone:
 handleOptimizeNoneAttr(S, D, AL);
 break;
+  case ParsedAttr::AT_NeverOptimizeNone:
+handleSimpleAttribute(S, D, AL);
+break;
   case ParsedAttr::AT_EnumExtensibility:
 handleEnumExtensibilityAttr(S, D, AL);
 break;
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -1928,7 +1928,8 @@
   ShouldAddOptNone &= !D->hasAttr();
 
   // Add optnone, but do so only if the function isn't always_inline.
-  if ((ShouldAddOptNone || D->hasAttr()) &&
+  if (!D->hasAttr() &&
+  (ShouldAddOptNone || D->hasAttr()) &&
   !F->hasFnAttribute(llvm::Attribute::AlwaysInline)) {
 B.addAttribute(llvm::Attribute::OptimizeNone);
 
Index: clang/include/clang/Basic/Attr.td
===
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -2255,6 +2255,13 @@
   let Documentation = [OptnoneDocs];
 }
 
+def NeverOptimizeNone : InheritableAttr {
+  // This attribute has no spellings as it is only ever created implicitly.
+  let Spellings = [];
+  let Subjects = SubjectList<[Function]>;
+  let Documentation = [Undocumented];
+}
+
 def Overloadable : Attr {
   let Spellings = [Clang<"overloadable">];
   let Subjects = SubjectList<[Function], ErrorDiag>;


Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -8519,6 +8519,9 @@
   case ParsedAttr::AT_OptimizeNone:
 handleOptimizeNoneAttr(S, D, AL);
 break;
+  case ParsedAttr::AT_NeverOptimizeNone:
+handleSimpleAttribute(S, D, AL);
+break;
   case ParsedAttr::AT_EnumExtensibility:
 handleEnumExtensibilityAttr(S, D, AL);
 break;
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -1928,7 +1928,8 @@
   ShouldAddOptNone &= !D->hasAttr();
 
   // Add optnone, but do so only if the function isn't always_inline.
-  if ((ShouldAddOptNone || D->hasAttr()) &&
+  if (!D->hasAttr() &&
+  (ShouldAddOptNone || D->hasAttr()) &&
   !F->hasFnAttribute(llvm::Attribute::AlwaysInline)) {
 B.addAttribute(llvm::Attribute::OptimizeNone);
 
Index: clang/include/clang/Basic/Attr.td
===
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -2255,6 +2255,13 @@
   let Documentation = [OptnoneDocs];
 }
 
+def NeverOptimizeNone : InheritableAttr {
+  // This attribute has no spellings as it is only ever created implicitly.
+  let Spellings = [];
+  let Subjects = SubjectList<[Function]>;
+  let Documentation = [Undocumented];
+}
+
 def Overloadable : Attr {
   let Spellings = [Clang<"overloadable">];
   let Subjects = SubjectList<[Function], ErrorDiag>;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D124726: Suggest typoed directives in preprocessor conditionals

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D124726#3516896 , @ken-matsui 
wrote:

> @nathanchance
>
> Oops, thank you for your comment!
> I would like to work on a hotfix patch for this issue.
>
> @aaron.ballman
>
> Should we entirely opt-out of this suggestion on `.S` files? I think there 
> are other approaches here, such as decreasing the max edit distance to avoid 
> suggesting this case in `.S` files, but this approach is avoiding just this 
> edge case so that it would possibly bring a new issue like this. Conversely, 
> opting out of this suggestion on the whole `.S` files will not be able to 
> catch any typoes. Considering possible edge cases (`#` directives are also 
> treated as comments), I think opting out would be the only way, 
> unfortunately. What do you think?
>
> For now, I will create a patch opting out of this suggestion on `.S` files.

I believe the `if (getLangOpts().AsmPreprocessor)` condition is appropriately 
accurate here.  I don't think changing the edit distance would be a good 
behavior for anyone, and I don't see any other appropriate heuristics.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124726

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

[PATCH] D125624: [gold] Remove an external dependency to GNU binutils' header file

[Peter Collingbourne via Phabricator via cfe-commits]

pcc added inline comments.



Comment at: llvm/tools/gold/gold-plugin.cpp:44-52
 // FIXME: remove this declaration when we stop maintaining Ubuntu Quantal and
 // Precise and Debian Wheezy (binutils 2.23 is required)
 #define LDPO_PIE 3
 
 #define LDPT_GET_SYMBOLS_V3 28
 
 // FIXME: Remove when binutils 2.31 (containing gold 1.16) is the minimum

Can we remove this stuff now?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125624

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

[PATCH] D125683: [runtimes] Replace LIBCXX_ENABLE_STATIC_ABI_LIBRARY & friends by a new LIBCXX_CXX_ABI choice

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

In D125683#3516883 , @ldionne wrote:

> In D125683#3515675 , @mstorsjo 
> wrote:
>
>> It looks like this patch still lacks compatibility handling for all external 
>> users that are setting this option, which need to be able to set the option 
>> in this way at least until after the 15.0.0 release.
>
> What would you think about erroring out if someone is still passing 
> `LIBCXX_STATICALLY_LINK_ABI_IN_{SHARED|STATIC}_LIBRARY`? Sure, we can try to 
> support both for a while, but I wonder whether there's any actual benefit to 
> balance the added complexity. Concretely, I suspect this impacts very few 
> people, and the fix is really simple. And if we error out during the CMake 
> step, we don't risk breaking anyone silently.

I guess that's better than silently (well, cmake warns, but it's easy to miss) 
no longer doing what it used to, but it's not ideal IMO. I do my continuous 
build testing with a build invocation that also works for the latest release - 
but with the suggested transition, there's no single build invocation that 
works for both the main branch and the 14.x release. I guess I could manage 
though, but I'd prefer a smoother upgrade path.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125683

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

[PATCH] D125624: [gold] Remove an external dependency to GNU binutils' header file

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

I haven't checked but for `ninja check-llvm-tools-gold`, we still hope that the 
tests are disabled if the system does not provide gold. There may need a toggle 
for the tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125624

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

[PATCH] D124726: Suggest typoed directives in preprocessor conditionals

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui added a comment.

@nathanchance

Oops, thank you for your comment!
I would like to work on a hotfix patch for this issue.

@aaron.ballman

Should we entirely opt-out of this suggestion on `.S` files? I think there are 
other approaches here, such as decreasing the max edit distance to avoid 
suggesting this case in `.S` files, but this approach is avoiding just this 
edge case so that it would possibly bring a new issue like this. Conversely, 
opting out of this suggestion on the whole `.S` files will not be able to catch 
any typoes. Considering possible edge cases (`#` directives are also treated as 
comments), I think opting out would be the only way, unfortunately. What do you 
think?

For now, I will create a patch opting out of this suggestion on `.S` files.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124726

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

[PATCH] D125683: [runtimes] Replace LIBCXX_ENABLE_STATIC_ABI_LIBRARY & friends by a new LIBCXX_CXX_ABI choice

[Louis Dionne via Phabricator via cfe-commits]

ldionne added a comment.

In D125683#3515675 , @mstorsjo wrote:

> It looks like this patch still lacks compatibility handling for all external 
> users that are setting this option, which need to be able to set the option 
> in this way at least until after the 15.0.0 release.

What would you think about erroring out if someone is still passing 
`LIBCXX_STATICALLY_LINK_ABI_IN_{SHARED|STATIC}_LIBRARY`? Sure, we can try to 
support both for a while, but I wonder whether there's any actual benefit to 
balance the added complexity. Concretely, I suspect this impacts very few 
people, and the fix is really simple. And if we error out during the CMake 
step, we don't risk breaking anyone silently.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125683

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

[PATCH] D125675: Optimise findRefs for XRefs and docHighlights

[Utkarsh Saxena 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 rG2fb6ece2ca83: Optimise findRefs for XRefs and docHighlights 
(authored by usaxena95).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125675

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

Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -18,6 +18,7 @@
 #include "index/Index.h"
 #include "index/Merge.h"
 #include "index/Relation.h"
+#include "index/SymbolID.h"
 #include "index/SymbolLocation.h"
 #include "support/Logger.h"
 #include "clang/AST/ASTContext.h"
@@ -47,10 +48,12 @@
 #include "clang/Index/USRGeneration.h"
 #include "clang/Tooling/Syntax/Tokens.h"
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/None.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/SmallSet.h"
+#include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/Error.h"
@@ -397,8 +400,8 @@
 }
   }
   // Special case: void foo() ^override: jump to the overridden method.
-if (NodeKind.isSame(ASTNodeKind::getFromNodeKind()) ||
-NodeKind.isSame(ASTNodeKind::getFromNodeKind())) {
+  if (NodeKind.isSame(ASTNodeKind::getFromNodeKind()) ||
+  NodeKind.isSame(ASTNodeKind::getFromNodeKind())) {
 // We may be overridding multiple methods - offer them all.
 for (const NamedDecl *ND : CMD->overridden_methods())
   AddResultDecl(ND);
@@ -887,8 +890,13 @@
   };
 
   ReferenceFinder(const ParsedAST ,
-  const llvm::DenseSet , bool PerToken)
-  : PerToken(PerToken), AST(AST), TargetIDs(TargetIDs) {}
+  const llvm::ArrayRef Targets, bool PerToken)
+  : PerToken(PerToken), AST(AST) {
+for (const NamedDecl *ND : Targets) {
+  const Decl *CD = ND->getCanonicalDecl();
+  TargetDeclToID[CD] = getSymbolID(CD);
+}
+  }
 
   std::vector take() && {
 llvm::sort(References, [](const Reference , const Reference ) {
@@ -913,12 +921,12 @@
llvm::ArrayRef Relations,
SourceLocation Loc,
index::IndexDataConsumer::ASTNodeInfo ASTNode) override {
+auto DeclID = TargetDeclToID.find(D->getCanonicalDecl());
+if (DeclID == TargetDeclToID.end())
+  return true;
 const SourceManager  = AST.getSourceManager();
 if (!isInsideMainFile(Loc, SM))
   return true;
-SymbolID ID = getSymbolID(D);
-if (!TargetIDs.contains(ID))
-  return true;
 const auto  = AST.getTokens();
 
 llvm::SmallVector Locs;
@@ -942,7 +950,7 @@
 for (SourceLocation L : Locs) {
   L = SM.getFileLoc(L);
   if (const auto *Tok = TB.spelledTokenAt(L))
-References.push_back({*Tok, Roles, ID});
+References.push_back({*Tok, Roles, DeclID->getSecond()});
 }
 return true;
   }
@@ -951,12 +959,13 @@
   bool PerToken; // If true, report 3 references for split ObjC selector names.
   std::vector References;
   const ParsedAST 
-  const llvm::DenseSet 
+  llvm::DenseMap TargetDeclToID;
 };
 
 std::vector
-findRefs(const llvm::DenseSet , ParsedAST , bool PerToken) {
-  ReferenceFinder RefFinder(AST, IDs, PerToken);
+findRefs(const llvm::ArrayRef TargetDecls, ParsedAST ,
+ bool PerToken) {
+  ReferenceFinder RefFinder(AST, TargetDecls, PerToken);
   index::IndexingOptions IndexOpts;
   IndexOpts.SystemSymbolFilter =
   index::IndexingOptions::SystemSymbolFilterKind::All;
@@ -1242,16 +1251,12 @@
 if (const SelectionTree::Node *N = ST.commonAncestor()) {
   DeclRelationSet Relations =
   DeclRelation::TemplatePattern | DeclRelation::Alias;
-  auto Decls =
-  targetDecl(N->ASTNode, Relations, AST.getHeuristicResolver());
-  if (!Decls.empty()) {
+  auto TargetDecls=
+   targetDecl(N->ASTNode, Relations, AST.getHeuristicResolver());
+  if (!TargetDecls.empty()) {
 // FIXME: we may get multiple DocumentHighlights with the same location
 // and different kinds, deduplicate them.
-llvm::DenseSet Targets;
-for (const NamedDecl *ND : Decls)
-  if (auto ID = getSymbolID(ND))
-Targets.insert(ID);
-for (const auto  : findRefs(Targets, AST, /*PerToken=*/true))
+for (const auto  : findRefs(TargetDecls, AST, /*PerToken=*/true))
   Result.push_back(toHighlight(Ref, SM));
 return true;
   }
@@ -1377,12 +1382,12 @@
 DeclRelation::TemplatePattern | DeclRelation::Alias;
 std::vector Decls =
 getDeclAtPosition(AST, *CurLoc, Relations);
-llvm::DenseSet TargetsInMainFile;
+llvm::SmallVector 

[clang-tools-extra] 2fb6ece - Optimise findRefs for XRefs and docHighlights

[Utkarsh Saxena via cfe-commits]

Author: Utkarsh Saxena
Date: 2022-05-16T21:07:14+02:00
New Revision: 2fb6ece2ca83d33909c628d682f821d05aa67a97

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

LOG: Optimise findRefs for XRefs and docHighlights

Reduces time spent in findRef by 66%.

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

Added: 


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

Removed: 




diff  --git a/clang-tools-extra/clangd/XRefs.cpp 
b/clang-tools-extra/clangd/XRefs.cpp
index 83175a8c721c..e090c01435c8 100644
--- a/clang-tools-extra/clangd/XRefs.cpp
+++ b/clang-tools-extra/clangd/XRefs.cpp
@@ -18,6 +18,7 @@
 #include "index/Index.h"
 #include "index/Merge.h"
 #include "index/Relation.h"
+#include "index/SymbolID.h"
 #include "index/SymbolLocation.h"
 #include "support/Logger.h"
 #include "clang/AST/ASTContext.h"
@@ -47,10 +48,12 @@
 #include "clang/Index/USRGeneration.h"
 #include "clang/Tooling/Syntax/Tokens.h"
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/None.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/SmallSet.h"
+#include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/Error.h"
@@ -397,8 +400,8 @@ locateASTReferent(SourceLocation CurLoc, const 
syntax::Token *TouchedIdentifier,
 }
   }
   // Special case: void foo() ^override: jump to the overridden method.
-if (NodeKind.isSame(ASTNodeKind::getFromNodeKind()) ||
-NodeKind.isSame(ASTNodeKind::getFromNodeKind())) {
+  if (NodeKind.isSame(ASTNodeKind::getFromNodeKind()) ||
+  NodeKind.isSame(ASTNodeKind::getFromNodeKind())) {
 // We may be overridding multiple methods - offer them all.
 for (const NamedDecl *ND : CMD->overridden_methods())
   AddResultDecl(ND);
@@ -887,8 +890,13 @@ class ReferenceFinder : public index::IndexDataConsumer {
   };
 
   ReferenceFinder(const ParsedAST ,
-  const llvm::DenseSet , bool PerToken)
-  : PerToken(PerToken), AST(AST), TargetIDs(TargetIDs) {}
+  const llvm::ArrayRef Targets, bool 
PerToken)
+  : PerToken(PerToken), AST(AST) {
+for (const NamedDecl *ND : Targets) {
+  const Decl *CD = ND->getCanonicalDecl();
+  TargetDeclToID[CD] = getSymbolID(CD);
+}
+  }
 
   std::vector take() && {
 llvm::sort(References, [](const Reference , const Reference ) {
@@ -913,12 +921,12 @@ class ReferenceFinder : public index::IndexDataConsumer {
llvm::ArrayRef Relations,
SourceLocation Loc,
index::IndexDataConsumer::ASTNodeInfo ASTNode) override 
{
+auto DeclID = TargetDeclToID.find(D->getCanonicalDecl());
+if (DeclID == TargetDeclToID.end())
+  return true;
 const SourceManager  = AST.getSourceManager();
 if (!isInsideMainFile(Loc, SM))
   return true;
-SymbolID ID = getSymbolID(D);
-if (!TargetIDs.contains(ID))
-  return true;
 const auto  = AST.getTokens();
 
 llvm::SmallVector Locs;
@@ -942,7 +950,7 @@ class ReferenceFinder : public index::IndexDataConsumer {
 for (SourceLocation L : Locs) {
   L = SM.getFileLoc(L);
   if (const auto *Tok = TB.spelledTokenAt(L))
-References.push_back({*Tok, Roles, ID});
+References.push_back({*Tok, Roles, DeclID->getSecond()});
 }
 return true;
   }
@@ -951,12 +959,13 @@ class ReferenceFinder : public index::IndexDataConsumer {
   bool PerToken; // If true, report 3 references for split ObjC selector names.
   std::vector References;
   const ParsedAST 
-  const llvm::DenseSet 
+  llvm::DenseMap TargetDeclToID;
 };
 
 std::vector
-findRefs(const llvm::DenseSet , ParsedAST , bool PerToken) {
-  ReferenceFinder RefFinder(AST, IDs, PerToken);
+findRefs(const llvm::ArrayRef TargetDecls, ParsedAST ,
+ bool PerToken) {
+  ReferenceFinder RefFinder(AST, TargetDecls, PerToken);
   index::IndexingOptions IndexOpts;
   IndexOpts.SystemSymbolFilter =
   index::IndexingOptions::SystemSymbolFilterKind::All;
@@ -1242,16 +1251,12 @@ std::vector 
findDocumentHighlights(ParsedAST ,
 if (const SelectionTree::Node *N = ST.commonAncestor()) {
   DeclRelationSet Relations =
   DeclRelation::TemplatePattern | DeclRelation::Alias;
-  auto Decls =
-  targetDecl(N->ASTNode, Relations, AST.getHeuristicResolver());
-  if (!Decls.empty()) {
+  auto TargetDecls=
+   targetDecl(N->ASTNode, Relations, AST.getHeuristicResolver());
+  if (!TargetDecls.empty()) {
 // FIXME: we may get multiple DocumentHighlights with the same location
 // and 
diff erent kinds, deduplicate them.
-llvm::DenseSet Targets;
- 

[PATCH] D125675: Optimise findRefs for XRefs and docHighlights

[Utkarsh Saxena via Phabricator via cfe-commits]

usaxena95 updated this revision to Diff 429807.
usaxena95 marked 4 inline comments as done.
usaxena95 added a comment.

Addressed comments and added performance measurements.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125675

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

Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -18,6 +18,7 @@
 #include "index/Index.h"
 #include "index/Merge.h"
 #include "index/Relation.h"
+#include "index/SymbolID.h"
 #include "index/SymbolLocation.h"
 #include "support/Logger.h"
 #include "clang/AST/ASTContext.h"
@@ -47,10 +48,12 @@
 #include "clang/Index/USRGeneration.h"
 #include "clang/Tooling/Syntax/Tokens.h"
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/None.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/SmallSet.h"
+#include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/Error.h"
@@ -397,8 +400,8 @@
 }
   }
   // Special case: void foo() ^override: jump to the overridden method.
-if (NodeKind.isSame(ASTNodeKind::getFromNodeKind()) ||
-NodeKind.isSame(ASTNodeKind::getFromNodeKind())) {
+  if (NodeKind.isSame(ASTNodeKind::getFromNodeKind()) ||
+  NodeKind.isSame(ASTNodeKind::getFromNodeKind())) {
 // We may be overridding multiple methods - offer them all.
 for (const NamedDecl *ND : CMD->overridden_methods())
   AddResultDecl(ND);
@@ -887,8 +890,13 @@
   };
 
   ReferenceFinder(const ParsedAST ,
-  const llvm::DenseSet , bool PerToken)
-  : PerToken(PerToken), AST(AST), TargetIDs(TargetIDs) {}
+  const llvm::ArrayRef Targets, bool PerToken)
+  : PerToken(PerToken), AST(AST) {
+for (const NamedDecl *ND : Targets) {
+  const Decl *CD = ND->getCanonicalDecl();
+  TargetDeclToID[CD] = getSymbolID(CD);
+}
+  }
 
   std::vector take() && {
 llvm::sort(References, [](const Reference , const Reference ) {
@@ -913,12 +921,12 @@
llvm::ArrayRef Relations,
SourceLocation Loc,
index::IndexDataConsumer::ASTNodeInfo ASTNode) override {
+auto DeclID = TargetDeclToID.find(D->getCanonicalDecl());
+if (DeclID == TargetDeclToID.end())
+  return true;
 const SourceManager  = AST.getSourceManager();
 if (!isInsideMainFile(Loc, SM))
   return true;
-SymbolID ID = getSymbolID(D);
-if (!TargetIDs.contains(ID))
-  return true;
 const auto  = AST.getTokens();
 
 llvm::SmallVector Locs;
@@ -942,7 +950,7 @@
 for (SourceLocation L : Locs) {
   L = SM.getFileLoc(L);
   if (const auto *Tok = TB.spelledTokenAt(L))
-References.push_back({*Tok, Roles, ID});
+References.push_back({*Tok, Roles, DeclID->getSecond()});
 }
 return true;
   }
@@ -951,12 +959,13 @@
   bool PerToken; // If true, report 3 references for split ObjC selector names.
   std::vector References;
   const ParsedAST 
-  const llvm::DenseSet 
+  llvm::DenseMap TargetDeclToID;
 };
 
 std::vector
-findRefs(const llvm::DenseSet , ParsedAST , bool PerToken) {
-  ReferenceFinder RefFinder(AST, IDs, PerToken);
+findRefs(const llvm::ArrayRef TargetDecls, ParsedAST ,
+ bool PerToken) {
+  ReferenceFinder RefFinder(AST, TargetDecls, PerToken);
   index::IndexingOptions IndexOpts;
   IndexOpts.SystemSymbolFilter =
   index::IndexingOptions::SystemSymbolFilterKind::All;
@@ -1242,16 +1251,12 @@
 if (const SelectionTree::Node *N = ST.commonAncestor()) {
   DeclRelationSet Relations =
   DeclRelation::TemplatePattern | DeclRelation::Alias;
-  auto Decls =
-  targetDecl(N->ASTNode, Relations, AST.getHeuristicResolver());
-  if (!Decls.empty()) {
+  auto TargetDecls=
+   targetDecl(N->ASTNode, Relations, AST.getHeuristicResolver());
+  if (!TargetDecls.empty()) {
 // FIXME: we may get multiple DocumentHighlights with the same location
 // and different kinds, deduplicate them.
-llvm::DenseSet Targets;
-for (const NamedDecl *ND : Decls)
-  if (auto ID = getSymbolID(ND))
-Targets.insert(ID);
-for (const auto  : findRefs(Targets, AST, /*PerToken=*/true))
+for (const auto  : findRefs(TargetDecls, AST, /*PerToken=*/true))
   Result.push_back(toHighlight(Ref, SM));
 return true;
   }
@@ -1377,12 +1382,12 @@
 DeclRelation::TemplatePattern | DeclRelation::Alias;
 std::vector Decls =
 getDeclAtPosition(AST, *CurLoc, Relations);
-llvm::DenseSet TargetsInMainFile;
+llvm::SmallVector TargetsInMainFile;
 for (const NamedDecl *D : Decls) {
   auto 

[PATCH] D123534: [dwarf] Emit a DIGlobalVariable for constant strings.

[Mitch Phillips via Phabricator via cfe-commits]

hctim updated this revision to Diff 429799.
hctim added a comment.

Touch ups from comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123534

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGDebugInfo.h
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGen/debug-info-variables.c
  clang/test/VFS/external-names.c
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
  llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
  llvm/test/Assembler/invalid-diglobalvariable-missing-name.ll
  llvm/test/DebugInfo/COFF/global-no-strings.ll

Index: llvm/test/DebugInfo/COFF/global-no-strings.ll
===
--- /dev/null
+++ llvm/test/DebugInfo/COFF/global-no-strings.ll
@@ -0,0 +1,59 @@
+; RUN: llc < %s | FileCheck %s
+; RUN: llc < %s -filetype=obj | llvm-readobj - --codeview | FileCheck %s
+
+; This test ensures that no debuginfo is emitted for the constant "123456789"
+; string. These global variables have debug expressions because DWARF has the
+; ability to tie them to a file name and line number, but this isn't possible
+; in CodeView, so we make sure it's omitted to save file size.
+;
+; The various CodeView outputs should have a description of "my_string", but not
+; the string contents itself.
+
+; C++ source to regenerate:
+; $ cat a.cpp
+; char* my_string =
+;   "12345679";
+;
+; $ clang-cl a.cpp /GS- /Z7 /GR- /clang:-S /clang:-emit-llvm
+
+; CHECK-NOT: S_LDATA
+; CHECK: S_GDATA
+; CHECK-NOT: S_LDATA
+; CHECK-NOT: S_GDATA
+
+; ModuleID = '/tmp/a.c'
+source_filename = "/tmp/a.c"
+target datalayout = "e-m:w-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-pc-windows-msvc19.20.0"
+
+$"??_C@_08HCBFHPJA@12345679?$AA@" = comdat any
+
+@"??_C@_08HCBFHPJA@12345679?$AA@" = linkonce_odr dso_local unnamed_addr constant [9 x i8] c"12345679\00", comdat, align 1, !dbg !0
+@my_string = dso_local global ptr @"??_C@_08HCBFHPJA@12345679?$AA@", align 8, !dbg !7
+
+!llvm.dbg.cu = !{!9}
+!llvm.linker.options = !{!13, !14}
+!llvm.module.flags = !{!15, !16, !17, !18, !19}
+!llvm.ident = !{!20}
+
+!0 = !DIGlobalVariableExpression(var: !1, expr: !DIExpression())
+!1 = distinct !DIGlobalVariable(scope: null, file: !2, line: 2, type: !3, isLocal: true, isDefinition: true)
+!2 = !DIFile(filename: "/tmp/a.c", directory: "", checksumkind: CSK_MD5, checksum: "b972961d64de3c90497767110ab58eb6")
+!3 = !DICompositeType(tag: DW_TAG_array_type, baseType: !4, size: 72, elements: !5)
+!4 = !DIBasicType(name: "char", size: 8, encoding: DW_ATE_signed_char)
+!5 = !{!6}
+!6 = !DISubrange(count: 9)
+!7 = !DIGlobalVariableExpression(var: !8, expr: !DIExpression())
+!8 = distinct !DIGlobalVariable(name: "my_string", scope: !9, file: !2, line: 1, type: !12, isLocal: false, isDefinition: true)
+!9 = distinct !DICompileUnit(language: DW_LANG_C99, file: !10, producer: "clang version 15.0.0 (https://github.com/llvm/llvm-project.git 7c1c0849f8a1a6f1bf5f5b554484bbf8b0debd0a)", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, globals: !11, splitDebugInlining: false, nameTableKind: None)
+!10 = !DIFile(filename: "/tmp/a.c", directory: "/usr/local/google/home/mitchp/llvm-build/opt", checksumkind: CSK_MD5, checksum: "b972961d64de3c90497767110ab58eb6")
+!11 = !{!0, !7}
+!12 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !4, size: 64)
+!13 = !{!"/DEFAULTLIB:libcmt.lib"}
+!14 = !{!"/DEFAULTLIB:oldnames.lib"}
+!15 = !{i32 2, !"CodeView", i32 1}
+!16 = !{i32 2, !"Debug Info Version", i32 3}
+!17 = !{i32 1, !"wchar_size", i32 2}
+!18 = !{i32 7, !"PIC Level", i32 2}
+!19 = !{i32 7, !"uwtable", i32 2}
+!20 = !{!"clang version 15.0.0 (https://github.com/llvm/llvm-project.git 7c1c0849f8a1a6f1bf5f5b554484bbf8b0debd0a)"}
Index: llvm/test/Assembler/invalid-diglobalvariable-missing-name.ll
===
--- llvm/test/Assembler/invalid-diglobalvariable-missing-name.ll
+++ /dev/null
@@ -1,4 +0,0 @@
-; RUN: not llvm-as < %s -disable-output 2>&1 | FileCheck %s
-
-; CHECK: :[[@LINE+1]]:24: error: missing required field 'name'
-!0 = !DIGlobalVariable()
Index: llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
===
--- llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
+++ llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
@@ -165,7 +165,9 @@
   } else {
 DeclContext = GV->getScope();
 // Add name and type.
-addString(*VariableDIE, dwarf::DW_AT_name, GV->getDisplayName());
+StringRef DisplayName = GV->getDisplayName();
+if (!DisplayName.empty())
+  addString(*VariableDIE, dwarf::DW_AT_name, GV->getDisplayName());
 if (GTy)
   addType(*VariableDIE, GTy);
 
Index: llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
===
--- 

[PATCH] D125682: Add documentHighlight in clangd check for performance measurements.

[Utkarsh Saxena via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG5edd7665fd16: Add documentHighlight in clangd check for 
performance measurements. (authored by usaxena95).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125682

Files:
  clang-tools-extra/clangd/tool/Check.cpp


Index: clang-tools-extra/clangd/tool/Check.cpp
===
--- clang-tools-extra/clangd/tool/Check.cpp
+++ clang-tools-extra/clangd/tool/Check.cpp
@@ -255,6 +255,9 @@
   auto Hover = getHover(*AST, Pos, Style, );
   vlog("hover: {0}", Hover.hasValue());
 
+  unsigned DocHighlights = findDocumentHighlights(*AST, Pos).size();
+  vlog("documentHighlight: {0}", DocHighlights);
+
   if (EnableCodeCompletion) {
 Position EndPos = offsetToPosition(Inputs.Contents, End);
 auto CC = codeComplete(File, EndPos, Preamble.get(), Inputs, CCOpts);


Index: clang-tools-extra/clangd/tool/Check.cpp
===
--- clang-tools-extra/clangd/tool/Check.cpp
+++ clang-tools-extra/clangd/tool/Check.cpp
@@ -255,6 +255,9 @@
   auto Hover = getHover(*AST, Pos, Style, );
   vlog("hover: {0}", Hover.hasValue());
 
+  unsigned DocHighlights = findDocumentHighlights(*AST, Pos).size();
+  vlog("documentHighlight: {0}", DocHighlights);
+
   if (EnableCodeCompletion) {
 Position EndPos = offsetToPosition(Inputs.Contents, End);
 auto CC = codeComplete(File, EndPos, Preamble.get(), Inputs, CCOpts);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] 5edd766 - Add documentHighlight in clangd check for performance measurements.

[Utkarsh Saxena via cfe-commits]

Author: Utkarsh Saxena
Date: 2022-05-16T20:22:36+02:00
New Revision: 5edd7665fd163555f1afe2187907eba95d24c11b

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

LOG: Add documentHighlight in clangd check for performance measurements.

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

Added: 


Modified: 
clang-tools-extra/clangd/tool/Check.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/tool/Check.cpp 
b/clang-tools-extra/clangd/tool/Check.cpp
index 7b73215364a7d..492da45aed754 100644
--- a/clang-tools-extra/clangd/tool/Check.cpp
+++ b/clang-tools-extra/clangd/tool/Check.cpp
@@ -255,6 +255,9 @@ class Checker {
   auto Hover = getHover(*AST, Pos, Style, );
   vlog("hover: {0}", Hover.hasValue());
 
+  unsigned DocHighlights = findDocumentHighlights(*AST, Pos).size();
+  vlog("documentHighlight: {0}", DocHighlights);
+
   if (EnableCodeCompletion) {
 Position EndPos = offsetToPosition(Inputs.Contents, End);
 auto CC = codeComplete(File, EndPos, Preamble.get(), Inputs, CCOpts);



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

[PATCH] D101471: [clang-tidy] Add proper emplace checks to modernize-use-emplace

[Jakub Kuderski via Phabricator via cfe-commits]

kuhar resigned from this revision.
kuhar added inline comments.
Herald added a subscriber: carlosgalvezp.
Herald added a project: All.



Comment at: clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp:44
+  // FullNameTrimmed matches any of the given Names.
+  const StringRef FullNameTrimmedRef = StringRef(FullNameTrimmed);
+  for (const StringRef Pattern : Names) {

nit: `StringRef(` seems unnecessary on the rhs of `=`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101471

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

[PATCH] D123538: [symbolizer] Parse DW_TAG_variable DIs to show line info for globals

[Mitch Phillips via Phabricator via cfe-commits]

hctim added inline comments.



Comment at: llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp:769-783
+DataExtractor Data(Location.Expr, /*IsLittleEndian=*/true, 8);
+uint64_t DataOffset = 0;
+uint8_t Operation = Data.getU8();
+if (Operation == dwarf::DW_OP_addr) {
+  uint64_t Pointer = Data.getAddress();
+  VariableDieMap[Pointer] = Die;
+  return;

dblaikie wrote:
> Are there any examples of global merge where this might not be correct?
> 
> Like if a global was described as "addrx, 5, add" (eg: 5 beyond this address) 
> then only looking at the first operation might mis-conclude that the variable 
> is at this address when it's at 5 bytes past this address - and some other 
> variable is at this address)
> 
> LLVM has a "global merge" optimization that might cause something like this. 
> Let's see if I can create an example.
> 
> Ah, yeah, here we go:
> ```
> static int x;
> static int y;
> int *f(bool b) { return b ?  :  }
> ```
> ```
> $ clang++-tot -O3 merge.cpp -c -g -target aarch64-linux-gnuabi -mllvm 
> -global-merge-ignore-single-use && llvm-dwarfdump-tot merge.o | grep 
> DW_AT_location
> DW_AT_location  (DW_OP_addrx 0x0)
> DW_AT_location  (DW_OP_addrx 0x0, DW_OP_plus_uconst 0x4)
> ```
> 
> (the `-global-merge-ignore-single-use` is to simplify the test case - without 
> that it could still be tickled with a more complicated example - seems we 
> only enable global merge on ARM 32 and 64 bit due to the higher register 
> pressure there, by the sounds of it)
> 
> I'm guessing this patch would overwrite the VariableDieMap entry for the 
> first global with the second one so queries for the first address would 
> result in the second DIE and queries for the second address wouldn't give any 
> result?
> 
> Hmm, also - how does this handle queries that aren't at exactly the starting 
> address of the variable? Presumably the `DW_AT_type` of the 
> `DW_TAG_global_variable` would have to be inspected to find the size of the 
> variable starting at the address, and any query in that range should be 
> successful?
I've added some handling for the `DW_OP_plus_uconst`. Unfortunately I didn't 
find any generic existing handling for the expression evaluation (which 
surprised me, maybe I just suck at looking), so I'm just making the assumption 
that global variable addresses aren't described using `DW_OP_plus` or anything 
else...

Fixed it up to now provide line info when you provide an address halfway 
through a symbol. Good thing there wasn't a big impact in D123534 about keeping 
the string sizes around :). Worst case (if the type info sucks) we just only 
accept the precise start addresses of the symbols.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123538

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

[PATCH] D123538: [symbolizer] Parse DW_TAG_variable DIs to show line info for globals

[Mitch Phillips via Phabricator via cfe-commits]

hctim updated this revision to Diff 429783.
hctim marked 2 inline comments as done.
hctim added a comment.

Patch update.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123538

Files:
  llvm/include/llvm/DebugInfo/DIContext.h
  llvm/include/llvm/DebugInfo/DWARF/DWARFContext.h
  llvm/include/llvm/DebugInfo/DWARF/DWARFDie.h
  llvm/include/llvm/DebugInfo/DWARF/DWARFUnit.h
  llvm/include/llvm/DebugInfo/PDB/PDBContext.h
  llvm/lib/DebugInfo/DWARF/DWARFContext.cpp
  llvm/lib/DebugInfo/DWARF/DWARFDie.cpp
  llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp
  llvm/lib/DebugInfo/PDB/PDBContext.cpp
  llvm/lib/DebugInfo/Symbolize/DIPrinter.cpp
  llvm/lib/DebugInfo/Symbolize/SymbolizableObjectFile.cpp
  llvm/test/DebugInfo/Symbolize/ELF/data-command-symtab.yaml
  llvm/test/tools/llvm-symbolizer/data-location.yaml
  llvm/test/tools/llvm-symbolizer/data.s

Index: llvm/test/tools/llvm-symbolizer/data.s
===
--- llvm/test/tools/llvm-symbolizer/data.s
+++ llvm/test/tools/llvm-symbolizer/data.s
@@ -7,9 +7,12 @@
 
 # CHECK:  d1
 # CHECK-NEXT: 0 8
+# CHECK-NEXT: ??:?
 # CHECK-EMPTY:
 # CHECK-NEXT: d2
 # CHECK-NEXT: 8 4
+# CHECK-NEXT: ??:?
+# CHECK-EMPTY:
 
 d1:
 .quad 0x1122334455667788
Index: llvm/test/tools/llvm-symbolizer/data-location.yaml
===
--- /dev/null
+++ llvm/test/tools/llvm-symbolizer/data-location.yaml
@@ -0,0 +1,450 @@
+## Show that when "DATA" is used with an address, it forces the found location
+## to be symbolized as data, including the source information.
+
+# RUN: yaml2obj %s -o %t.so
+
+# RUN: llvm-symbolizer 'DATA 0x304d0' 'DATA 0x304d1' 'DATA 0x304d3' \
+# RUN:   'DATA 0x304c0' 'DATA 0x304c8' 'DATA 0x304d4' 'DATA 0x304dc' \
+# RUN:   'DATA 0x304d8' --obj=%t.so | FileCheck %s
+
+# CHECK:  bss_global
+# CHECK-NEXT: {{[0-9]+}} 4
+# CHECK-NEXT: /tmp/file.cpp:1
+# CHECK-EMPTY:
+
+## Check that lookups in the middle of the symbol are also resolved correctly.
+# CHECK:  bss_global
+# CHECK-NEXT: {{[0-9]+}} 4
+# CHECK-NEXT: /tmp/file.cpp:1
+# CHECK-EMPTY:
+# CHECK:  bss_global
+# CHECK-NEXT: {{[0-9]+}} 4
+# CHECK-NEXT: /tmp/file.cpp:1
+# CHECK-EMPTY:
+
+## Now, the remainder of the symbols.
+# CHECK-NEXT: data_global
+# CHECK-NEXT: {{[0-9]+}} 4
+# CHECK-NEXT: /tmp/file.cpp:2
+# CHECK-EMPTY:
+# CHECK-NEXT: str
+# CHECK-NEXT: {{[0-9]+}} 8
+# CHECK-NEXT: /tmp/file.cpp:4
+# CHECK-EMPTY:
+# CHECK-NEXT: f()::function_global
+# CHECK-NEXT: {{[0-9]+}} 4
+# CHECK-NEXT: /tmp/file.cpp:8
+# CHECK-EMPTY:
+
+## Including the one that includes an addend.
+# CHECK-NEXT: alpha
+# CHECK-NEXT: {{[0-9]+}} 4
+# CHECK-NEXT: /tmp/file.cpp:12
+# CHECK-EMPTY:
+# CHECK-NEXT: beta
+# CHECK-NEXT: {{[0-9]+}} 4
+# CHECK-NEXT: /tmp/file.cpp:13
+# CHECK-EMPTY:
+
+## Ensure there's still a global that's offset-based.
+# RUN: llvm-dwarfdump --debug-info %t.so | FileCheck %s --check-prefix=OFFSET
+
+# OFFSET: DW_AT_location (DW_OP_addrx 0x4, DW_OP_plus_uconst 0x4)
+
+
+## File below was generated using:
+##
+##   $ clang++ -g -O3 /tmp/file.cpp -shared -fuse-ld=lld -nostdlib \
+## -target aarch64-linux-gnuabi -mllvm -global-merge-ignore-single-use \
+## -o /tmp/file.so
+##
+##  With /tmp/file.cpp as:
+##1: int bss_global;
+##2: int data_global = 2;
+##3:
+##4: const char* str =
+##5: "12345678";
+##6:
+##7: int* f() {
+##8:   static int function_global;
+##9:   return _global;
+##   10: }
+##   11:
+##   12: static int alpha;
+##   13: static int beta;
+##   14: int *f(bool b) { return beta ?  :  }
+##   15:
+##
+## ... then, one can get the offsets using `nm`, like:
+##   $ nm out.so | grep bss_global
+## 38fc B bss_global
+##
+## Note the use of the aarch64 target (with -nostdlib in order to allow linkage
+## without libraries for cross-compilation) as well as -O3 and
+## -global-merge-ignore-single-use. This is a specific combination that makes
+## the compiler emit the `alpha` global variable with a more complex
+## DW_AT_location than just a DW_OP_addr/DW_OP_addrx. In this instance, it
+## outputs a `DW_AT_location  (DW_OP_addrx 0x4, DW_OP_plus_uconst 0x4)`.
+##
+## Ideally, this would be tested by invoking clang directly on a C source file,
+## but unfortunately there's no way to do that for LLVM tests. The other option
+## is to compile IR to an objfile, but llvm-symbolizer doesn't understand that
+## two symbols can have the same address in different sections. In the code
+## above, for example, we'd have bss_global at .bss+0x0, and data_global at
+## .data+0x0, and so the symbolizer would only print one of them. Hence, we have
+## the ugly dso-to-yaml blob below.
+##
+## For now, constant strings don't have a debuginfo entry, and so can't be
+## symbolized correctly. In future 

[PATCH] D125711: [concepts] Implement dcl.decl.general p4: No constraints on non-template funcs

[Erich Keane via Phabricator via cfe-commits]

erichkeane created this revision.
erichkeane added a reviewer: clang-language-wg.
Herald added a project: All.
erichkeane requested review of this revision.

The standard says:

  The optional requires-clause ([temp.pre]) in an init-declarator or
  member-declarator shall be present only if the declarator declares a
  templated function ([dcl.fct]).

This implements that limitation, and updates the tests to the best of my
ability to capture the intent of the original checks.


https://reviews.llvm.org/D125711

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDecl.cpp
  clang/test/CXX/dcl.decl/dcl.decl.general/p4-20.cpp
  clang/test/CXX/dcl/dcl.decl/p3.cpp
  clang/test/CXX/expr/expr.prim/expr.prim.id/p4.cpp
  clang/test/CXX/over/over.match/over.match.best/p1-2a.cpp
  clang/test/CXX/over/over.match/over.match.viable/p3.cpp
  clang/test/CXX/over/over.over/p4-2a.cpp
  clang/test/Parser/cxx-concepts-requires-clause.cpp
  clang/test/Parser/cxx2a-concepts-requires-expr.cpp
  clang/test/SemaCXX/cxx20-check-fptr-constraints.cpp

Index: clang/test/SemaCXX/cxx20-check-fptr-constraints.cpp
===
--- clang/test/SemaCXX/cxx20-check-fptr-constraints.cpp
+++ clang/test/SemaCXX/cxx20-check-fptr-constraints.cpp
@@ -1,12 +1,15 @@
 // RUN: %clang_cc1 -std=c++20 -verify %s
 
 namespace P1972 {
-void f(int) requires false; // expected-note 4{{because 'false' evaluated to false}} \
-// expected-note{{constraints not satisfied}}
+template 
+struct S {
+  static void f(int)
+requires false; // expected-note 4{{because 'false' evaluated to false}}
+};
 void g() {
-  f(0);  // expected-error{{no matching function for call to 'f'}}
-  void (*p1)(int) = f;   // expected-error{{invalid reference to function 'f': constraints not satisfied}}
-  void (*p21)(int) =  // expected-error{{invalid reference to function 'f': constraints not satisfied}}
-  decltype(f) *p2 = nullptr; // expected-error{{invalid reference to function 'f': constraints not satisfied}}
+  S::f(0);  // expected-error{{invalid reference to function 'f': constraints not satisfied}}
+  void (*p1)(int) = S::f;   // expected-error{{invalid reference to function 'f': constraints not satisfied}}
+  void (*p21)(int) = ::f; // expected-error{{invalid reference to function 'f': constraints not satisfied}}
+  decltype(S::f) *p2 = nullptr; // expected-error{{invalid reference to function 'f': constraints not satisfied}}
 }
 }
Index: clang/test/Parser/cxx2a-concepts-requires-expr.cpp
===
--- clang/test/Parser/cxx2a-concepts-requires-expr.cpp
+++ clang/test/Parser/cxx2a-concepts-requires-expr.cpp
@@ -130,6 +130,7 @@
 
 bool r36 = requires (bool b) { requires sizeof(b) == 1; };
 
+template
 void r37(bool b) requires requires { 1 } {}
 // expected-error@-1 {{expected ';' at end of requirement}}
 
Index: clang/test/Parser/cxx-concepts-requires-clause.cpp
===
--- clang/test/Parser/cxx-concepts-requires-clause.cpp
+++ clang/test/Parser/cxx-concepts-requires-clause.cpp
@@ -139,8 +139,10 @@
 void bar() requires (sizeof(T)) == 0;
 // expected-error@-1{{parentheses are required around this expression in a requires clause}}
 
+template
 void bar(int x, int y) requires (x, y, true);
 
+  template
 struct B {
   int x;
   void foo(int y) requires (x, this, this->x, y, true);
Index: clang/test/CXX/over/over.over/p4-2a.cpp
===
--- clang/test/CXX/over/over.over/p4-2a.cpp
+++ clang/test/CXX/over/over.over/p4-2a.cpp
@@ -12,50 +12,48 @@
 template
 concept AtMost8 = sizeof(T) <= 8;
 
-int foo() requires AtLeast2 && AtMost8 {
+template
+struct S {
+static int foo() requires AtLeast2 && AtMost8 {
   return 0;
 }
 
-double foo() requires AtLeast2 {
+static double foo() requires AtLeast2 {
   return 0.0;
 }
 
-char bar() requires AtLeast2 { // expected-note {{possible target for call}}
+static char bar() requires AtLeast2 {
   return 1.0;
 }
 
-short bar() requires AtLeast2 && AtMost8 {
-// expected-note@-1{{possible target for call}}
-// expected-note@-2{{candidate function}}
+static short bar() requires AtLeast2 && AtMost8 {
   return 0.0;
 }
 
-int bar() requires AtMost8 && AtLeast2 {
-// expected-note@-1{{possible target for call}}
-// expected-note@-2{{candidate function}}
+static int bar() requires AtMost8 && AtLeast2 {
   return 0.0;
 }
 
-char baz() requires AtLeast2 {
+static char baz() requires AtLeast2 {
   return 1.0;
 }
 
-short baz() requires AtLeast2 && AtMost8 {
+static short baz() requires AtLeast2 && AtMost8 {
   return 0.0;
 }
 
-int baz() requires AtMost8 && AtLeast2 {
+static int baz() requires AtMost8 && AtLeast2 {
   return 0.0;
 }
 
-long baz() requires AtMost8 && AtLeast2 && AtLeast2 {
+static long baz() requires 

[clang] 0b69b83 - [Driver] Change "zlib not installed" diagnostic to "zlib not enabled"

[Fangrui Song via cfe-commits]

Author: Fangrui Song
Date: 2022-05-16T10:42:44-07:00
New Revision: 0b69b8384d9b92898ec29f78a2364bec2277e516

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

LOG: [Driver] Change "zlib not installed" diagnostic to "zlib not enabled"

The former is a bit misleading and a user may try installing zlib which
will not help.

Added: 


Modified: 
clang/include/clang/Basic/DiagnosticDriverKinds.td
clang/test/Driver/nozlibcompress.c

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td 
b/clang/include/clang/Basic/DiagnosticDriverKinds.td
index e58cfe6c4c56..7961b006a9a0 100644
--- a/clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -412,7 +412,7 @@ def warn_missing_sysroot : Warning<"no such sysroot 
directory: '%0'">,
   InGroup>;
 def warn_incompatible_sysroot : Warning<"using sysroot for '%0' but targeting 
'%1'">,
   InGroup>;
-def warn_debug_compression_unavailable : Warning<"cannot compress debug 
sections (zlib not installed)">,
+def warn_debug_compression_unavailable : Warning<"cannot compress debug 
sections (zlib not enabled)">,
   InGroup>;
 def warn_drv_disabling_vptr_no_rtti_default : Warning<
   "implicitly disabling vptr sanitizer because rtti wasn't enabled">,

diff  --git a/clang/test/Driver/nozlibcompress.c 
b/clang/test/Driver/nozlibcompress.c
index 9a704e70bc17..45935f595285 100644
--- a/clang/test/Driver/nozlibcompress.c
+++ b/clang/test/Driver/nozlibcompress.c
@@ -3,5 +3,5 @@
 // RUN: %clang -### -fintegrated-as -gz -c %s 2>&1 | FileCheck %s 
-check-prefix CHECK-WARN
 // RUN: %clang -### -fintegrated-as -gz=none -c %s 2>&1 | FileCheck 
-allow-empty -check-prefix CHECK-NOWARN %s
 
-// CHECK-WARN: warning: cannot compress debug sections (zlib not installed)
-// CHECK-NOWARN-NOT: warning: cannot compress debug sections (zlib not 
installed)
+// CHECK-WARN: warning: cannot compress debug sections (zlib not enabled)
+// CHECK-NOWARN-NOT: warning: cannot compress debug sections (zlib not enabled)



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

[PATCH] D125709: [analyzer][Casting] Support isa, cast, dyn_cast of SVals

[Aman LaChapelle via Phabricator via cfe-commits]

bzcheeseman added inline comments.



Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h:102
   template
   T castAs() const {
 assert(T::classof(*this));

Is it worth refactoring this and getAs to use `cast`  and `dyn_cast` as 
appropriate to avoid code duplication?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125709

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

[PATCH] D125709: [analyzer][Casting] Support isa, cast, dyn_cast of SVals

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

steakhal created this revision.
steakhal added reviewers: NoQ, martong, Szelethus, balazske, ASDenysPetrov, 
bzcheeseman.
Herald added subscribers: manas, dkrupp, donat.nagy, mikhail.ramalho, 
a.sidorin, rnkovacs, szepet, baloghadamsoftware, xazax.hun.
Herald added a project: All.
steakhal requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This change specializes the LLVM RTTI mechanism for SVals.
After this change, we can use the well-known `isa`, `cast`, `dyn_cast`.

Examples:

  // SVal V = ...;
  // Loc MyLoc = ...;
  
  bool IsInteresting = isa(MyLoc);
  auto MRV = cast(MyLoc);
  Optional MaybeMRV = dyn_cast(V)

The current `SVal::getAs` and `castAs` member functions are redundant at
this point, but I believe that they are still handy.

The member function version is terse and reads left-to-right, which IMO
is a great plus. However, we should probably add a variadic `isa` member
function version to have the same casting API in both cases.

I'm not sure, shall I add tests?

Thanks for the extensive TMP help @bzcheeseman!


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125709

Files:
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h


Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
===
--- clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
+++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
@@ -571,4 +571,28 @@
 } // namespace ento
 } // namespace clang
 
+namespace llvm {
+template 
+struct CastInfo<
+To, From,
+std::enable_if_t::value>>
+: public CastIsPossible {
+  using Self = CastInfo<
+  To, From,
+  std::enable_if_t::value>>;
+  static bool isPossible(const From ) {
+return To::classof(*static_cast());
+  }
+  static Optional castFailed() { return Optional{}; }
+  static Optional doCast(const From ) {
+return *static_cast(cast<::clang::ento::SVal>());
+  }
+  static Optional doCastIfPossible(const From ) {
+if (!Self::isPossible(f))
+  return Self::castFailed();
+return doCast(f);
+  }
+};
+} // namespace llvm
+
 #endif // LLVM_CLANG_STATICANALYZER_CORE_PATHSENSITIVE_SVALS_H


Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
===
--- clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
+++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
@@ -571,4 +571,28 @@
 } // namespace ento
 } // namespace clang
 
+namespace llvm {
+template 
+struct CastInfo<
+To, From,
+std::enable_if_t::value>>
+: public CastIsPossible {
+  using Self = CastInfo<
+  To, From,
+  std::enable_if_t::value>>;
+  static bool isPossible(const From ) {
+return To::classof(*static_cast());
+  }
+  static Optional castFailed() { return Optional{}; }
+  static Optional doCast(const From ) {
+return *static_cast(cast<::clang::ento::SVal>());
+  }
+  static Optional doCastIfPossible(const From ) {
+if (!Self::isPossible(f))
+  return Self::castFailed();
+return doCast(f);
+  }
+};
+} // namespace llvm
+
 #endif // LLVM_CLANG_STATICANALYZER_CORE_PATHSENSITIVE_SVALS_H
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D125708: [analyzer][NFC] Remove unused default SVal constructors

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

steakhal created this revision.
steakhal added reviewers: NoQ, martong, Szelethus, balazske, ASDenysPetrov, 
bzcheeseman.
Herald added subscribers: manas, dkrupp, donat.nagy, mikhail.ramalho, 
a.sidorin, rnkovacs, szepet, baloghadamsoftware, xazax.hun.
Herald added a project: All.
steakhal 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/D125708

Files:
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h

Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
===
--- clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
+++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
@@ -234,7 +234,6 @@
   static bool classof(SVal V) { return !V.isUndef(); }
 
 protected:
-  DefinedOrUnknownSVal() = default;
   explicit DefinedOrUnknownSVal(const void *d, bool isLoc, unsigned ValKind)
   : SVal(d, isLoc, ValKind) {}
   explicit DefinedOrUnknownSVal(BaseKind k, void *D = nullptr) : SVal(k, D) {}
@@ -258,15 +257,12 @@
   static bool classof(SVal V) { return !V.isUnknownOrUndef(); }
 
 protected:
-  DefinedSVal() = default;
   explicit DefinedSVal(const void *d, bool isLoc, unsigned ValKind)
   : DefinedOrUnknownSVal(d, isLoc, ValKind) {}
 };
 
 /// Represents an SVal that is guaranteed to not be UnknownVal.
 class KnownSVal : public SVal {
-  KnownSVal() = default;
-
 public:
   KnownSVal(const DefinedSVal ) : SVal(V) {}
   KnownSVal(const UndefinedVal ) : SVal(V) {}
@@ -275,7 +271,6 @@
 
 class NonLoc : public DefinedSVal {
 protected:
-  NonLoc() = default;
   explicit NonLoc(unsigned SubKind, const void *d)
   : DefinedSVal(d, false, SubKind) {}
 
@@ -292,7 +287,6 @@
 
 class Loc : public DefinedSVal {
 protected:
-  Loc() = default;
   explicit Loc(unsigned SubKind, const void *D)
   : DefinedSVal(const_cast(D), true, SubKind) {}
 
@@ -359,9 +353,6 @@
   }
 
   static bool classof(NonLoc V) { return V.getSubKind() == ConcreteIntKind; }
-
-private:
-  ConcreteInt() = default;
 };
 
 class LocAsInteger : public NonLoc {
@@ -401,9 +392,6 @@
   }
 
   static bool classof(NonLoc V) { return V.getSubKind() == LocAsIntegerKind; }
-
-private:
-  LocAsInteger() = default;
 };
 
 class CompoundVal : public NonLoc {
@@ -426,9 +414,6 @@
   }
 
   static bool classof(NonLoc V) { return V.getSubKind() == CompoundValKind; }
-
-private:
-  CompoundVal() = default;
 };
 
 class LazyCompoundVal : public NonLoc {
@@ -453,9 +438,6 @@
   static bool classof(NonLoc V) {
 return V.getSubKind() == LazyCompoundValKind;
   }
-
-private:
-  LazyCompoundVal() = default;
 };
 
 /// Value representing pointer-to-member.
@@ -503,7 +485,6 @@
   }
 
 private:
-  PointerToMember() = default;
   explicit PointerToMember(const PTMDataType D)
   : NonLoc(PointerToMemberKind, D.getOpaqueValue()) {}
 };
@@ -531,9 +512,6 @@
   }
 
   static bool classof(Loc V) { return V.getSubKind() == GotoLabelKind; }
-
-private:
-  GotoLabel() = default;
 };
 
 class MemRegionVal : public Loc {
@@ -568,9 +546,6 @@
   }
 
   static bool classof(Loc V) { return V.getSubKind() == MemRegionValKind; }
-
-private:
-  MemRegionVal() = default;
 };
 
 class ConcreteInt : public Loc {
@@ -590,9 +565,6 @@
   }
 
   static bool classof(Loc V) { return V.getSubKind() == ConcreteIntKind; }
-
-private:
-  ConcreteInt() = default;
 };
 
 } // namespace loc
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D125707: [analyzer][NFC] Remove unused friend SVal declarations

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

steakhal created this revision.
steakhal added reviewers: NoQ, martong, Szelethus, balazske, ASDenysPetrov, 
bzcheeseman.
Herald added subscribers: manas, dkrupp, donat.nagy, mikhail.ramalho, 
a.sidorin, rnkovacs, szepet, baloghadamsoftware, xazax.hun.
Herald added a project: All.
steakhal 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/D125707

Files:
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h

Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
===
--- clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
+++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
@@ -221,11 +221,7 @@
 class UndefinedVal : public SVal {
 public:
   UndefinedVal() : SVal(UndefinedValKind) {}
-
   static bool classof(SVal V) { return V.getBaseKind() == UndefinedValKind; }
-
-private:
-  friend class SVal;
 };
 
 class DefinedOrUnknownSVal : public SVal {
@@ -242,9 +238,6 @@
   explicit DefinedOrUnknownSVal(const void *d, bool isLoc, unsigned ValKind)
   : SVal(d, isLoc, ValKind) {}
   explicit DefinedOrUnknownSVal(BaseKind k, void *D = nullptr) : SVal(k, D) {}
-
-private:
-  friend class SVal;
 };
 
 class UnknownVal : public DefinedOrUnknownSVal {
@@ -252,9 +245,6 @@
   explicit UnknownVal() : DefinedOrUnknownSVal(UnknownValKind) {}
 
   static bool classof(SVal V) { return V.getBaseKind() == UnknownValKind; }
-
-private:
-  friend class SVal;
 };
 
 class DefinedSVal : public DefinedOrUnknownSVal {
@@ -271,14 +261,10 @@
   DefinedSVal() = default;
   explicit DefinedSVal(const void *d, bool isLoc, unsigned ValKind)
   : DefinedOrUnknownSVal(d, isLoc, ValKind) {}
-
-private:
-  friend class SVal;
 };
 
 /// Represents an SVal that is guaranteed to not be UnknownVal.
 class KnownSVal : public SVal {
-  friend class SVal;
   KnownSVal() = default;
 
 public:
@@ -302,9 +288,6 @@
   }
 
   static bool classof(SVal V) { return V.getBaseKind() == NonLocKind; }
-
-private:
-  friend class SVal;
 };
 
 class Loc : public DefinedSVal {
@@ -322,9 +305,6 @@
   }
 
   static bool classof(SVal V) { return V.getBaseKind() == LocKind; }
-
-private:
-  friend class SVal;
 };
 
 //====//
@@ -355,9 +335,6 @@
   }
 
   static bool classof(NonLoc V) { return V.getSubKind() == SymbolValKind; }
-
-private:
-  friend class SVal;
 };
 
 /// Value representing integer constant.
@@ -384,7 +361,6 @@
   static bool classof(NonLoc V) { return V.getSubKind() == ConcreteIntKind; }
 
 private:
-  friend class SVal;
   ConcreteInt() = default;
 };
 
@@ -427,7 +403,6 @@
   static bool classof(NonLoc V) { return V.getSubKind() == LocAsIntegerKind; }
 
 private:
-  friend class SVal;
   LocAsInteger() = default;
 };
 
@@ -453,7 +428,6 @@
   static bool classof(NonLoc V) { return V.getSubKind() == CompoundValKind; }
 
 private:
-  friend class SVal;
   CompoundVal() = default;
 };
 
@@ -481,7 +455,6 @@
   }
 
 private:
-  friend class SVal;
   LazyCompoundVal() = default;
 };
 
@@ -530,8 +503,6 @@
   }
 
 private:
-  friend class SVal;
-
   PointerToMember() = default;
   explicit PointerToMember(const PTMDataType D)
   : NonLoc(PointerToMemberKind, D.getOpaqueValue()) {}
@@ -562,7 +533,6 @@
   static bool classof(Loc V) { return V.getSubKind() == GotoLabelKind; }
 
 private:
-  friend class SVal;
   GotoLabel() = default;
 };
 
@@ -600,7 +570,6 @@
   static bool classof(Loc V) { return V.getSubKind() == MemRegionValKind; }
 
 private:
-  friend class SVal;
   MemRegionVal() = default;
 };
 
@@ -623,7 +592,6 @@
   static bool classof(Loc V) { return V.getSubKind() == ConcreteIntKind; }
 
 private:
-  friend class SVal;
   ConcreteInt() = default;
 };
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D125706: [analyzer][NFC] Use idiomatic classof instead of isKind

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

steakhal created this revision.
steakhal added reviewers: NoQ, martong, Szelethus, balazske, ASDenysPetrov, 
bzcheeseman.
Herald added subscribers: manas, dkrupp, donat.nagy, mikhail.ramalho, 
a.sidorin, rnkovacs, szepet, baloghadamsoftware, xazax.hun.
Herald added a project: All.
steakhal requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

- Rename `isKind()` to `classof()` to follow the llvm style RTTI.
- Take SVal by-value instead of reference.
- Mark `classof` public.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125706

Files:
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h

Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
===
--- clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
+++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
@@ -100,7 +100,7 @@
   /// the desired type.
   template
   T castAs() const {
-assert(T::isKind(*this));
+assert(T::classof(*this));
 return *static_cast(this);
   }
 
@@ -108,7 +108,7 @@
   /// not of the desired type.
   template
   Optional getAs() const {
-if (!T::isKind(*this))
+if (!T::classof(*this))
   return None;
 return *static_cast(this);
   }
@@ -124,13 +124,11 @@
 ID.AddPointer(Data);
   }
 
-  bool operator==(const SVal ) const {
+  bool operator==(SVal R) const {
 return getRawKind() == R.getRawKind() && Data == R.Data;
   }
 
-  bool operator!=(const SVal ) const {
-return !(*this == R);
-  }
+  bool operator!=(SVal R) const { return !(*this == R); }
 
   bool isUnknown() const {
 return getRawKind() == UnknownValKind;
@@ -224,12 +222,10 @@
 public:
   UndefinedVal() : SVal(UndefinedValKind) {}
 
+  static bool classof(SVal V) { return V.getBaseKind() == UndefinedValKind; }
+
 private:
   friend class SVal;
-
-  static bool isKind(const SVal& V) {
-return V.getBaseKind() == UndefinedValKind;
-  }
 };
 
 class DefinedOrUnknownSVal : public SVal {
@@ -239,6 +235,8 @@
   bool isUndef() const = delete;
   bool isValid() const = delete;
 
+  static bool classof(SVal V) { return !V.isUndef(); }
+
 protected:
   DefinedOrUnknownSVal() = default;
   explicit DefinedOrUnknownSVal(const void *d, bool isLoc, unsigned ValKind)
@@ -247,22 +245,16 @@
 
 private:
   friend class SVal;
-
-  static bool isKind(const SVal& V) {
-return !V.isUndef();
-  }
 };
 
 class UnknownVal : public DefinedOrUnknownSVal {
 public:
   explicit UnknownVal() : DefinedOrUnknownSVal(UnknownValKind) {}
 
+  static bool classof(SVal V) { return V.getBaseKind() == UnknownValKind; }
+
 private:
   friend class SVal;
-
-  static bool isKind(const SVal ) {
-return V.getBaseKind() == UnknownValKind;
-  }
 };
 
 class DefinedSVal : public DefinedOrUnknownSVal {
@@ -273,6 +265,8 @@
   bool isUnknownOrUndef() const = delete;
   bool isValid() const = delete;
 
+  static bool classof(SVal V) { return !V.isUnknownOrUndef(); }
+
 protected:
   DefinedSVal() = default;
   explicit DefinedSVal(const void *d, bool isLoc, unsigned ValKind)
@@ -280,25 +274,17 @@
 
 private:
   friend class SVal;
-
-  static bool isKind(const SVal& V) {
-return !V.isUnknownOrUndef();
-  }
 };
 
 /// Represents an SVal that is guaranteed to not be UnknownVal.
 class KnownSVal : public SVal {
   friend class SVal;
-
   KnownSVal() = default;
 
-  static bool isKind(const SVal ) {
-return !V.isUnknown();
-  }
-
 public:
   KnownSVal(const DefinedSVal ) : SVal(V) {}
   KnownSVal(const UndefinedVal ) : SVal(V) {}
+  static bool classof(SVal V) { return !V.isUnknown(); }
 };
 
 class NonLoc : public DefinedSVal {
@@ -315,12 +301,10 @@
T->isAnyComplexType() || T->isVectorType();
   }
 
+  static bool classof(SVal V) { return V.getBaseKind() == NonLocKind; }
+
 private:
   friend class SVal;
-
-  static bool isKind(const SVal& V) {
-return V.getBaseKind() == NonLocKind;
-  }
 };
 
 class Loc : public DefinedSVal {
@@ -337,12 +321,10 @@
T->isReferenceType() || T->isNullPtrType();
   }
 
+  static bool classof(SVal V) { return V.getBaseKind() == LocKind; }
+
 private:
   friend class SVal;
-
-  static bool isKind(const SVal& V) {
-return V.getBaseKind() == LocKind;
-  }
 };
 
 //====//
@@ -368,17 +350,14 @@
 return !isa(getSymbol());
   }
 
-private:
-  friend class SVal;
-
-  static bool isKind(const SVal& V) {
-return V.getBaseKind() == NonLocKind &&
-   V.getSubKind() == SymbolValKind;
+  static bool classof(SVal V) {
+return V.getBaseKind() == NonLocKind && V.getSubKind() == SymbolValKind;
   }
 
-  static bool isKind(const NonLoc& V) {
-return V.getSubKind() == SymbolValKind;
-  }
+  static bool classof(NonLoc V) { return V.getSubKind() == SymbolValKind; }
+
+private:
+  friend class SVal;
 };
 
 /// Value representing integer constant.
@@ 

[clang] babbd96 - [docs] Re-generate ClangCommandLineReference.rst

[Fangrui Song via cfe-commits]

Author: Fangrui Song
Date: 2022-05-16T10:34:01-07:00
New Revision: babbd96f23d5e21712223ece8f6318e6bc93269b

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

LOG: [docs] Re-generate ClangCommandLineReference.rst

Added: 


Modified: 
clang/docs/ClangCommandLineReference.rst

Removed: 




diff  --git a/clang/docs/ClangCommandLineReference.rst 
b/clang/docs/ClangCommandLineReference.rst
index d8fa4b33baf0..13e175a4e520 100644
--- a/clang/docs/ClangCommandLineReference.rst
+++ b/clang/docs/ClangCommandLineReference.rst
@@ -76,16 +76,6 @@ Pass  to fatbinary invocation
 
 Pass  to the ptxas assembler
 
-.. option:: -Xopenmp-target 
-
-Pass  to the target offloading toolchain.
-
-.. program:: clang1
-.. option:: -Xopenmp-target= 
-.. program:: clang
-
-Pass  to the target offloading toolchain identified by .
-
 .. option:: -Z
 
 .. option:: -a, --profile-blocks
@@ -142,17 +132,9 @@ Specifies configuration file
 
 .. option:: --constant-cfstrings
 
-.. option:: --cuda-compile-host-device
-
-Compile CUDA code for both host and device (default).  Has no effect on 
non-CUDA compilations.
-
-.. option:: --cuda-device-only
-
-Compile CUDA code for device only
+.. option:: --cuda-feature=
 
-.. option:: --cuda-host-only
-
-Compile CUDA code for host only.  Has no effect on non-CUDA compilations.
+Manually specify the CUDA feature to use
 
 .. option:: --cuda-include-ptx=, --no-cuda-include-ptx=
 
@@ -168,6 +150,14 @@ An ID for compilation unit, which should be the same for 
the same compilation un
 
 .. option:: -current\_version
 
+.. option:: -darwin-target-variant 
+
+Generate code for an additional runtime variant of the deployment target
+
+.. option:: -darwin-target-variant-triple 
+
+Specify the darwin target variant triple
+
 .. option:: -dead\_strip
 
 .. option:: -dependency-dot 
@@ -226,6 +216,10 @@ Start emitting warnings for unused driver arguments
 
 Reserve register r19 (Hexagon only)
 
+.. option:: -fgpu-default-stream=
+
+Specify default stream. The default value is 'legacy'. (HIP only).  must 
be 'legacy' or 'per-thread'.
+
 .. option:: -fgpu-flush-denormals-to-zero, -fcuda-flush-denormals-to-zero, 
-fno-gpu-flush-denormals-to-zero
 
 Flush denormal floating point values to zero in CUDA/HIP device mode.
@@ -238,22 +232,6 @@ Flush denormal floating point values to zero in CUDA/HIP 
device mode.
 
 Specify comma-separated list of triples OpenMP offloading targets to be 
supported
 
-.. option:: -fopenmp-new-driver, -fno-openmp-new-driver
-
-Use the new driver for OpenMP offloading.
-
-.. option:: --offload-new-driver, --no-offload-new-driver
-
-Use the new driver for offloading compilation.
-
-.. option:: --offload-host-only
-
-Only compile for the host when offloading.
-
-.. option:: --offload-device-only
-
-Only compile for the device when offloading.
-
 .. option:: -force\_cpusubtype\_ALL
 
 .. program:: clang1
@@ -313,7 +291,7 @@ Default max threads per block for kernel launch bounds for 
HIP
 
 .. option:: -headerpad\_max\_install\_names
 
-.. option:: -help, --help
+.. option:: -help, --help, /help, -help, --help
 
 Display available options
 
@@ -355,10 +333,6 @@ Make the next included directory (-I or -F) an indexer 
header map
 
 .. option:: -mbig-endian, -EB
 
-.. option:: -mbranch-protection=
-
-Enforce targets of indirect branches and function returns
-
 .. option:: -menable-unsafe-fp-math
 
 Allow unsafe floating-point math optimizations which may decrease precision
@@ -381,6 +355,10 @@ Run the migrator
 
 Additional arguments to forward to LLVM's option processing
 
+.. option:: -mmlir 
+
+Additional arguments to forward to MLIR's option processing
+
 .. option:: -module-dependency-dir 
 
 Directory to dump module dependencies to
@@ -401,6 +379,10 @@ Directory to dump module dependencies to
 
 Don't error out if the detected version of the CUDA install is too low for the 
requested CUDA gpu architecture.
 
+.. option:: -no-hip-rt
+
+Do not link against HIP runtime libraries
+
 .. option:: -no-integrated-cpp, --no-integrated-cpp
 
 .. option:: -no\_dead\_strip\_inits\_and\_terms
@@ -411,6 +393,8 @@ Disable builtin #include directories
 
 .. option:: -nodefaultlibs
 
+.. option:: -nodriverkitlib
+
 .. option:: -nofixprebinding
 
 .. option:: -nogpuinc, -nocudainc
@@ -449,7 +433,9 @@ Disable standard #include directories for the C++ standard 
library
 
 .. option:: -nostdlibinc
 
-.. option:: -o, --output , --output=
+.. program:: clang1
+.. option:: -o, /Fo, -Fo, --output , --output=
+.. program:: clang
 
 Write output to 
 
@@ -527,6 +513,18 @@ Set the output  for debug infos
 
 CUDA offloading device architecture (e.g. sm\_35), or HIP offloading target ID 
in the form of a device architecture followed by target ID features delimited 
by a colon. Each target 

[PATCH] D125487: [Tooling/DependencyScanning] Refactor dependency scanning to produce pre-lexed preprocessor directive tokens, instead of minimized sources

[Argyrios Kyrtzidis via Phabricator via cfe-commits]

akyrtzi updated this revision to Diff 429766.
akyrtzi added a comment.

Make sure to enable line comments for dependency directive lexing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125487

Files:
  clang/include/clang/Lex/DependencyDirectivesScanner.h
  clang/include/clang/Lex/Lexer.h
  clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
  clang/lib/Frontend/FrontendActions.cpp
  clang/lib/Lex/DependencyDirectivesScanner.cpp
  clang/lib/Lex/Lexer.cpp
  clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
  clang/test/Lexer/minimize_source_to_dependency_directives_invalid_macro_name.c
  clang/test/Lexer/minimize_source_to_dependency_directives_pragmas.c
  clang/unittests/Lex/DependencyDirectivesScannerTest.cpp
  clang/unittests/Tooling/DependencyScannerTest.cpp

Index: clang/unittests/Tooling/DependencyScannerTest.cpp
===
--- clang/unittests/Tooling/DependencyScannerTest.cpp
+++ clang/unittests/Tooling/DependencyScannerTest.cpp
@@ -204,51 +204,5 @@
   EXPECT_EQ(convert_to_slash(Deps[5]), "/root/symlink.h");
 }
 
-namespace dependencies {
-TEST(DependencyScanningFilesystem, IgnoredFilesAreCachedSeparately1) {
-  auto VFS = llvm::makeIntrusiveRefCnt();
-  VFS->addFile("/mod.h", 0,
-   llvm::MemoryBuffer::getMemBuffer("#include \n"
-"// hi there!\n"));
-
-  DependencyScanningFilesystemSharedCache SharedCache;
-  DependencyScanningWorkerFilesystem DepFS(SharedCache, VFS);
-
-  DepFS.enableDirectivesScanningOfAllFiles(); // Let's be explicit for clarity.
-  auto StatusMinimized0 = DepFS.status("/mod.h");
-  DepFS.disableDirectivesScanning("/mod.h");
-  auto StatusFull1 = DepFS.status("/mod.h");
-
-  EXPECT_TRUE(StatusMinimized0);
-  EXPECT_TRUE(StatusFull1);
-  EXPECT_EQ(StatusMinimized0->getSize(), 17u);
-  EXPECT_EQ(StatusFull1->getSize(), 30u);
-  EXPECT_EQ(StatusMinimized0->getName(), StringRef("/mod.h"));
-  EXPECT_EQ(StatusFull1->getName(), StringRef("/mod.h"));
-}
-
-TEST(DependencyScanningFilesystem, IgnoredFilesAreCachedSeparately2) {
-  auto VFS = llvm::makeIntrusiveRefCnt();
-  VFS->addFile("/mod.h", 0,
-   llvm::MemoryBuffer::getMemBuffer("#include \n"
-"// hi there!\n"));
-
-  DependencyScanningFilesystemSharedCache SharedCache;
-  DependencyScanningWorkerFilesystem DepFS(SharedCache, VFS);
-
-  DepFS.disableDirectivesScanning("/mod.h");
-  auto StatusFull0 = DepFS.status("/mod.h");
-  DepFS.enableDirectivesScanningOfAllFiles();
-  auto StatusMinimized1 = DepFS.status("/mod.h");
-
-  EXPECT_TRUE(StatusFull0);
-  EXPECT_TRUE(StatusMinimized1);
-  EXPECT_EQ(StatusFull0->getSize(), 30u);
-  EXPECT_EQ(StatusMinimized1->getSize(), 17u);
-  EXPECT_EQ(StatusFull0->getName(), StringRef("/mod.h"));
-  EXPECT_EQ(StatusMinimized1->getName(), StringRef("/mod.h"));
-}
-
-} // end namespace dependencies
 } // end namespace tooling
 } // end namespace clang
Index: clang/unittests/Lex/DependencyDirectivesScannerTest.cpp
===
--- clang/unittests/Lex/DependencyDirectivesScannerTest.cpp
+++ clang/unittests/Lex/DependencyDirectivesScannerTest.cpp
@@ -14,39 +14,58 @@
 using namespace clang;
 using namespace clang::dependency_directives_scan;
 
-static bool minimizeSourceToDependencyDirectives(StringRef Input,
- SmallVectorImpl ) {
-  SmallVector Directives;
-  return scanSourceForDependencyDirectives(Input, Out, Directives);
+static bool minimizeSourceToDependencyDirectives(
+StringRef Input, SmallVectorImpl ,
+SmallVectorImpl ,
+SmallVectorImpl ) {
+  Out.clear();
+  Tokens.clear();
+  Directives.clear();
+  if (scanSourceForDependencyDirectives(Input, Tokens, Directives))
+return true;
+
+  raw_svector_ostream OS(Out);
+  printDependencyDirectivesAsSource(Input, Directives, OS);
+  if (!Out.empty() && Out.back() != '\n')
+Out.push_back('\n');
+  Out.push_back('\0');
+  Out.pop_back();
+
+  return false;
 }
 
-static bool
-minimizeSourceToDependencyDirectives(StringRef Input,
- SmallVectorImpl ,
- SmallVectorImpl ) {
-  return scanSourceForDependencyDirectives(Input, Out, Directives);
+static bool minimizeSourceToDependencyDirectives(StringRef Input,
+ SmallVectorImpl ) {
+  SmallVector Tokens;
+  SmallVector Directives;
+  return minimizeSourceToDependencyDirectives(Input, Out, Tokens, Directives);
 }
 
 namespace {
 
 TEST(MinimizeSourceToDependencyDirectivesTest, Empty) {
   SmallVector Out;
+  SmallVector Tokens;
   SmallVector Directives;
 
-  ASSERT_FALSE(minimizeSourceToDependencyDirectives("", Out, Directives));
+  ASSERT_FALSE(
+  

[PATCH] D125622: [clang-tidy] Reject invalid enum initializers in C files

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/modernize-macro-to-enum.c:9
+
+// C forbids comma operator in initializing expressions.
+#define BAD_OP 1, 2

LegalizeAdulthood wrote:
> aaron.ballman wrote:
> > It's also forbidden in C++.
> C++ just talks about a constant expression and doesn't explicitly disallow 
> `operator,` in such expressions that I could find.  Can you point me to where 
> it is disallowed?
> 
> If it is disallowed universally, then I don't see why you asked me to parse 
> it `:)`
> C++ just talks about a constant expression and doesn't explicitly disallow 
> operator, in such expressions that I could find. Can you point me to where it 
> is disallowed?

It falls out from the grammar:

http://eel.is/c++draft/enum#nt:enumerator-definition
http://eel.is/c++draft/expr.const#nt:constant-expression
http://eel.is/c++draft/expr.cond#nt:conditional-expression

> If it is disallowed universally, then I don't see why you asked me to parse 
> it :)

It can show up in paren expressions and the interface is generally about 
integer literal expressions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125622

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

[PATCH] D125705: [OpenMP] Don't build the offloading driver without a source input

[Johannes Doerfert via Phabricator via cfe-commits]

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

LG


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125705

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

[PATCH] D125705: [OpenMP] Don't build the offloading driver without a source input

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 created this revision.
jhuber6 added reviewers: jdoerfert, tianshilei1992, wsmoses.
Herald added subscribers: guansong, yaxunl.
Herald added a project: All.
jhuber6 requested review of this revision.
Herald added subscribers: cfe-commits, sstefan1, MaskRay.
Herald added a project: clang.

The Clang driver additional stages to build a complete offloading
program for applications using CUDA or OpenMP offloading. This normally
requires either a source file input or a valid object file to be
handled. This would cause problems when trying to compile an assembly or
LLVM IR file through clang with flags that would enable offloading. This
patch simply adds a check to prevent the offloading toolchain from being
used if we don't have a valid source file.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125705

Files:
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/openmp-offload-gpu-new.c


Index: clang/test/Driver/openmp-offload-gpu-new.c
===
--- clang/test/Driver/openmp-offload-gpu-new.c
+++ clang/test/Driver/openmp-offload-gpu-new.c
@@ -70,6 +70,11 @@
 // CHECK-NVIDIA-AMDGPU: "x86_64-unknown-linux-gnu" - "clang", inputs: 
["[[HOST_BC]]", "[[BINARY]]"], output: "[[HOST_OBJ:.+]]"
 // CHECK-NVIDIA-AMDGPU: "x86_64-unknown-linux-gnu" - "Offload::Linker", 
inputs: ["[[HOST_OBJ]]"], output: "a.out"
 
+// RUN:   %clang -x ir -### --target=x86_64-unknown-linux-gnu 
-ccc-print-bindings -fopenmp --offload-arch=sm_52 -nogpulib %s 2>&1 | FileCheck 
%s --check-prefix=CHECK-IR
+
+// CHECK-IR: "x86_64-unknown-linux-gnu" - "clang", inputs: 
["[[INPUT_IR:.+]]"], output: "[[OBJECT:.+]]" 
+// CHECK-IR: "x86_64-unknown-linux-gnu" - "Offload::Linker", inputs: 
["[[OBJECT]]"], output: "a.out"
+
 // RUN:   %clang -### --target=x86_64-unknown-linux-gnu -emit-llvm -S -fopenmp 
-fopenmp-targets=nvptx64-nvidia-cuda -Xopenmp-target=nvptx64-nvidia-cuda 
-march=sm_52 -nogpulib %s 2>&1 | FileCheck %s --check-prefix=CHECK-EMIT-LLVM-IR
 // CHECK-EMIT-LLVM-IR: "-cc1"{{.*}}"-triple" 
"nvptx64-nvidia-cuda"{{.*}}"-emit-llvm"
 
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -4377,9 +4377,11 @@
   Mode && Mode->getOption().matches(options::OPT_offload_device_only);
 
   // Don't build offloading actions if explicitly disabled or we do not have a
-  // compile action to embed it in. If preprocessing only ignore embedding.
-  if (HostOnly || !(isa(HostAction) ||
-getFinalPhase(Args) == phases::Preprocess))
+  // valid source input and compile action to embed it in. If preprocessing 
only
+  // ignore embedding.
+  if (HostOnly || !types::isSrcFile(Input.first) ||
+  !(isa(HostAction) ||
+getFinalPhase(Args) == phases::Preprocess))
 return HostAction;
 
   ActionList OffloadActions;


Index: clang/test/Driver/openmp-offload-gpu-new.c
===
--- clang/test/Driver/openmp-offload-gpu-new.c
+++ clang/test/Driver/openmp-offload-gpu-new.c
@@ -70,6 +70,11 @@
 // CHECK-NVIDIA-AMDGPU: "x86_64-unknown-linux-gnu" - "clang", inputs: ["[[HOST_BC]]", "[[BINARY]]"], output: "[[HOST_OBJ:.+]]"
 // CHECK-NVIDIA-AMDGPU: "x86_64-unknown-linux-gnu" - "Offload::Linker", inputs: ["[[HOST_OBJ]]"], output: "a.out"
 
+// RUN:   %clang -x ir -### --target=x86_64-unknown-linux-gnu -ccc-print-bindings -fopenmp --offload-arch=sm_52 -nogpulib %s 2>&1 | FileCheck %s --check-prefix=CHECK-IR
+
+// CHECK-IR: "x86_64-unknown-linux-gnu" - "clang", inputs: ["[[INPUT_IR:.+]]"], output: "[[OBJECT:.+]]" 
+// CHECK-IR: "x86_64-unknown-linux-gnu" - "Offload::Linker", inputs: ["[[OBJECT]]"], output: "a.out"
+
 // RUN:   %clang -### --target=x86_64-unknown-linux-gnu -emit-llvm -S -fopenmp -fopenmp-targets=nvptx64-nvidia-cuda -Xopenmp-target=nvptx64-nvidia-cuda -march=sm_52 -nogpulib %s 2>&1 | FileCheck %s --check-prefix=CHECK-EMIT-LLVM-IR
 // CHECK-EMIT-LLVM-IR: "-cc1"{{.*}}"-triple" "nvptx64-nvidia-cuda"{{.*}}"-emit-llvm"
 
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -4377,9 +4377,11 @@
   Mode && Mode->getOption().matches(options::OPT_offload_device_only);
 
   // Don't build offloading actions if explicitly disabled or we do not have a
-  // compile action to embed it in. If preprocessing only ignore embedding.
-  if (HostOnly || !(isa(HostAction) ||
-getFinalPhase(Args) == phases::Preprocess))
+  // valid source input and compile action to embed it in. If preprocessing only
+  // ignore embedding.
+  if (HostOnly || !types::isSrcFile(Input.first) ||
+  !(isa(HostAction) ||
+getFinalPhase(Args) == phases::Preprocess))
 return HostAction;
 
   ActionList OffloadActions;
___

[PATCH] D124726: Suggest typoed directives in preprocessor conditionals

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D124726#3516346 , @nathanchance 
wrote:

> I see an instance of this warning in the Linux kernel due to the "Now, for 
> unknown directives inside a skipped conditional block, we diagnose the 
> unknown directive as a warning if it is sufficiently similar to a directive 
> specific to preprocessor conditional blocks" part of this change:
>
>   arch/x86/crypto/aesni-intel_asm.S:532:8: warning: invalid preprocessing 
> directive, did you mean '#if'? [-Wunknown-directives]
>   # in in order to perform
> ^
>   arch/x86/crypto/aesni-intel_asm.S:547:8: warning: invalid preprocessing 
> directive, did you mean '#if'? [-Wunknown-directives]
>   # in in order to perform
> ^
>   2 warnings generated.

Oh wow, that's a really neat one!

> This is a comment within an assembler file that will be preprocessed (this is 
> a 32-bit x86 build and the block is guarded by `#ifdef __x86_64__` on line 
> 500):
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/x86/crypto/aesni-intel_asm.S?h=v5.18-rc7#n532
>
> Is there anything that can be done to improve this heuristic for this case? I 
> can try to push a patch to change the comment style for this one instance but 
> I always worry that a warning of this nature will appear in the future and 
> result in the kernel disabling this warning entirely.

Ah, and we don't get an error for those because of special logic: 
https://github.com/llvm/llvm-project/blob/main/clang/lib/Lex/PPDirectives.cpp#L1243
 and it looks like we may need similar logic before issuing the warnings as 
well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124726

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

[PATCH] D125555: [clang] Add __has_target_feature

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

Oops. forgot to hit 'submit' last week, so there's some overlap with Aaron's 
question.




Comment at: clang/docs/LanguageExtensions.rst:275
+  // On amdgcn target
+  #if __has_target_feature("s-memtime-inst")
+x = __builtin_amdgcn_s_memtime();

yaxunl wrote:
> tra wrote:
> > Do you have a better example? This particular case could've been handled by 
> > existing `__has_builtin()`.
> > 
> > While I could see usefulness of checking features (e.g. for CUDA/NVPTX it 
> > could be used to chose inline assembly supported only by newer PTX 
> > versions, but even then that choice could be made using existing methods, 
> > even if they are not always direct (e.g. by using CUDA_VERSION as a proxy 
> > for "new enough PTX version").
> > 
> `__has_builtin` returns 1 as long as the builtin is known to clang, even if 
> the builtin is not supported by the target CPU. This is because the required 
> target feature for a builtin is in ASTContext, whereas `__has_builtin` is 
> evaluated in preprocessor, where the information is not known.
I'm missing something.  `__has_target_feature("s-memtime-inst")` is also 
evaluated by preprocessor, right next to where `__has_target_builtin` is 
processed.
I guess what you're saying is that preprocessor does not pay attention to the 
target feature conditions attached to those builtins.
Those are evaluted in `CodeGenFunction::checkTargetFeatures`.

This means that in order to use `__has_feature()` to detect availability of 
target builtins would effectively require the user to specify exactly the same 
set of conditions, as applied to the builtin. That will work for builtins that 
map to features 1:1, but it will be prone to breaking for NVPTX builtins that 
carry fairly large set of feature requirements and that set changes every time 
we add a new PTX or GPU variant, which happens fairly often.

I wonder if it may be better to generalize target builtin feature evaluation 
and use it from both preprocessor and the codegen and just stick with 
`__has_builtin`, only now working correctly to indicate availability of the 
target builtins.







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

https://reviews.llvm.org/D12

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

[PATCH] D125622: [clang-tidy] Reject invalid enum initializers in C files

[Richard via Phabricator via cfe-commits]

LegalizeAdulthood added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/modernize-macro-to-enum.c:9
+
+// C forbids comma operator in initializing expressions.
+#define BAD_OP 1, 2

aaron.ballman wrote:
> It's also forbidden in C++.
C++ just talks about a constant expression and doesn't explicitly disallow 
`operator,` in such expressions that I could find.  Can you point me to where 
it is disallowed?

If it is disallowed universally, then I don't see why you asked me to parse it 
`:)`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125622

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

[PATCH] D124726: Suggest typoed directives in preprocessor conditionals

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added subscribers: nickdesaulniers, nathanchance.
nathanchance added a comment.

I see an instance of this warning in the Linux kernel due to the "Now, for 
unknown directives inside a skipped conditional block, we diagnose the unknown 
directive as a warning if it is sufficiently similar to a directive specific to 
preprocessor conditional blocks" part of this change:

  arch/x86/crypto/aesni-intel_asm.S:532:8: warning: invalid preprocessing 
directive, did you mean '#if'? [-Wunknown-directives]
  # in in order to perform
^
  arch/x86/crypto/aesni-intel_asm.S:547:8: warning: invalid preprocessing 
directive, did you mean '#if'? [-Wunknown-directives]
  # in in order to perform
^
  2 warnings generated.

This is a comment within an assembler file that will be preprocessed (this is a 
32-bit x86 build and the block is guarded by `#ifdef __x86_64__` on line 500):

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/x86/crypto/aesni-intel_asm.S?h=v5.18-rc7#n532

Is there anything that can be done to improve this heuristic for this case? I 
can try to push a patch to change the comment style for this one instance but I 
always worry that a warning of this nature will appear in the future and result 
in the kernel disabling this warning entirely.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124726

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

[PATCH] D124613: In MSVC compatibility mode, friend function declarations behave as function declarations

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

I haven't given any input yet because I haven't yet seen a reduced example of 
the conforming code that is broken by this change. If someone can put together 
a small godbolt example that shows the issue affecting ADL, I'd appreciate it.

I don't fully understand the code patterns, but I think what I find most 
compelling so far is that we want clang users to **be able** to write 
conforming code, while accepting as much code that MSVC accepts with possible, 
ideally with warnings guiding folks to make code conform. Every goal can be 
compromised, there are no absolutes, so in the end, it's all tradeoffs.

Initially this patch appealed to me because it was a small change to accept 
what seemed like a small edge case of friend function definitions. It didn't 
seem worth coding up new codepaths for diagnostics. However, on reflection, the 
friend function definition is one of the gnarliest corners of the C++ language 
because of the mismatch between the lexical and semantic scopes. More care is 
probably required.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124613

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

[PATCH] D124918: [clang-tidy] Add a new check for non-trivial unused variables.

[Luca Versari via Phabricator via cfe-commits]

veluca93 added inline comments.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/performance-unused-no-side-effect.rst:93
+   {
+  "std::string": ["swap"],
+  "absl::int128": [],

LegalizeAdulthood wrote:
> Since `std::string` is just a type alias, shouldn't we be considering 
> `basic_string`?
> What about `wstring`?
Looks like std::string is indeed unnecessary.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/performance-unused-no-side-effect.rst:172-173
+List of functions that are considered not to read some of their arguments
+unless their return value is read. Arguments are identified by a bitmask, where
+the i-th LSB being set indicates that the i-th argument is unused.
+

LegalizeAdulthood wrote:
> This isn't user-friendly at all.  Why not an array of argument indices 
> starting at zero?
Done. I'm using an empty array to indicate "everything", which is perhaps a bit 
weird.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124918

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

[PATCH] D124918: [clang-tidy] Add a new check for non-trivial unused variables.

[Luca Versari via Phabricator via cfe-commits]

veluca93 updated this revision to Diff 429745.
veluca93 marked 6 inline comments as done.
veluca93 added a comment.

Switch from bitmasks to arrays. Add more tests & update doc.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124918

Files:
  clang-tools-extra/clang-tidy/performance/CMakeLists.txt
  clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp
  clang-tools-extra/clang-tidy/performance/UnusedNoSideEffectCheck.cpp
  clang-tools-extra/clang-tidy/performance/UnusedNoSideEffectCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/performance-unused-no-side-effect.rst
  
clang-tools-extra/test/clang-tidy/checkers/performance-unused-no-side-effect.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/performance-unused-no-side-effect.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/performance-unused-no-side-effect.cpp
@@ -0,0 +1,144 @@
+// RUN: %check_clang_tidy %s performance-unused-no-side-effect %t -- \
+// RUN:  -config="{CheckOptions: [{key: \"performance-unused-no-side-effect.TypesByBase\", value: '{\"Marker\":[\"swap\"]}'}]}"
+
+namespace std {
+
+template 
+struct char_traits {};
+
+template 
+struct allocator {};
+
+template <
+class CharT,
+class Traits = std::char_traits,
+class Allocator = std::allocator>
+class basic_string {
+public:
+  basic_string +=(const char *);
+  ~basic_string();
+};
+
+using string = basic_string;
+
+class thread {
+public:
+  ~thread();
+};
+
+template >
+class vector {
+public:
+  void push_back(T);
+};
+
+template 
+class unique_ptr {
+public:
+  unique_ptr(T *);
+};
+
+} // namespace std
+
+struct Marker {
+};
+
+struct SideEffectFree : Marker {
+  SideEffectFree();
+  ~SideEffectFree();
+  void swap(SideEffectFree *);
+};
+
+struct POD {
+  int A;
+};
+
+template 
+struct CustomPair {
+  T t;
+  U u;
+};
+
+void f(const std::string &);
+
+void unusedString() {
+  std::string SomeString;
+  // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: variable 'SomeString' is never read [performance-unused-no-side-effect]
+}
+
+void unusedStringWithMethodCall() {
+  std::string SomeString;
+  // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: variable 'SomeString' is never read [performance-unused-no-side-effect]
+  SomeString += "hi";
+}
+
+void unusedStdThread() {
+  std::thread T;
+}
+
+void usedString() {
+  std::string SomeString;
+  SomeString += "hi";
+  f(SomeString);
+}
+
+void vectorString() {
+  std::vector VecString;
+  // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: variable 'VecString' is never read [performance-unused-no-side-effect]
+  VecString.push_back(std::string{});
+}
+
+void vectorThread() {
+  std::vector VecThread;
+  VecThread.push_back(std::thread{});
+}
+
+void withMarker() {
+  SideEffectFree M;
+  // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: variable 'M' is never read [performance-unused-no-side-effect]
+}
+
+void withMarkerAndExcludedMethod(SideEffectFree *S) {
+  SideEffectFree M;
+  M.swap(S);
+}
+
+void withPOD() {
+  POD P;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: variable 'P' is never read [performance-unused-no-side-effect]
+  P.A = 1;
+}
+
+int withPODReturn() {
+  POD P;
+  P.A = 1;
+  return P.A;
+}
+
+void withLoops() {
+  std::vector VecLoops;
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: variable 'VecLoops' is never read [performance-unused-no-side-effect]
+  for (int i = 0; i < 15; i++) {
+VecLoops.push_back(1);
+  }
+  int j = 0;
+  while (j < 1) {
+VecLoops.push_back(2);
+j++;
+  }
+  (VecLoops).push_back(3);
+}
+
+void withSmartPointer(int *Arg) {
+  std::unique_ptr Unused(new int);
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: variable 'Unused' is never read [performance-unused-no-side-effect]
+
+  std::unique_ptr Used(Arg);
+}
+
+void withCustomPair() {
+  CustomPair> Unused{};
+  // CHECK-MESSAGES: :[[@LINE-1]]:45: warning: variable 'Unused' is never read [performance-unused-no-side-effect]
+
+  CustomPair Used;
+}
Index: clang-tools-extra/docs/clang-tidy/checks/performance-unused-no-side-effect.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/performance-unused-no-side-effect.rst
@@ -0,0 +1,193 @@
+.. title:: clang-tidy - performance-unused-no-side-effect
+
+performance-unused-no-side-effect
+=
+
+Finds variables that are not used and do not have user-visible side effects.
+
+A variable is defined to be side-effect-free and unused if:
+
+- It has a side-effect-free type (defined below).
+- It is not passed to a function that reads its content and whose result is used.
+
+A type is side-effect-free if it is any of the following:
+
+- A type with a base class specified in `TypesByBase`.
+- A type listed in `RawTypes`.
+- 

[PATCH] D123534: [dwarf] Emit a DIGlobalVariable for constant strings.

[Reid Kleckner via Phabricator via cfe-commits]

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

Looks good to me. I generally agree with Adrian's suggestions.




Comment at: llvm/test/DebugInfo/COFF/global-no-strings.ll:1
+; RUN: llc < %s | FileCheck %s
+; RUN: llc < %s -filetype=obj | llvm-readobj - --codeview | FileCheck %s

test lgtm, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123534

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

[PATCH] D125400: [clang][Analyzer] Add errno state to standard functions modeling.

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

balazske added a comment.

@martong Do you mean that a "describe" function is needed for the return value 
constraint of the function and for the errno constraint type? Then a note tag 
can contain what the function is assumed to return on success and what is 
allowed (or not) to do with `errno`. For example: "Assuming that 'mkdir' is 
successful, it returns zero in this case and value of 'errno' is unspecified 
after the call".




Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:435
+  assert(Cond);
+  State = State->assume(*Cond, true);
+  return errno_modeling::setErrnoValue(State, C.getLocationContext(),

martong wrote:
> Please check if `State` can be nullptr.
I think here it is never null. A relation is created between a new conjured 
symbol and zero, this can never fail, or not? (The `ErrnoSVal` should be here a 
newly created symbol. If the `Tag` is not used it may be the same symbol that 
was previously bound to the expression if `EvalCallAsPure` is used.)



Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:603
 
-Summary (ConstraintSet &, StringRef Note = "") {
-  Cases.push_back(SummaryCase(std::move(CS), Note));
+Summary (ConstraintSet &, const ErrnoConstraintKind ,
+  StringRef Note = "") {

martong wrote:
> Would it make sense to have a `ErrnoIrrelevant` as the default value for 
> `ErrnoC`?
It would be a bit more convenient but in the current design it is not possible 
to pass the member variable as default value. `ErrnoIrrelevant` is really used 
in less than half of the cases.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125400

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

[PATCH] D113743: [RFC][clang][DebugInfo] Allow function-local statics and types to be scoped within a lexical block

[Kristina Bessonova via Phabricator via cfe-commits]

krisb abandoned this revision.
krisb added a comment.
Herald added a project: All.

Split on two: D125694  and D125695 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113743

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

[PATCH] D125695: [clang][DebugInfo] Allow function-local type-like entities to be scoped within a lexical block (5/5)

[Kristina Bessonova via Phabricator via cfe-commits]

krisb created this revision.
krisb added reviewers: dblaikie, aprantl, probinson.
Herald added a project: All.
krisb requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This is a part from D113743  split to make it 
easier to test and review.

It also enables lexical blocks as a scope for types and type-like entities
only if -debugger-tunning=gdb. lldb can't (yet) handle lexical block scoped
types (see D115277  for details), other 
debuggers were not verified (yet).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125695

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGDebugInfo.h
  clang/lib/CodeGen/CGDecl.cpp
  clang/test/CodeGenCXX/debug-info-local-types.cpp

Index: clang/test/CodeGenCXX/debug-info-local-types.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/debug-info-local-types.cpp
@@ -0,0 +1,101 @@
+// RUN: %clang_cc1 -triple x86_64-none-linux-gnu -emit-llvm -debug-info-kind=limited -debugger-tuning=gdb %s -o - | FileCheck %s --check-prefixes=CHECK,LIMITED
+// RUN: %clang_cc1 -triple x86_64-none-linux-gnu -emit-llvm -debug-info-kind=unused-types -debugger-tuning=gdb %s -o - | FileCheck %s --check-prefixes=UNUSED_TYPES
+
+void test() {
+  {
+struct S { int a; };
+class C { int b; };
+S s;
+{
+  C c;
+}
+  }
+
+  {
+typedef char Char;
+using Int = int;
+Char c;
+{
+  Int i;
+}
+  }
+
+  {
+enum E { a, b, c };
+enum class T { aa, bb, cc };
+E e = E::a;
+{
+  T t = T::aa;
+}
+  }
+
+  {
+union U { int i; char c; };
+U u = { 256 };
+  }
+}
+
+// LIMITED: distinct !DICompileUnit
+// LIMITED-NOT: retainedTypes:
+
+// CHECK: !DILocalVariable(name: "s", scope: [[LBSCOPE_1:![0-9]+]]
+// CHECK-SAME:type: [[STRUCT:![0-9]+]]
+// CHECK: [[LBSCOPE_1]] = distinct !DILexicalBlock({{.*}}, localDecls: [[LB1_DECLS:![0-9]+]]
+// CHECK: [[LB1_DECLS]] = !{[[STRUCT]], [[CLASS:![0-9]+]]}
+// CHECK: [[STRUCT]] = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "S", scope: [[LBSCOPE_1]]
+// CHECK: [[CLASS]] = distinct !DICompositeType(tag: DW_TAG_class_type, name: "C", scope: [[LBSCOPE_1]]
+// CHECK: !DILocalVariable(name: "c", scope: [[LBSCOPE_11:![0-9]+]]
+// CHECK-SAME:type: [[CLASS]]
+// CHECK: [[LBSCOPE_11]] = distinct !DILexicalBlock(scope: [[LBSCOPE_1]]
+//
+// CHECK: !DILocalVariable(name: "c", scope: [[LBSCOPE_2:![0-9]+]]
+// CHECK-SAME:type: [[TYPEDEF:![0-9]+]]
+// CHECK: [[LBSCOPE_2]] = distinct !DILexicalBlock({{.*}}, localDecls: [[LB2_DECLS:![0-9]+]]
+// CHECK: [[LB2_DECLS]] = !{[[TYPEDEF]], [[USING:![0-9]+]]}
+// CHECK: [[TYPEDEF]] = !DIDerivedType(tag: DW_TAG_typedef, name: "Char", scope: [[LBSCOPE_2]]
+// CHECK: [[USING]] = !DIDerivedType(tag: DW_TAG_typedef, name: "Int", scope: [[LBSCOPE_2]]
+// CHECK: !DILocalVariable(name: "i", scope: [[LBSCOPE_21:![0-9]+]]
+// CHECK-SAME:type: [[USING]]
+// CHECK: [[LBSCOPE_21]] = distinct !DILexicalBlock(scope: [[LBSCOPE_2]]
+//
+// CHECK: !DILocalVariable(name: "e", scope: [[LBSCOPE_3:![0-9]+]]
+// CHECK-SAME:type: [[ENUM:![0-9]+]]
+// CHECK: [[LBSCOPE_3]] = distinct !DILexicalBlock({{.*}}, localDecls: [[LB3_DECLS:![0-9]+]]
+// CHECK: [[LB3_DECLS]] = !{[[ENUM]], [[ENUM_CLASS:![0-9]+]]}
+// CHECK: [[ENUM]] = !DICompositeType(tag: DW_TAG_enumeration_type, name: "E", scope: [[LBSCOPE_3]]
+// CHECK: [[ENUM_CLASS]] = !DICompositeType(tag: DW_TAG_enumeration_type, name: "T", scope: [[LBSCOPE_3]]
+// CHECK: !DILocalVariable(name: "t", scope: [[LBSCOPE_31:![0-9]+]]
+// CHECK-SAME:type: [[ENUM_CLASS]]
+// CHECK: [[LBSCOPE_31]] = distinct !DILexicalBlock(scope: [[LBSCOPE_3]]
+//
+// CHECK: !DILocalVariable(name: "u", scope: [[LBSCOPE_4:![0-9]+]]
+// CHECK-SAME:type: [[UNION:![0-9]+]]
+// CHECK: [[LBSCOPE_4]] = distinct !DILexicalBlock({{.*}}, localDecls: [[LB4_DECLS:![0-9]+]]
+// CHECK: [[LB4_DECLS]] = !{[[UNION]]}
+// CHECK: [[UNION]] = distinct !DICompositeType(tag: DW_TAG_union_type, name: "U", scope: [[LBSCOPE_4]]
+
+void test_unused() {
+  {
+struct X {};
+typedef int Y; // typedef doesn't go to retainedTypes.
+enum Z { z };
+  }
+}
+
+// UNUSED_TYPES: distinct !DICompileUnit
+// UNUSED_TYPES-SAME: retainedTypes: [[RETAINED_TYPES:![0-9]+]]
+//
+// struct, class, char, int, enum, enum class, union, unused struct, unused enum, ::F
+// UNUSED_TYPES: [[RETAINED_TYPES]] = !{!{{[0-9]+}}, !{{[0-9]+}}, !{{[0-9]+}}, !{{[0-9]+}}, !{{[0-9]+}}, !{{[0-9]+}}, !{{[0-9]+}}, [[UNUSED_STRUCT:![0-9]+]], [[UNUSED_ENUM:![0-9]+]], !{{.*}}}
+// UNUSED_TYPES: [[UNUSED_STRUCT]] = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "X", scope: [[UNUSED_LB:![0-9]+]]
+// UNUSED_TYPES: [[UNUSED_LB]] = distinct 

[PATCH] D91157: [AArch64] Out-of-line atomics (-moutline-atomics) implementation.

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

https://github.com/llvm/llvm-project/issues/29472 was never fixed; whatever 
issues exist with -moutline-atomics also exist with -mno-outline-atomics.  (I 
don't think anyone has run into any practical issues with this, so it hasn't 
been a priority for anyone.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91157

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

[PATCH] D115232: [clangd] Indexing of standard library

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

sorry for the long turn around here, LGTM. let's ship it!




Comment at: clang-tools-extra/clangd/index/StdLib.cpp:233
+  // Sadly we can't use IndexOpts.FileFilter to restrict indexing scope.
+  // Files from outside the location may define true std symbols anyway.
+  // We end up "blessing" such headers, and can only do that by indexing

sammccall wrote:
> kadircet wrote:
> > what does `the location` refer to here? I think we should also stress the 
> > fact that even when indexing the same file, we have a good chance of seeing 
> > different symbols due to PP directives (and different std versions)
> > what does the location refer to here? 
> 
> It refers to the StdLibLocation Loc, made that explicit.
> 
> > I think we should also stress the fact that even when indexing the same 
> > file, we have a good chance of seeing different symbols due to PP 
> > directives (and different std versions)
> 
> Different than what? Do you mean "why might different calls to 
> indexStandardLibrary see different symbols" from the same file?
> Different than what? Do you mean "why might different calls to 
> indexStandardLibrary see different symbols" from the same file?
yes, i meant compared to a previous runs. but i don't think it's as relevant 
here. i believe i was thinking about caching indexing status across runs and 
using that cache to implement filefilter, so that we don't index the same file 
twice (as we normally do in bgindex).



Comment at: clang-tools-extra/clangd/index/StdLib.cpp:256
+  if (HadErrors) {
+log("Errors when generating the standard library index, index may be "
+"incomplete");

sammccall wrote:
> kadircet wrote:
> > i'd make this part of the next log
> Can you say why? I generally like one thought per line. Scanning vertically 
> through familiar lines, it's easy to miss something unfamiliar tacked onto 
> the end. This message should be rare, and log lines aren't precious.
> 
> (I reordered them, which seems a bit more natural)
i was rather implying to add it as a `(in)complete` field into the current log 
line you have. usually when clangd is printing lots of logs across threads it 
might be hard to correlate these. hence having them printed as a single log 
would help.



Comment at: clang-tools-extra/clangd/index/StdLib.cpp:313
+llvm::StringRef DirPath = llvm::sys::path::parent_path(HeaderPath);
+if (!HS.getFileMgr().getVirtualFileSystem().getRealPath(DirPath, Path))
+  SearchPaths.emplace_back(Path);

sammccall wrote:
> sammccall wrote:
> > kadircet wrote:
> > > why do we resolve the symlinks ?
> > Oops, because I read the documentation of getCanonicalPath() instead of the 
> > implementation, and they diverged in 
> > https://github.com/llvm/llvm-project/commit/dd67793c0c549638cd93cad1142d324b979ba682
> >  :-D
> > 
> > Ultimately we're forming URIs to lexically compare with the ones emitted by 
> > the indexer, which uses getCanonicalPath(). Today getCanonicalPath() wants 
> > a FileEntry and we don't have one, but I think there's no fundamental 
> > reason for that, I can fix it.
> > 
> > (I'll do it as a separate patch, for now it's just calling getCanonicalPath 
> > with the wrong arguments)
> Actually, nevermind, the code is correct and I'd just forgotten why :-) Added 
> a comment to StdLibLocation.
> 
> getCanonicalPath() does actually resolve symlinks and so on: it asks the 
> FileManager for the directory entry and grabs the its "canonical name" which 
> is just FS->getRealPath behind a cache.
> So the strings are going to match the indexer after all.
> 
> It'd be possible to modify getCanonicalPath() so we can call it here, but I 
> don't think it helps. Calling it with (path, filemanager) is inconvenient for 
> the (many) existing callsites, so it'd have to be a new overload just for 
> this case. And the FileManager caching we'd gain doesn't matter here.
> I can still do it if you like, though.
> 
> (Also, relevant to your interests, realpath is probably taking care of case 
> mismatches too!)
>So the strings are going to match the indexer after all.

thanks, this makes sense.

> It'd be possible to modify getCanonicalPath() so we can call it here, but I 
> don't think it helps. Calling it with (path, filemanager) is inconvenient for 
> the (many) existing callsites, so it'd have to be a new overload just for 
> this case. And the FileManager caching we'd gain doesn't matter here.
> I can still do it if you like, though.

No need. We can take a look at that if the logic is likely to change (or get 
more complicated) in the future.



Comment at: clang-tools-extra/clangd/index/StdLib.h:66
+  // This function is threadsafe.
+  llvm::Optional add(const LangOptions &, const HeaderSearch 
&);
+

sammccall wrote:
> 

[PATCH] D125694: [clang][DebugInfo] Allow function-local statics to be scoped within a lexical block (4/5)

[Kristina Bessonova via Phabricator via cfe-commits]

krisb created this revision.
krisb added reviewers: dblaikie, aprantl, probinson.
Herald added a project: All.
krisb requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This is a part from D113743  split to make it 
easier to test and review.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125694

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/test/CodeGenCXX/debug-info-static-locals.cpp


Index: clang/test/CodeGenCXX/debug-info-static-locals.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/debug-info-static-locals.cpp
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -triple x86_64-none-linux-gnu -emit-llvm 
-debug-info-kind=limited %s -o - | FileCheck %s
+
+// Test that static local variables emitted in correct scopes.
+
+void test() {
+  static int bar = 2;
+  {
+static int bar = 1;
+{
+  static int bar = 0;
+}
+  }
+}
+
+// CHECK: [[FS_GVE:![0-9]+]] = !DIGlobalVariableExpression(var: 
[[FS_GV:![0-9]+]]
+// CHECK: [[FS_GV]] = distinct !DIGlobalVariable(name: "bar", scope: 
[[FSCOPE:![0-9]+]]
+// CHECK: [[FSCOPE]] = distinct !DISubprogram(name: "test"
+// CHECK-SAME:localDecls: [[FS_DECLS:![0-9]+]]
+// CHECK: [[FS_DECLS]] = !{[[FS_GVE]]}
+// CHECK: [[LB1_GVE:![0-9]+]] = !DIGlobalVariableExpression(var: 
[[LB1_GV:![0-9]+]]
+// CHECK: [[LB1_GV]] = distinct !DIGlobalVariable(name: "bar", scope: 
[[LB1SCOPE:![0-9]+]]
+// CHECK: [[LB1SCOPE]] = distinct !DILexicalBlock(scope: [[FSCOPE]]
+// CHECK-SAME:localDecls: 
[[LB1_DECLS:![0-9]+]]
+// CHECK: [[LB1_DECLS]] = !{[[LB1_GVE]]}
+// CHECK: [[LB2_GVE:![0-9]+]] = !DIGlobalVariableExpression(var: 
[[LB2_GV:![0-9]+]]
+// CHECK: [[LB2_GV]] = distinct !DIGlobalVariable(name: "bar", scope: 
[[LB2SCOPE:![0-9]+]]
+// CHECK: [[LB2SCOPE]] = distinct !DILexicalBlock(scope: [[LB1SCOPE]]
+// CHECK-SAME:localDecls: 
[[LB2_DECLS:![0-9]+]]
+// CHECK: [[LB2_DECLS]] = !{[[LB2_GVE]]}
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -3746,6 +3746,14 @@
 TemplateParameters = nullptr;
   }
 
+  // Get context for static locals (that are technically globals) the same way
+  // we do for "local" locals -- by using current lexical block.
+  if (VD->isStaticLocal()) {
+assert(!LexicalBlockStack.empty() && "Region stack mismatch, stack 
empty!");
+VDContext = LexicalBlockStack.back();
+return;
+  }
+
   // Since we emit declarations (DW_AT_members) for static members, place the
   // definition of those static members in the namespace they were declared in
   // in the source code (the lexical decl context).


Index: clang/test/CodeGenCXX/debug-info-static-locals.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/debug-info-static-locals.cpp
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -triple x86_64-none-linux-gnu -emit-llvm -debug-info-kind=limited %s -o - | FileCheck %s
+
+// Test that static local variables emitted in correct scopes.
+
+void test() {
+  static int bar = 2;
+  {
+static int bar = 1;
+{
+  static int bar = 0;
+}
+  }
+}
+
+// CHECK: [[FS_GVE:![0-9]+]] = !DIGlobalVariableExpression(var: [[FS_GV:![0-9]+]]
+// CHECK: [[FS_GV]] = distinct !DIGlobalVariable(name: "bar", scope: [[FSCOPE:![0-9]+]]
+// CHECK: [[FSCOPE]] = distinct !DISubprogram(name: "test"
+// CHECK-SAME:localDecls: [[FS_DECLS:![0-9]+]]
+// CHECK: [[FS_DECLS]] = !{[[FS_GVE]]}
+// CHECK: [[LB1_GVE:![0-9]+]] = !DIGlobalVariableExpression(var: [[LB1_GV:![0-9]+]]
+// CHECK: [[LB1_GV]] = distinct !DIGlobalVariable(name: "bar", scope: [[LB1SCOPE:![0-9]+]]
+// CHECK: [[LB1SCOPE]] = distinct !DILexicalBlock(scope: [[FSCOPE]]
+// CHECK-SAME:localDecls: [[LB1_DECLS:![0-9]+]]
+// CHECK: [[LB1_DECLS]] = !{[[LB1_GVE]]}
+// CHECK: [[LB2_GVE:![0-9]+]] = !DIGlobalVariableExpression(var: [[LB2_GV:![0-9]+]]
+// CHECK: [[LB2_GV]] = distinct !DIGlobalVariable(name: "bar", scope: [[LB2SCOPE:![0-9]+]]
+// CHECK: [[LB2SCOPE]] = distinct !DILexicalBlock(scope: [[LB1SCOPE]]
+// CHECK-SAME:localDecls: [[LB2_DECLS:![0-9]+]]
+// CHECK: [[LB2_DECLS]] = !{[[LB2_GVE]]}
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -3746,6 +3746,14 @@
 TemplateParameters = nullptr;
   }
 
+  // Get context for static locals (that are technically globals) the same way
+  // we do for "local" locals -- by using current lexical block.
+  if (VD->isStaticLocal()) {
+assert(!LexicalBlockStack.empty() && "Region stack 

[PATCH] D125693: [DebugInfo][WIP] Support types, imports and static locals declared in a lexical block (3/5)

[Kristina Bessonova via Phabricator via cfe-commits]

krisb added a comment.

Mark this and D125691  as WIP since I'm still 
testing the approach on various combinations of debug info and optimization 
options (O0, O3 , thinlto, 
split-dwarf, split-dwarf-inlining, gline-tables-only, etc.).
Except of the testing purpose itself I'm also trying to catch the issue 
reported by David here https://reviews.llvm.org/D113741#3437182 for previous 
implementation (while I hope this approach wouldn't trigger the issue, it'd be 
good to catch and fix it anyway).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125693

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

[PATCH] D91157: [AArch64] Out-of-line atomics (-moutline-atomics) implementation.

[Sebastian Pop via Phabricator via cfe-commits]

sebpop added a comment.
Herald added a subscriber: MaskRay.
Herald added a project: All.

Hi Pavel,

We need to handle one more case for __sync_* builtins, please see testcase and 
patches applied to GCC:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105162


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91157

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

[PATCH] D123676: [clang-format] Fix WhitespaceSensitiveMacros not being honoured when macro closing parenthesis is followed by a newline.

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir added a comment.

It looks like this regressed the following example by adding an unwanted level 
of indentation to the `#elif B` branch:

  % ./clang-format --version
  clang-format version 15.0.0 (https://github.com/llvm/llvm-project.git 
50cd52d9357224cce66a9e00c9a0417c658a5655)
  % cat test.cc 
  #define MACRO_BEGIN
  
  MACRO_BEGIN
  
  namespace internal {
  
  #if A
  int f() { return 0; }
  #elif B
  int f() { return 1; }
  #endif
  
  }  // namespace internal
  % ./clang-format test.cc
  #define MACRO_BEGIN
  
  MACRO_BEGIN
  
  namespace internal {
  
  #if A
  int f() { return 0; }
  #elif B
int f() { return 1; }
  #endif
  
  } // namespace internal
  % 

@curdeius could you please take a look?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123676

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

[PATCH] D125555: [clang] Add __has_target_feature

[Yaxun Liu via Phabricator via cfe-commits]

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



Comment at: clang/docs/LanguageExtensions.rst:275
+  // On amdgcn target
+  #if __has_target_feature("s-memtime-inst")
+x = __builtin_amdgcn_s_memtime();

yaxunl wrote:
> aaron.ballman wrote:
> > yaxunl wrote:
> > > tra wrote:
> > > > Do you have a better example? This particular case could've been 
> > > > handled by existing `__has_builtin()`.
> > > > 
> > > > While I could see usefulness of checking features (e.g. for CUDA/NVPTX 
> > > > it could be used to chose inline assembly supported only by newer PTX 
> > > > versions, but even then that choice could be made using existing 
> > > > methods, even if they are not always direct (e.g. by using CUDA_VERSION 
> > > > as a proxy for "new enough PTX version").
> > > > 
> > > `__has_builtin` returns 1 as long as the builtin is known to clang, even 
> > > if the builtin is not supported by the target CPU. This is because the 
> > > required target feature for a builtin is in ASTContext, whereas 
> > > `__has_builtin` is evaluated in preprocessor, where the information is 
> > > not known.
> > > `__has_builtin` returns 1 as long as the builtin is known to clang, even 
> > > if the builtin is not supported by the target CPU. This is because the 
> > > required target feature for a builtin is in ASTContext, whereas 
> > > `__has_builtin` is evaluated in preprocessor, where the information is 
> > > not known.
> > 
> > Why is that not a deficiency with `__has_builtin` that we should fix?
> It is arguable whether this is a bug for `__has_builtin`. I tend to treat it 
> as a bug and think it should be fixed. However, fixing it may cause 
> regressions since there may be existing code expecting `__has_builtin` not 
> depending on availability of required target feature. This will limit the 
> usability of a fix for this issue.
> 
> Even if we agree that this is a bug for `__has_builtin` which should be 
> fixed, this does conflict with adding `__has_target_feature`, as 
> `__has_target_feature` has more uses than determining whether a target 
> builtin is available, e.g. choosing different algorithms based on 
> availability of certain target feature, or using certain inline assembly code.
> It is arguable whether this is a bug for `__has_builtin`. I tend to treat it 
> as a bug and think it should be fixed. However, fixing it may cause 
> regressions since there may be existing code expecting `__has_builtin` not 
> depending on availability of required target feature. This will limit the 
> usability of a fix for this issue.
> 
> Even if we agree that this is a bug for `__has_builtin` which should be 
> fixed, this does conflict with adding `__has_target_feature`, as 
> `__has_target_feature` has more uses than determining whether a target 
> builtin is available, e.g. choosing different algorithms based on 
> availability of certain target feature, or using certain inline assembly code.

sorry typo. this does not conflict


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

https://reviews.llvm.org/D12

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

[PATCH] D125555: [clang] Add __has_target_feature

[Yaxun Liu via Phabricator via cfe-commits]

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



Comment at: clang/docs/LanguageExtensions.rst:260
 
+``__has_target_feature``
+

aaron.ballman wrote:
> The first question that comes to mind for me is: why is `__has_feature` not 
> sufficient?
`__has_target_feature` accepts predefined target features for the specified 
target triple and CPU. The names of target features are determined by the 
target. They may overlap with clang features and cause ambiguity. Another issue 
is that they usually contain '-', which makes them not valid identifiers of 
C/C++, therefore the builtin macro has to accept a string literal argument 
instead of identifier argument like `__has_feature`. 



Comment at: clang/docs/LanguageExtensions.rst:275
+  // On amdgcn target
+  #if __has_target_feature("s-memtime-inst")
+x = __builtin_amdgcn_s_memtime();

aaron.ballman wrote:
> yaxunl wrote:
> > tra wrote:
> > > Do you have a better example? This particular case could've been handled 
> > > by existing `__has_builtin()`.
> > > 
> > > While I could see usefulness of checking features (e.g. for CUDA/NVPTX it 
> > > could be used to chose inline assembly supported only by newer PTX 
> > > versions, but even then that choice could be made using existing methods, 
> > > even if they are not always direct (e.g. by using CUDA_VERSION as a proxy 
> > > for "new enough PTX version").
> > > 
> > `__has_builtin` returns 1 as long as the builtin is known to clang, even if 
> > the builtin is not supported by the target CPU. This is because the 
> > required target feature for a builtin is in ASTContext, whereas 
> > `__has_builtin` is evaluated in preprocessor, where the information is not 
> > known.
> > `__has_builtin` returns 1 as long as the builtin is known to clang, even if 
> > the builtin is not supported by the target CPU. This is because the 
> > required target feature for a builtin is in ASTContext, whereas 
> > `__has_builtin` is evaluated in preprocessor, where the information is not 
> > known.
> 
> Why is that not a deficiency with `__has_builtin` that we should fix?
It is arguable whether this is a bug for `__has_builtin`. I tend to treat it as 
a bug and think it should be fixed. However, fixing it may cause regressions 
since there may be existing code expecting `__has_builtin` not depending on 
availability of required target feature. This will limit the usability of a fix 
for this issue.

Even if we agree that this is a bug for `__has_builtin` which should be fixed, 
this does conflict with adding `__has_target_feature`, as 
`__has_target_feature` has more uses than determining whether a target builtin 
is available, e.g. choosing different algorithms based on availability of 
certain target feature, or using certain inline assembly code.


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

https://reviews.llvm.org/D12

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

[PATCH] D124806: [clang-tidy] add support for Demorgan conversions to readability-simplify-bool-expr

[Nathan James via Phabricator via cfe-commits]

njames93 updated this revision to Diff 429715.
njames93 added a comment.

Rebased.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124806

Files:
  clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
  clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/readability-simplify-boolean-expr.rst
  
clang-tools-extra/test/clang-tidy/checkers/readability-simplify-bool-expr-demorgan.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability-simplify-bool-expr-demorgan.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability-simplify-bool-expr-demorgan.cpp
@@ -0,0 +1,67 @@
+// RUN: %check_clang_tidy %s readability-simplify-boolean-expr %t
+
+void foo(bool A1, bool A2, bool A3, bool A4) {
+  bool X;
+  X = !(!A1 || A2);
+  X = !(A1 || !A2);
+  X = !(!A1 || !A2);
+  // CHECK-MESSAGES: :[[@LINE-3]]:7: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-MESSAGES: :[[@LINE-3]]:7: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-MESSAGES: :[[@LINE-3]]:7: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-FIXES: X = (A1 && !A2);
+  // CHECK-FIXES-NEXT: X = (!A1 && A2);
+  // CHECK-FIXES-NEXT: X = (A1 && A2);
+
+  X = !(!A1 && A2);
+  X = !(A1 && !A2);
+  X = !(!A1 && !A2);
+  // CHECK-MESSAGES: :[[@LINE-3]]:7: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-MESSAGES: :[[@LINE-3]]:7: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-MESSAGES: :[[@LINE-3]]:7: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-FIXES: X = (A1 || !A2);
+  // CHECK-FIXES-NEXT: X = (!A1 || A2);
+  // CHECK-FIXES-NEXT: X = (A1 || A2);
+
+  X = !(!A1 && !A2 && !A3);
+  X = !(!A1 && (!A2 && !A3));
+  X = !(!A1 && (A2 && A3));
+  // CHECK-MESSAGES: :[[@LINE-3]]:7: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-MESSAGES: :[[@LINE-3]]:7: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-MESSAGES: :[[@LINE-3]]:7: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-FIXES: X = (A1 || A2 || A3);
+  // CHECK-FIXES-NEXT: X = (A1 || A2 || A3);
+  // CHECK-FIXES-NEXT: X = (A1 || !A2 || !A3);
+
+  X = !(A1 && A2 == A3);
+  X = !(!A1 && A2 > A3);
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-FIXES: X = (!A1 || A2 != A3);
+  // CHECK-FIXES-NEXT: X = (A1 || A2 <= A3);
+
+  // Ensure the check doesn't try to combine fixes for the inner and outer demorgan simplification.
+  X = !(!A1 && !(!A2 && !A3));
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-MESSAGES: :[[@LINE-2]]:16: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-FIXES: X = (A1 || (!A2 && !A3));
+
+  // Testing to see how it handles parens
+  X = !(A1 && !A2 && !A3);
+  X = !(A1 && !A2 || !A3);
+  X = !(!A1 || A2 && !A3);
+  X = !((A1 || !A2) && !A3);
+  X = !((A1 || !A2) || !A3);
+  // CHECK-MESSAGES: :[[@LINE-5]]:7: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-MESSAGES: :[[@LINE-5]]:7: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-MESSAGES: :[[@LINE-5]]:7: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-MESSAGES: :[[@LINE-5]]:7: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-MESSAGES: :[[@LINE-5]]:7: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-FIXES: X = (!A1 || A2 || A3);
+  // CHECK-FIXES-NEXT: X = ((!A1 || A2) && A3);
+  // CHECK-FIXES-NEXT: X = (A1 && (!A2 || A3));
+  // CHECK-FIXES-NEXT: X = ((!A1 && A2) || A3);
+  // CHECK-FIXES-NEXT: X = (!A1 && A2 && A3);
+  X = !((A1 || A2) && (!A3 || A4));
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-FIXES: X = ((!A1 && !A2) || (A3 && !A4));
+}
Index: clang-tools-extra/docs/clang-tidy/checks/readability-simplify-boolean-expr.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/readability-simplify-boolean-expr.rst
+++ clang-tools-extra/docs/clang-tidy/checks/readability-simplify-boolean-expr.rst
@@ -4,7 +4,8 @@
 =
 
 Looks for boolean expressions involving boolean constants and simplifies
-them to use the appropriate boolean expression directly.
+them to use the appropriate boolean expression