[PATCH] D148505: Allow `__attribute__((warn_unused))` on individual constructors

[Stephan Bergmann via Phabricator via cfe-commits]

sberg added a comment.

In D148505#4302702 , @aaron.ballman 
wrote:

> Thank you for poking on this! FWIW, I don't know that there's a way to 
> cross-post to Discourse (but if I'm wrong and there is, I'd love to know 
> how!).

Ping, any further input from anybody?

(The cross-posting didn't work, but there was also almost no response on the 
GCC side, https://gcc.gnu.org/pipermail/gcc/2023-April/241220.html "Re: 
GCC/Clang attributes guiding warnings about unused entities".)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148505

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

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

[Stephan Bergmann via Phabricator via cfe-commits]

sberg added a comment.

In D146178#4314999 , @sberg wrote:

> Since this commit...

ah, already taken care of with 
https://github.com/llvm/llvm-project/commit/3e850a6eea5277082a0b7b701754c86530d25c40
 "Revert '[Clang][Sema] Fix comparison of constraint expressions'"


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146178

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

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

[Stephan Bergmann via Phabricator via cfe-commits]

sberg added a comment.

Since this commit (plus its follow-up 
https://github.com/llvm/llvm-project/commit/ce861ec782ae3f41807b61e855512aaccf3c2149
 "[Clang][Sema] Add a temporary workaround in SemaConcept.cpp", to avoid 
`clang/lib/AST/ExprConstant.cpp:15332: bool 
clang::Expr::EvaluateAsConstantExpr(EvalResult &, const ASTContext &, 
ConstantExprKind) const: Assertion `!isValueDependent() && "Expression 
evaluator can't be called on a dependent expression."' failed` in 
`-DLLVM_ENABLE_ASSERTIONS=ON` builds):

  $ cat test.cc
  #include 
  #include 
  #include 
  std::vector> v1;
  std::vector> v2;
  void f() {
v2.insert(v2.end(), std::make_move_iterator(v1.begin()), 
std::make_move_iterator(v1.end()));
  }

  $ clang++ -std=c++20 -fsyntax-only test.cc
  In file included from test.cc:1:
  In file included from 
/usr/lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/memory:66:
  In file included from 
/usr/lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/bits/stl_tempbuf.h:61:
  
/usr/lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/bits/stl_construct.h:115:4:
 error: no matching function for call to 'construct_at'
std::construct_at(__p, std::forward<_Args>(__args)...);
^
  
/usr/lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/bits/stl_uninitialized.h:120:11:
 note: in instantiation of function template specialization 
'std::_Construct, std::unique_ptr &>' requested here
  std::_Construct(std::__addressof(*__cur), *__first);
   ^
  
/usr/lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/bits/stl_uninitialized.h:371:14:
 note: in instantiation of function template specialization 
'std::__do_uninit_copy *>, 
std::unique_ptr *>' requested here
  return std::__do_uninit_copy(__first, __last, __result);
  ^
  
/usr/lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/bits/stl_uninitialized.h:384:19:
 note: in instantiation of function template specialization 
'std::__uninitialized_copy_a *>, 
std::unique_ptr *, std::unique_ptr>' requested here
return std::__uninitialized_copy_a(_GLIBCXX_MAKE_MOVE_ITERATOR(__first),
^
  
/usr/lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/bits/vector.tcc:766:12:
 note: in instantiation of function template specialization 
'std::__uninitialized_move_a *, std::unique_ptr *, 
std::allocator>>' requested here
  std::__uninitialized_move_a(this->_M_impl._M_finish - __n,
   ^
  
/usr/lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/bits/stl_vector.h:1481:4:
 note: in instantiation of function template specialization 
'std::vector>::_M_range_insert
 *, std::vector' requested here
_M_range_insert(begin() + __offset, __first, __last,
^
  test.cc:7:6: note: in instantiation of function template specialization 
'std::vector>::insert
 *, std::vector>>>, void>' requested here
v2.insert(v2.end(), std::make_move_iterator(v1.begin()), 
std::make_move_iterator(v1.end()));
   ^
  
/usr/lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/bits/stl_construct.h:94:5:
 note: candidate template ignored: substitution failure [with _Tp = 
std::unique_ptr, _Args =  &>]: call to deleted 
constructor of 'std::unique_ptr'
  construct_at(_Tp* __location, _Args&&... __args)
  ^
  
/usr/lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/bits/stl_construct.h:119:25:
 error: call to deleted constructor of 'std::unique_ptr'
::new((void*)__p) _Tp(std::forward<_Args>(__args)...);
  ^   ~~~
  
/usr/lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/bits/unique_ptr.h:522:7:
 note: 'unique_ptr' has been explicitly marked deleted here
unique_ptr(const unique_ptr&) = delete;
^
  In file included from test.cc:1:
  In file included from 
/usr/lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/memory:69:
  
/usr/lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/bits/stl_uninitialized.h:120:6:
 error: no matching function for call to '_Construct'
  std::_Construct(std::__addressof(*__cur), *__first);
  ^~~
  
/usr/lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/bits/stl_uninitialized.h:371:14:
 note: in instantiation of function template specialization 
'std::__do_uninit_copy
 *, std::vector>>>, std::unique_ptr *>' requested here
  return std::__do_uninit_copy(__first, __last, __result);
  ^
  
/usr/lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/bits/vector.tcc:781:12:
 note: in instantiation of function template specialization 
'std::__uninitialized_copy_a
 *, std::vector>>>, std::unique_ptr *, 
std::unique_ptr>' requested here
  std::__uninitialized_copy_a(__mid, __last,
   ^
  

[PATCH] D148505: Allow `__attribute__((warn_unused))` on individual constructors

[Stephan Bergmann via Phabricator via cfe-commits]

sberg added a comment.

> But the fact that these two attributes already confuse users (AND we've got 
> unused as an attribute in the mix as well, which doesn't help) means we 
> should proceed with caution (and likely coordinate with GCC given that all 
> three of these attributes came from their implementation).

I've brought this up at 
https://gcc.gnu.org/pipermail/gcc/2023-April/241205.html "GCC/Clang attributes 
guiding warnings about unused entities" now.  (I tried to cross-post that also 
to the Clang Frontend discourse category, but that probably didn't work.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148505

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

[PATCH] D148505: Allow `__attribute__((warn_unused))` on individual constructors

[Stephan Bergmann via Phabricator via cfe-commits]

sberg added inline comments.



Comment at: clang/include/clang/Basic/Attr.td:2996
   let Spellings = [GCC<"warn_unused">];
-  let Subjects = SubjectList<[Record]>;
+  let Subjects = SubjectList<[Record, CXXConstructor]>;
   let Documentation = [Undocumented];

aaron.ballman wrote:
> I'm confused -- if you want this to behave like `nodiscard`, why aren't these 
> changes for `warn_unused_result` (which is what implements `nodiscard`)? I 
> don't think this should go on `warn_unused` because that's for the 
> declaration of the type as a unit and not written on functions.
> 
> I think you don't need any changes to Attr.td for this functionality.
`[[nodiscard]]` and `__attribute__((warn_unused))` already have different 
semantics when applied to a class:  While both warn about unused expression 
results (`-Wunused-value`),  only the latter warns about unused variables 
(`-Wunused-variable`).  This patch keeps that difference, just extends it to 
individual constructors:
```
$ cat test.cc
struct [[nodiscard]] S1 {
  S1(int) {}
  S1(double) {}
};
struct S2 {
  [[nodiscard]] S2(int) {}
  S2(double) {}
};
struct __attribute__((warn_unused)) S3 {
  S3(int) {}
  S3(double) {}
};
struct S4 {
  __attribute__((warn_unused)) S4(int) {}
  S4(double) {}
};
int main() {
  S1(0); // expected-warning {{ignoring temporary}}
  S1(0.0); // expected-warning {{ignoring temporary}}
  S2(0); // expected-warning {{ignoring temporary}}
  S2(0.0);
  S3(0); // expected-warning {{expression result unused}}
  S3(0.0); // expected-warning {{expression result unused}}
  S4(0); // expected-warning {{expression result unused}}
  S4(0.0);
  S1 s1a(0);
  S1 s1b(0.0);
  S2 s2a(0);
  S2 s2b(0.0);
  S3 s3a(0); // expected-warning {{unused variable}}
  S3 s3b(0.0); // expected-warning {{unused variable}}
  S4 s4a(0); // expected-warning {{unused variable}}
  S4 s4b(0.0);
}
```
```
$ clang++ -Wunused-value -Wunused-variable -fsyntax-only -Xclang -verify test.cc
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148505

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

[PATCH] D148505: Allow `__attribute__((warn_unused))` on individual constructors

[Stephan Bergmann via Phabricator via cfe-commits]

sberg created this revision.
sberg added a reviewer: erichkeane.
Herald added a reviewer: aaron.ballman.
Herald added a subscriber: jdoerfert.
Herald added a project: All.
sberg requested review of this revision.
Herald added a project: clang.

This takes inspiration from the standard `nodiscard` attribute, which can also 
be declared on individual constructors.  (I have used this change locally for 
about a year on the LibreOffice code base, marking various compilers as 
`warn_unused`, without any positive or negative effects.  Until I happened to 
notice an additional constructor that would benefit from `warn_unused`, and 
which found about 20 unused variables across the LibreOffice code base, see 
e.g., 
https://git.libreoffice.org/core/+/7cdbe504ff3b59d3aec1b1e86caf7f24223dce72%5E! 
"Fix what looks like copy/paste typos".)

(The changes in `clang/test/Misc/pragma-attribute-strict-subjects.c`, 
`clang/test/Parser/pragma-attribute.cpp`, and 
`clang/test/Sema/pragma-attribute-strict-subjects.c` are necessary to make 
those tests not fail after adding `SubRuleForCXXConstructor` to 
`SubjectMatcherForFunction` in `clang/include/clang/Basic/Attr.td`.  It appears 
that the exact diagnostic output that is generated is somewhat brittle, 
depending on implementation details.)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D148505

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/DeclNodes.td
  clang/lib/AST/Expr.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/test/Misc/pragma-attribute-strict-subjects.c
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/Parser/pragma-attribute.cpp
  clang/test/Sema/pragma-attribute-strict-subjects.c
  clang/test/SemaCXX/warn-unused-attribute.cpp

Index: clang/test/SemaCXX/warn-unused-attribute.cpp
===
--- clang/test/SemaCXX/warn-unused-attribute.cpp
+++ clang/test/SemaCXX/warn-unused-attribute.cpp
@@ -9,12 +9,21 @@
   TestNormal();
 };
 
+struct TestCtor {
+  __attribute__((warn_unused)) TestCtor();
+  TestCtor(int);
+  void __attribute__((warn_unused)) f(); // expected-warning {{'warn_unused' attribute only applies to structs, unions, classes, and constructors}}
+};
+
 int main(void) {
   Test unused; // expected-warning {{unused variable 'unused'}}
   Test used;
   TestNormal normal;
   used.use();
 
-  int i __attribute__((warn_unused)) = 12; // expected-warning {{'warn_unused' attribute only applies to structs, unions, and classes}}
+  TestCtor ctorUnused; // expected-warning {{unused variable 'ctorUnused'}}
+  TestCtor ctorUsed(0);
+
+  int i __attribute__((warn_unused)) = 12; // expected-warning {{'warn_unused' attribute only applies to structs, unions, classes, and constructors}}
   return i;
 }
Index: clang/test/Sema/pragma-attribute-strict-subjects.c
===
--- clang/test/Sema/pragma-attribute-strict-subjects.c
+++ clang/test/Sema/pragma-attribute-strict-subjects.c
@@ -28,7 +28,7 @@
 #pragma clang attribute pop
 
 #pragma clang attribute push (__attribute__((annotate("negatedSubRuleContradictions2"))), apply_to = any(variable(unless(is_parameter)), variable(is_thread_local), function, variable(is_global)))
-// expected-error@-1 {{negated attribute subject matcher sub-rule 'variable(unless(is_parameter))' contradicts sub-rule 'variable(is_global)'}}
+// expected-error@-1 {{negated attribute subject matcher sub-rule 'variable(unless(is_parameter))' contradicts sub-rule 'variable(is_thread_local)'}}
 // We have just one error, don't error on 'variable(is_global)'
 
 #pragma clang attribute pop
Index: clang/test/Parser/pragma-attribute.cpp
===
--- clang/test/Parser/pragma-attribute.cpp
+++ clang/test/Parser/pragma-attribute.cpp
@@ -190,7 +190,7 @@
 #pragma clang attribute pop
 #pragma clang attribute push([[clang::uninitialized]], apply_to = variable(is_global)) // expected-error {{attribute 'uninitialized' can't be applied to 'variable(is_global)'}}
 #pragma clang attribute pop
-#pragma clang attribute push([[clang::uninitialized]], apply_to = any(variable(is_parameter), variable(unless(is_parameter // expected-error {{attribute 'uninitialized' can't be applied to 'variable(is_parameter)', and 'variable(unless(is_parameter))'}}
+#pragma clang attribute push([[clang::uninitialized]], apply_to = any(variable(is_parameter), variable(unless(is_parameter // expected-error {{attribute 'uninitialized' can't be applied to 'variable(unless(is_parameter))', and 'variable(is_parameter)'}}
 #pragma clang attribute pop
 // We're allowed to apply attributes to subsets of allowed subjects.
 #pragma clang attribute push([[clang::no_destroy]], apply_to = variable)
Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test

[PATCH] D145123: Call MarkVirtualMembersReferenced on an actual class definition

[Stephan Bergmann via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGd812488d3c54: Call MarkVirtualMembersReferenced on an actual 
class definition (authored by sberg).

Changed prior to commit:
  https://reviews.llvm.org/D145123?vs=501669=501852#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145123

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/SemaCXX/warn-undefined-internal.cpp


Index: clang/test/SemaCXX/warn-undefined-internal.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/warn-undefined-internal.cpp
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -fsyntax-only -Wundefined-internal -verify %s
+
+void test1() {
+  struct S { virtual void f(); };
+  // expected-warning@-1{{function 'test1()::S::f' has internal linkage but is 
not defined}}
+  S s;
+  // expected-note@-1{{used here}}
+}
+
+void test2() {
+  struct S;
+  struct S { virtual void f(); };
+  // expected-warning@-1{{function 'test2()::S::f' has internal linkage but is 
not defined}}
+  S s;
+  // expected-note@-1{{used here}}
+}
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -17975,7 +17975,7 @@
   // immediately. For all other classes, we mark their virtual members
   // at the end of the translation unit.
   if (Class->isLocalClass())
-MarkVirtualMembersReferenced(Loc, Class);
+MarkVirtualMembersReferenced(Loc, Class->getDefinition());
   else
 VTableUses.push_back(std::make_pair(Class, Loc));
 }
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -155,6 +155,8 @@
 - Clang now warns by default for C++20 and later about deprecated capture of
   ``this`` with a capture default of ``=``. This warning can be disabled with
   ``-Wno-deprecated-this-capture``.
+- Clang had failed to emit some ``-Wundefined-internal`` for members of a local
+  class if that class was first introduced with a forward declaration.
 
 Bug Fixes in This Version
 -


Index: clang/test/SemaCXX/warn-undefined-internal.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/warn-undefined-internal.cpp
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -fsyntax-only -Wundefined-internal -verify %s
+
+void test1() {
+  struct S { virtual void f(); };
+  // expected-warning@-1{{function 'test1()::S::f' has internal linkage but is not defined}}
+  S s;
+  // expected-note@-1{{used here}}
+}
+
+void test2() {
+  struct S;
+  struct S { virtual void f(); };
+  // expected-warning@-1{{function 'test2()::S::f' has internal linkage but is not defined}}
+  S s;
+  // expected-note@-1{{used here}}
+}
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -17975,7 +17975,7 @@
   // immediately. For all other classes, we mark their virtual members
   // at the end of the translation unit.
   if (Class->isLocalClass())
-MarkVirtualMembersReferenced(Loc, Class);
+MarkVirtualMembersReferenced(Loc, Class->getDefinition());
   else
 VTableUses.push_back(std::make_pair(Class, Loc));
 }
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -155,6 +155,8 @@
 - Clang now warns by default for C++20 and later about deprecated capture of
   ``this`` with a capture default of ``=``. This warning can be disabled with
   ``-Wno-deprecated-this-capture``.
+- Clang had failed to emit some ``-Wundefined-internal`` for members of a local
+  class if that class was first introduced with a forward declaration.
 
 Bug Fixes in This Version
 -
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D145123: Call MarkVirtualMembersReferenced on an actual class definition

[Stephan Bergmann via Phabricator via cfe-commits]

sberg added inline comments.



Comment at: clang/lib/Sema/SemaDeclCXX.cpp:17940
   LoadExternalVTableUses();
   Class = Class->getCanonicalDecl();
   std::pair::iterator, bool>

That call of `getCanonicalDecl` originated with 

 "Rework when and how vtables are emitted, by tracking where vtables are", and 
I wonder if more of the uses of the modified `Class` below suffer from the same 
issue, that they expect `Class` to represent a definition when it might 
potentially reference just a declaration.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145123

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

[PATCH] D145123: Call MarkVirtualMembersReferenced on an actual class definition

[Stephan Bergmann via Phabricator via cfe-commits]

sberg created this revision.
sberg added reviewers: doug.gregor, aaron.ballman.
sberg added a project: clang.
Herald added a project: All.
sberg requested review of this revision.

...rather than on potentially just a declaration.
Without the fix, the newly added 
`clang/test/SemaCXX/warn-undefined-internal.cpp` failed with

  error: 'warning' diagnostics expected but not seen:
File 
/home/sbergman/github.com/llvm/llvm-project/clang/test/SemaCXX/warn-undefined-internal.cpp
 Line 12 (directive at 
/home/sbergman/github.com/llvm/llvm-project/clang/test/SemaCXX/warn-undefined-internal.cpp:13):
 function 'test2()::S::f' has internal linkage but is not defined
  error: 'note' diagnostics expected but not seen:
File 
/home/sbergman/github.com/llvm/llvm-project/clang/test/SemaCXX/warn-undefined-internal.cpp
 Line 14 (directive at 
/home/sbergman/github.com/llvm/llvm-project/clang/test/SemaCXX/warn-undefined-internal.cpp:15):
 used here

(I ran into this when two LibreOffice Clang plugins produced false positive 
warnings, as they relied on `Decl::isReferenced()` returning true for such 
virtual member functions of local classes.)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D145123

Files:
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/SemaCXX/warn-undefined-internal.cpp


Index: clang/test/SemaCXX/warn-undefined-internal.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/warn-undefined-internal.cpp
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -fsyntax-only -Wundefined-internal -verify %s
+
+void test1() {
+  struct S { virtual void f(); };
+  // expected-warning@-1{{function 'test1()::S::f' has internal linkage but is 
not defined}}
+  S s;
+  // expected-note@-1{{used here}}
+}
+
+void test2() {
+  struct S;
+  struct S { virtual void f(); };
+  // expected-warning@-1{{function 'test2()::S::f' has internal linkage but is 
not defined}}
+  S s;
+  // expected-note@-1{{used here}}
+}
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -17975,7 +17975,7 @@
   // immediately. For all other classes, we mark their virtual members
   // at the end of the translation unit.
   if (Class->isLocalClass())
-MarkVirtualMembersReferenced(Loc, Class);
+MarkVirtualMembersReferenced(Loc, Class->getDefinition());
   else
 VTableUses.push_back(std::make_pair(Class, Loc));
 }


Index: clang/test/SemaCXX/warn-undefined-internal.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/warn-undefined-internal.cpp
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -fsyntax-only -Wundefined-internal -verify %s
+
+void test1() {
+  struct S { virtual void f(); };
+  // expected-warning@-1{{function 'test1()::S::f' has internal linkage but is not defined}}
+  S s;
+  // expected-note@-1{{used here}}
+}
+
+void test2() {
+  struct S;
+  struct S { virtual void f(); };
+  // expected-warning@-1{{function 'test2()::S::f' has internal linkage but is not defined}}
+  S s;
+  // expected-note@-1{{used here}}
+}
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -17975,7 +17975,7 @@
   // immediately. For all other classes, we mark their virtual members
   // at the end of the translation unit.
   if (Class->isLocalClass())
-MarkVirtualMembersReferenced(Loc, Class);
+MarkVirtualMembersReferenced(Loc, Class->getDefinition());
   else
 VTableUses.push_back(std::make_pair(Class, Loc));
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D139847: Also allow __is_unsigned to be used as an identifier

[Stephan Bergmann via Phabricator via cfe-commits]

sberg abandoned this revision.
sberg added a comment.

In D139847#4026966 , @aaron.ballman 
wrote:

> Do you know if folks are hitting problems here in practice, or is this 
> speculative?

It had hit me when building LibreOffice against libstdc++ 13 trunk, so I 
started writing this hack.  In parallel, libstdc++ 13 trunk got changed again 
to not use `__is_unsigned` as an identifier, so there should be no published 
releases of libstdc++ where this would be an issue.  (And I had finished 
writing this hack anyway, just in case it might be useful.  And then forgot 
about it...  But maybe its best to just abandon it now.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139847

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

[PATCH] D86881: Make -fvisibility-inlines-hidden apply to static local variables in inline functions on Darwin

[Stephan Bergmann via Phabricator via cfe-commits]

sberg added a comment.
Herald added a project: All.

In D86881#2777808 , @arphaman wrote:

> Hey, thanks for following up on this PR. I've done some more digging and I 
> think we can remove this Darwin-specific workaround in the near future. I'm 
> hoping to provide an update in the next few weeks.

The Darwin `-fvisibility-inlines-hidden-static-local-var` hack is still present 
on Clang 16 trunk.  Any update on this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86881

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

[PATCH] D139847: Also allow __is_unsigned to be used as an identifier

[Stephan Bergmann via Phabricator via cfe-commits]

sberg created this revision.
sberg added reviewers: doug.gregor, aaron.ballman.
Herald added a project: All.
sberg requested review of this revision.
Herald added a project: clang.

...similar to 068730992cf08d7d7e82e7bb147d85f429fc3ddd "libstdc++ 4.4 uses 
__is_signed as an identifier [...]"

Starting with 
https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=5329e1a8e1480d536ff96283a6556e51112ba470
 "libstdc++: Make chrono::hh_mm_ss more compact", libstdc++ 13 trunk used 
`__is_signed` as

  static constexpr bool __is_unsigned
= __and_v,
  is_unsigned>;

in `libstdc++-v3/include/std/chrono`.  Even though that got reverted with 
https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=cb363fd9f19eb791e1ee1eb0d5c61f5fdf21af32
 "libstdc++: Change names that clash with Win32 or Clang", it might make sense 
to treat `__is_unsigned` the same as `__is_signed` here (and to also make the 
workaround hit for `constexpr` in addition to `const`).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D139847

Files:
  clang/lib/Parse/ParseDecl.cpp
  clang/test/SemaCXX/libstdcxx_is_pod_hack.cpp


Index: clang/test/SemaCXX/libstdcxx_is_pod_hack.cpp
===
--- clang/test/SemaCXX/libstdcxx_is_pod_hack.cpp
+++ clang/test/SemaCXX/libstdcxx_is_pod_hack.cpp
@@ -29,6 +29,13 @@
 
 bool check_signed = test_is_signed::__is_signed;
 
+// Another, similar egregious hack for __is_unsigned, which is a type
+// trait in Embarcadero's compiler but is used as an identifier in
+// libstdc++.
+struct test_is_unsigned {
+  static constexpr bool __is_unsigned = true; // expected-warning {{keyword 
'__is_unsigned' will be made available as an identifier}}
+};
+
 template struct must_be_true {};
 template<> struct must_be_true;
 
Index: clang/lib/Parse/ParseDecl.cpp
===
--- clang/lib/Parse/ParseDecl.cpp
+++ clang/lib/Parse/ParseDecl.cpp
@@ -3519,6 +3519,7 @@
 }
 
 case tok::kw___is_signed:
+case tok::kw___is_unsigned:
   // GNU libstdc++ 4.4 uses __is_signed as an identifier, but Clang
   // typically treats it as a trait. If we see __is_signed as it appears
   // in libstdc++, e.g.,
@@ -3526,8 +3527,15 @@
   //   static const bool __is_signed;
   //
   // then treat __is_signed as an identifier rather than as a keyword.
+  //
+  // Similarly, libstdc++ 13 trunk at one point used __is_unsigned as an
+  // identifier as
+  //
+  //   static constexpr bool __is_unsigned;
+  //
   if (DS.getTypeSpecType() == TST_bool &&
-  DS.getTypeQualifiers() == DeclSpec::TQ_const &&
+  (DS.getTypeQualifiers() == DeclSpec::TQ_const ||
+   DS.getConstexprSpecifier() == ConstexprSpecKind::Constexpr) &&
   DS.getStorageClassSpec() == DeclSpec::SCS_static)
 TryKeywordIdentFallback(true);
 


Index: clang/test/SemaCXX/libstdcxx_is_pod_hack.cpp
===
--- clang/test/SemaCXX/libstdcxx_is_pod_hack.cpp
+++ clang/test/SemaCXX/libstdcxx_is_pod_hack.cpp
@@ -29,6 +29,13 @@
 
 bool check_signed = test_is_signed::__is_signed;
 
+// Another, similar egregious hack for __is_unsigned, which is a type
+// trait in Embarcadero's compiler but is used as an identifier in
+// libstdc++.
+struct test_is_unsigned {
+  static constexpr bool __is_unsigned = true; // expected-warning {{keyword '__is_unsigned' will be made available as an identifier}}
+};
+
 template struct must_be_true {};
 template<> struct must_be_true;
 
Index: clang/lib/Parse/ParseDecl.cpp
===
--- clang/lib/Parse/ParseDecl.cpp
+++ clang/lib/Parse/ParseDecl.cpp
@@ -3519,6 +3519,7 @@
 }
 
 case tok::kw___is_signed:
+case tok::kw___is_unsigned:
   // GNU libstdc++ 4.4 uses __is_signed as an identifier, but Clang
   // typically treats it as a trait. If we see __is_signed as it appears
   // in libstdc++, e.g.,
@@ -3526,8 +3527,15 @@
   //   static const bool __is_signed;
   //
   // then treat __is_signed as an identifier rather than as a keyword.
+  //
+  // Similarly, libstdc++ 13 trunk at one point used __is_unsigned as an
+  // identifier as
+  //
+  //   static constexpr bool __is_unsigned;
+  //
   if (DS.getTypeSpecType() == TST_bool &&
-  DS.getTypeQualifiers() == DeclSpec::TQ_const &&
+  (DS.getTypeQualifiers() == DeclSpec::TQ_const ||
+   DS.getConstexprSpecifier() == ConstexprSpecKind::Constexpr) &&
   DS.getStorageClassSpec() == DeclSpec::SCS_static)
 TryKeywordIdentFallback(true);
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D134529: [C++20][Clang] P2468R2 The Equality Operator You Are Looking For

[Stephan Bergmann via Phabricator via cfe-commits]

sberg added a comment.

I just ran into newly-failing

  $ cat test.cc
  struct S1 {
  bool operator==(int);
  bool operator!=(int);
  };
  struct S2;
  bool operator ==(S2, S2);
  bool f(S1 s) { return 0 == s; }

  $ clang++ -std=c++20 -fsyntax-only test.cc
  test.cc:7:25: error: invalid operands to binary expression ('int' and 'S1')
  bool f(S1 s) { return 0 == s; }
~ ^  ~
  test.cc:6:6: note: candidate function not viable: no known conversion from 
'int' to 'S2' for 1st argument
  bool operator ==(S2, S2);
   ^
  1 error generated.

It looks to me like this is correctly rejected now per P2468R2.  But it is 
rather confusing that a note mentions the `S2` candidate, while the relevant 
`S1` operators `==` and `!=` are not mentioned at all.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134529

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

[PATCH] D126864: [clang] Introduce -fstrict-flex-arrays= for stricter handling of flexible arrays

[Stephan Bergmann via Phabricator via cfe-commits]

sberg added a comment.

In D126864#3698798 , @sberg wrote:

> I'm surprised that

[...]

> causes a warning?  I would have expected it to be suppressed in this case, as 
> with the lax `-fstrict-flex-arrays=0` default, and only to hit with the 
> stricter `-fstrict-flex-arrays=2`.

appears to be addressed with https://reviews.llvm.org/D132853 "[clang] Fix 
-Warray-bound interaction with -fstrict-flex-arrays=1", thanks


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126864

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

[PATCH] D131307: [Clang] Allow downgrading to a warning the diagnostic for setting a non fixed enum to a value outside the range of the enumeration values

[Stephan Bergmann via Phabricator via cfe-commits]

sberg added a comment.

In D131307#3714629 , @shafik wrote:

> In D131307#3713011 , @sberg wrote:
>
>> With this commit,
>>
>>   $ cat test.cc
>>   #include "boost/numeric/conversion/cast.hpp"
>>   int main() { return boost::numeric_cast(0L); }
>>   
>>   $ clang++ test.cc
>>
>> succeeds without any diagnostic, while with its parent commit 
>> https://github.com/llvm/llvm-project/commit/b3645353041818f61e2580635409ddb81ff5a272
>>  " [Clang] Diagnose ill-formed constant expression when setting a non fixed 
>> enum to a value outside the range of the enumeration values" it had started 
>> to fail with
>
> Yes, that is intended. When modifying the change to allow it to be turned 
> into a warning it started applying outside of constant expression contexts 
> and that broke a lot more stuff.

I think we might be talking past each other here.  There are three commits:

1. D130058  "[Clang] Diagnose ill-formed 
constant expression when setting a non fixed enum to a value outside the range 
of the enumeration values"
2. D131307  "[Clang] Allow downgrading to a 
warning the diagnostic for setting a non fixed enum to a value outside the 
range of the enumeration values"
3. D131528  "[Clang] Restrict non fixed enum 
to a value outside the range of the enumeration values warning to context 
requiring a constant expression"

(1) had started to diagnose my reproducer as an error (and I assume it did 
rightly so).  But then (2) stopped diagnosing it at all, letting the reproducer 
compile successfully without any diagnostic (and I assume wrongly so).  (3) 
didn't make any further change regarding that behavior.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131307

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

[PATCH] D131307: [Clang] Allow downgrading to a warning the diagnostic for setting a non fixed enum to a value outside the range of the enumeration values

[Stephan Bergmann via Phabricator via cfe-commits]

sberg added a comment.

With this commit,

  $ cat test.cc
  #include "boost/numeric/conversion/cast.hpp"
  int main() { return boost::numeric_cast(0L); }
  
  $ clang++ test.cc

succeeds without any diagnostic, while with its parent commit 
https://github.com/llvm/llvm-project/commit/b3645353041818f61e2580635409ddb81ff5a272
 " [Clang] Diagnose ill-formed constant expression when setting a non fixed 
enum to a value outside the range of the enumeration values" it had started to 
fail with

  In file included from test.cc:1:
  In file included from /usr/include/boost/numeric/conversion/cast.hpp:33:
  In file included from /usr/include/boost/numeric/conversion/converter.hpp:13:
  In file included from 
/usr/include/boost/numeric/conversion/conversion_traits.hpp:13:
  In file included from 
/usr/include/boost/numeric/conversion/detail/conversion_traits.hpp:18:
  In file included from 
/usr/include/boost/numeric/conversion/detail/int_float_mixture.hpp:19:
  In file included from /usr/include/boost/mpl/integral_c.hpp:32:
  /usr/include/boost/mpl/aux_/integral_wrapper.hpp:73:31: error: non-type 
template argument is not a constant expression
  typedef AUX_WRAPPER_INST( 
BOOST_MPL_AUX_STATIC_CAST(AUX_WRAPPER_VALUE_TYPE, (value - 1)) ) prior;
  
~~^~~~
  /usr/include/boost/mpl/aux_/static_cast.hpp:24:47: note: expanded from macro 
'BOOST_MPL_AUX_STATIC_CAST'
  #   define BOOST_MPL_AUX_STATIC_CAST(T, expr) static_cast(expr)
^
  /usr/include/boost/mpl/integral_c.hpp:31:54: note: expanded from macro 
'AUX_WRAPPER_INST'
  #define AUX_WRAPPER_INST(value) AUX_WRAPPER_NAME< T, value >
   ^
  /usr/include/boost/numeric/conversion/detail/meta.hpp:30:46: note: in 
instantiation of template class 
'mpl_::integral_c' requested here
 enum { x = ( BOOST_MPL_AUX_VALUE_WKND(T1)::value == 
BOOST_MPL_AUX_VALUE_WKND(T2)::value ) };
   ^
  /usr/include/boost/mpl/if.hpp:63:68: note: in instantiation of template class 
'boost::numeric::convdetail::equal_to, 
mpl_::integral_c>' requested here
BOOST_MPL_AUX_STATIC_CAST(bool, BOOST_MPL_AUX_VALUE_WKND(T1)::value)
 ^
  /usr/include/boost/mpl/eval_if.hpp:37:22: note: in instantiation of template 
class 
'boost::mpl::if_, 
mpl_::integral_c>, 
boost::mpl::identity>, 
boost::mpl::eval_if, 
mpl_::integral_c>, 
boost::mpl::identity>>, 
boost::mpl::if_, 
mpl_::integral_c>, 
boost::mpl::identity>, boost::mpl::identity' requested here
  typedef typename if_::type f_;
   ^
  /usr/include/boost/numeric/conversion/detail/meta.hpp:81:12: note: in 
instantiation of template class 
'boost::mpl::eval_if, 
mpl_::integral_c>, 
boost::mpl::identity>, 
boost::mpl::eval_if, 
mpl_::integral_c>, 
boost::mpl::identity>>, 
boost::mpl::if_, 
mpl_::integral_c>, 
boost::mpl::identity>, boost::mpl::identity' requested here
mpl::eval_if::type
 ^
  /usr/include/boost/numeric/conversion/detail/udt_builtin_mixture.hpp:41:7: 
note: in instantiation of template class 
'boost::numeric::convdetail::ct_switch4, 
mpl_::integral_c, 
mpl_::integral_c, 
mpl_::integral_c, 
boost::numeric::convdetail::get_subranged_BuiltIn2BuiltIn, 
boost::mpl::identity>, 
boost::mpl::identity>, boost::mpl::identity>>' requested here
ct_switch4::type
  ^
  /usr/include/boost/numeric/conversion/conversion_traits.hpp:22:7: note: in 
instantiation of template class 
'boost::numeric::convdetail::non_trivial_traits_impl' requested here
  : convdetail::get_conversion_traits::type
^
  /usr/include/boost/numeric/conversion/detail/converter.hpp:584:22: note: in 
instantiation of template class 'boost::numeric::conversion_traits' 
requested here
  typedef typename Traits::trivial trivial ;
   ^
  /usr/include/boost/numeric/conversion/converter.hpp:29:32: note: in 
instantiation of template class 
'boost::numeric::convdetail::get_converter_impl, boost::numeric::def_overflow_handler, boost::numeric::Trunc, 
boost::numeric::raw_converter>, 
boost::numeric::UseInternalRangeChecker>' requested here
  struct converter : convdetail::get_converter_impl, 
boost::numeric::def_overflow_handler, boost::numeric::Trunc>' requested 
here
  return converter::convert(arg);
 ^
  test.cc:2:28: note: in instantiation of function template specialization 
'boost::numeric_cast' requested here
  int main() { return boost::numeric_cast(0L); }
 ^
  /usr/include/boost/mpl/aux_/integral_wrapper.hpp:73:31: note: integer value 
-1 is outside the valid range of values [0, 3] for this enumeration type
  typedef AUX_WRAPPER_INST( 
BOOST_MPL_AUX_STATIC_CAST(AUX_WRAPPER_VALUE_TYPE, (value - 1)) ) prior;

[PATCH] D126864: [clang] Introduce -fstrict-flex-arrays= for stricter handling of flexible arrays

[Stephan Bergmann via Phabricator via cfe-commits]

sberg added a comment.

I'm surprised that

  $ cat test.c
  struct S {
  int m1;
  int m2[1];
  };
  void f(struct S * s) {
  s->m2[1] = 0;
  }
  
  $ clang -fsyntax-only -fstrict-flex-arrays=1 test.c
  test.c:6:5: warning: array index 1 is past the end of the array (which 
contains 1 element) [-Warray-bounds]
  s->m2[1] = 0;
  ^ ~
  test.c:3:5: note: array 'm2' declared here
  int m2[1];
  ^
  1 warning generated.

causes a warning?  I would have expected it to be suppressed in this case, as 
with the lax `-fstrict-flex-arrays=0` default, and only to hit with the 
stricter `-fstrict-flex-arrays=2`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126864

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

[PATCH] D128783: [test] Check for more -fsanitize=array-bounds regressions

[Stephan Bergmann via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
sberg marked an inline comment as done.
Closed by commit rG4996e3f68315: [test] Check for more -fsanitize=array-bounds 
behavior (authored by sberg).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128783

Files:
  clang/test/CodeGen/bounds-checking-fam.c


Index: clang/test/CodeGen/bounds-checking-fam.c
===
--- clang/test/CodeGen/bounds-checking-fam.c
+++ clang/test/CodeGen/bounds-checking-fam.c
@@ -1,5 +1,6 @@
 // REQUIRES: x86-registered-target
 // RUN: %clang_cc1 -emit-llvm -triple x86_64 -fsanitize=array-bounds %s -o - | 
FileCheck %s --check-prefixes=CHECK,CHECK-STRICT-0
+// RUN: %clang_cc1 -emit-llvm -triple x86_64 -fsanitize=array-bounds -x c++ %s 
-o - | FileCheck %s --check-prefixes=CHECK,CHECK-STRICT-0,CXX,CXX-STRICT-0
 
 /// Before flexible array member was added to C99, many projects use a
 /// one-element array as the last emember of a structure as an alternative.
@@ -15,20 +16,58 @@
   int a[3];
 };
 
-// CHECK-LABEL: define {{.*}} @test_one(
+// CHECK-LABEL: define {{.*}} @{{.*}}test_one{{.*}}(
 int test_one(struct One *p, int i) {
   // CHECK-STRICT-0-NOT: @__ubsan
   return p->a[i] + (p->a)[i];
 }
 
-// CHECK-LABEL: define {{.*}} @test_two(
+// CHECK-LABEL: define {{.*}} @{{.*}}test_two{{.*}}(
 int test_two(struct Two *p, int i) {
   // CHECK-STRICT-0: call void @__ubsan_handle_out_of_bounds_abort(
   return p->a[i] + (p->a)[i];
 }
 
-// CHECK-LABEL: define {{.*}} @test_three(
+// CHECK-LABEL: define {{.*}} @{{.*}}test_three{{.*}}(
 int test_three(struct Three *p, int i) {
   // CHECK-STRICT-0: call void @__ubsan_handle_out_of_bounds_abort(
   return p->a[i] + (p->a)[i];
 }
+
+#define FLEXIBLE 1
+struct Macro {
+  int a[FLEXIBLE];
+};
+
+// CHECK-LABEL: define {{.*}} @{{.*}}test_macro{{.*}}(
+int test_macro(struct Macro *p, int i) {
+  // CHECK-STRICT-0-NOT: @__ubsan
+  return p->a[i] + (p->a)[i];
+}
+
+#if defined __cplusplus
+
+struct Base {
+  int b;
+};
+struct NoStandardLayout : Base {
+  int a[1];
+};
+
+// CXX-LABEL: define {{.*}} @{{.*}}test_nostandardlayout{{.*}}(
+int test_nostandardlayout(NoStandardLayout *p, int i) {
+  // CXX-STRICT-0-NOT: @__ubsan
+  return p->a[i] + (p->a)[i];
+}
+
+template struct Template {
+  int a[N];
+};
+
+// CXX-LABEL: define {{.*}} @{{.*}}test_template{{.*}}(
+int test_template(Template<1> *p, int i) {
+  // CXX-STRICT-0-NOT: @__ubsan
+  return p->a[i] + (p->a)[i];
+}
+
+#endif


Index: clang/test/CodeGen/bounds-checking-fam.c
===
--- clang/test/CodeGen/bounds-checking-fam.c
+++ clang/test/CodeGen/bounds-checking-fam.c
@@ -1,5 +1,6 @@
 // REQUIRES: x86-registered-target
 // RUN: %clang_cc1 -emit-llvm -triple x86_64 -fsanitize=array-bounds %s -o - | FileCheck %s --check-prefixes=CHECK,CHECK-STRICT-0
+// RUN: %clang_cc1 -emit-llvm -triple x86_64 -fsanitize=array-bounds -x c++ %s -o - | FileCheck %s --check-prefixes=CHECK,CHECK-STRICT-0,CXX,CXX-STRICT-0
 
 /// Before flexible array member was added to C99, many projects use a
 /// one-element array as the last emember of a structure as an alternative.
@@ -15,20 +16,58 @@
   int a[3];
 };
 
-// CHECK-LABEL: define {{.*}} @test_one(
+// CHECK-LABEL: define {{.*}} @{{.*}}test_one{{.*}}(
 int test_one(struct One *p, int i) {
   // CHECK-STRICT-0-NOT: @__ubsan
   return p->a[i] + (p->a)[i];
 }
 
-// CHECK-LABEL: define {{.*}} @test_two(
+// CHECK-LABEL: define {{.*}} @{{.*}}test_two{{.*}}(
 int test_two(struct Two *p, int i) {
   // CHECK-STRICT-0: call void @__ubsan_handle_out_of_bounds_abort(
   return p->a[i] + (p->a)[i];
 }
 
-// CHECK-LABEL: define {{.*}} @test_three(
+// CHECK-LABEL: define {{.*}} @{{.*}}test_three{{.*}}(
 int test_three(struct Three *p, int i) {
   // CHECK-STRICT-0: call void @__ubsan_handle_out_of_bounds_abort(
   return p->a[i] + (p->a)[i];
 }
+
+#define FLEXIBLE 1
+struct Macro {
+  int a[FLEXIBLE];
+};
+
+// CHECK-LABEL: define {{.*}} @{{.*}}test_macro{{.*}}(
+int test_macro(struct Macro *p, int i) {
+  // CHECK-STRICT-0-NOT: @__ubsan
+  return p->a[i] + (p->a)[i];
+}
+
+#if defined __cplusplus
+
+struct Base {
+  int b;
+};
+struct NoStandardLayout : Base {
+  int a[1];
+};
+
+// CXX-LABEL: define {{.*}} @{{.*}}test_nostandardlayout{{.*}}(
+int test_nostandardlayout(NoStandardLayout *p, int i) {
+  // CXX-STRICT-0-NOT: @__ubsan
+  return p->a[i] + (p->a)[i];
+}
+
+template struct Template {
+  int a[N];
+};
+
+// CXX-LABEL: define {{.*}} @{{.*}}test_template{{.*}}(
+int test_template(Template<1> *p, int i) {
+  // CXX-STRICT-0-NOT: @__ubsan
+  return p->a[i] + (p->a)[i];
+}
+
+#endif
___
cfe-commits mailing list
cfe-commits@lists.llvm.org

[PATCH] D128783: [test] Check for more -fsanitize=array-bounds regressions

[Stephan Bergmann via Phabricator via cfe-commits]

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



Comment at: clang/test/CodeGen/bounds-checking-fam.c:59
+  // CXX-STRICT-0-NOT: @__ubsan
+  return p->a[i] + (p->a)[i];
+}

serge-sans-paille wrote:
> Another C++-specific behavior: if the bound results from the substitution of 
> a template parameter, event if its value is 1 it's currently not considered a 
> FAM
I hadn't encountered that in the wild, so didn't bother to create a test for 
it; but yes, makes sense to add it too


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

https://reviews.llvm.org/D128783

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

[PATCH] D128783: [test] Check for more -fsanitize=array-bounds regressions

[Stephan Bergmann via Phabricator via cfe-commits]

sberg updated this revision to Diff 441640.
sberg added a comment.

added test involving template argument substitution


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

https://reviews.llvm.org/D128783

Files:
  clang/test/CodeGen/bounds-checking-fam.c


Index: clang/test/CodeGen/bounds-checking-fam.c
===
--- clang/test/CodeGen/bounds-checking-fam.c
+++ clang/test/CodeGen/bounds-checking-fam.c
@@ -1,5 +1,6 @@
 // REQUIRES: x86-registered-target
 // RUN: %clang_cc1 -emit-llvm -triple x86_64 -fsanitize=array-bounds %s -o - | 
FileCheck %s --check-prefixes=CHECK,CHECK-STRICT-0
+// RUN: %clang_cc1 -emit-llvm -triple x86_64 -fsanitize=array-bounds -x c++ %s 
-o - | FileCheck %s --check-prefixes=CHECK,CHECK-STRICT-0,CXX,CXX-STRICT-0
 
 /// Before flexible array member was added to C99, many projects use a
 /// one-element array as the last emember of a structure as an alternative.
@@ -15,20 +16,58 @@
   int a[3];
 };
 
-// CHECK-LABEL: define {{.*}} @test_one(
+// CHECK-LABEL: define {{.*}} @{{.*}}test_one{{.*}}(
 int test_one(struct One *p, int i) {
   // CHECK-STRICT-0-NOT: @__ubsan
   return p->a[i] + (p->a)[i];
 }
 
-// CHECK-LABEL: define {{.*}} @test_two(
+// CHECK-LABEL: define {{.*}} @{{.*}}test_two{{.*}}(
 int test_two(struct Two *p, int i) {
   // CHECK-STRICT-0: call void @__ubsan_handle_out_of_bounds_abort(
   return p->a[i] + (p->a)[i];
 }
 
-// CHECK-LABEL: define {{.*}} @test_three(
+// CHECK-LABEL: define {{.*}} @{{.*}}test_three{{.*}}(
 int test_three(struct Three *p, int i) {
   // CHECK-STRICT-0: call void @__ubsan_handle_out_of_bounds_abort(
   return p->a[i] + (p->a)[i];
 }
+
+#define FLEXIBLE 1
+struct Macro {
+  int a[FLEXIBLE];
+};
+
+// CHECK-LABEL: define {{.*}} @{{.*}}test_macro{{.*}}(
+int test_macro(struct Macro *p, int i) {
+  // CHECK-STRICT-0-NOT: @__ubsan
+  return p->a[i] + (p->a)[i];
+}
+
+#if defined __cplusplus
+
+struct Base {
+  int b;
+};
+struct NoStandardLayout : Base {
+  int a[1];
+};
+
+// CXX-LABEL: define {{.*}} @{{.*}}test_nostandardlayout{{.*}}(
+int test_nostandardlayout(NoStandardLayout *p, int i) {
+  // CXX-STRICT-0-NOT: @__ubsan
+  return p->a[i] + (p->a)[i];
+}
+
+template struct Template {
+  int a[N];
+};
+
+// CXX-LABEL: define {{.*}} @{{.*}}test_template{{.*}}(
+int test_template(Template<1> *p, int i) {
+  // CXX-STRICT-0-NOT: @__ubsan
+  return p->a[i] + (p->a)[i];
+}
+
+#endif


Index: clang/test/CodeGen/bounds-checking-fam.c
===
--- clang/test/CodeGen/bounds-checking-fam.c
+++ clang/test/CodeGen/bounds-checking-fam.c
@@ -1,5 +1,6 @@
 // REQUIRES: x86-registered-target
 // RUN: %clang_cc1 -emit-llvm -triple x86_64 -fsanitize=array-bounds %s -o - | FileCheck %s --check-prefixes=CHECK,CHECK-STRICT-0
+// RUN: %clang_cc1 -emit-llvm -triple x86_64 -fsanitize=array-bounds -x c++ %s -o - | FileCheck %s --check-prefixes=CHECK,CHECK-STRICT-0,CXX,CXX-STRICT-0
 
 /// Before flexible array member was added to C99, many projects use a
 /// one-element array as the last emember of a structure as an alternative.
@@ -15,20 +16,58 @@
   int a[3];
 };
 
-// CHECK-LABEL: define {{.*}} @test_one(
+// CHECK-LABEL: define {{.*}} @{{.*}}test_one{{.*}}(
 int test_one(struct One *p, int i) {
   // CHECK-STRICT-0-NOT: @__ubsan
   return p->a[i] + (p->a)[i];
 }
 
-// CHECK-LABEL: define {{.*}} @test_two(
+// CHECK-LABEL: define {{.*}} @{{.*}}test_two{{.*}}(
 int test_two(struct Two *p, int i) {
   // CHECK-STRICT-0: call void @__ubsan_handle_out_of_bounds_abort(
   return p->a[i] + (p->a)[i];
 }
 
-// CHECK-LABEL: define {{.*}} @test_three(
+// CHECK-LABEL: define {{.*}} @{{.*}}test_three{{.*}}(
 int test_three(struct Three *p, int i) {
   // CHECK-STRICT-0: call void @__ubsan_handle_out_of_bounds_abort(
   return p->a[i] + (p->a)[i];
 }
+
+#define FLEXIBLE 1
+struct Macro {
+  int a[FLEXIBLE];
+};
+
+// CHECK-LABEL: define {{.*}} @{{.*}}test_macro{{.*}}(
+int test_macro(struct Macro *p, int i) {
+  // CHECK-STRICT-0-NOT: @__ubsan
+  return p->a[i] + (p->a)[i];
+}
+
+#if defined __cplusplus
+
+struct Base {
+  int b;
+};
+struct NoStandardLayout : Base {
+  int a[1];
+};
+
+// CXX-LABEL: define {{.*}} @{{.*}}test_nostandardlayout{{.*}}(
+int test_nostandardlayout(NoStandardLayout *p, int i) {
+  // CXX-STRICT-0-NOT: @__ubsan
+  return p->a[i] + (p->a)[i];
+}
+
+template struct Template {
+  int a[N];
+};
+
+// CXX-LABEL: define {{.*}} @{{.*}}test_template{{.*}}(
+int test_template(Template<1> *p, int i) {
+  // CXX-STRICT-0-NOT: @__ubsan
+  return p->a[i] + (p->a)[i];
+}
+
+#endif
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D128783: [test] Check for more -fsanitize=array-bounds regressions

[Stephan Bergmann via Phabricator via cfe-commits]

sberg updated this revision to Diff 441313.
sberg added a comment.

Updated the prospective git commit message as follow:

  [test] Check for more -fsanitize=array-bounds behavior
  
  ...that had temporarily regressed with (since reverted)
  

  "[clang] Introduce -fstrict-flex-arrays= for stricter handling of flexible
  arrays", and had then been seen to cause issues in the wild:
  
  For one, the HarfBuzz project has various "fake" flexible array members of the
  form
  
  > TypearrayZ[HB_VAR_ARRAY];
  
  in , where
  HB_VAR_ARRAY is a macro defined as
  
  > #ifndef HB_VAR_ARRAY
  > #define HB_VAR_ARRAY 1
  > #endif
  
  in .
  
  For another, the Firebird project in
   
uses
  a trailing member
  
  > srq lhb_hash[1];// Hash table
  
  as a "fake" flexible array, but declared in a
  
  > struct lhb : public Firebird::MemoryHeader
  
  that is not a standard-layout class (because the Firebird::MemoryHeader base
  class also declares non-static data members).
  
  (The second case is specific to C++.  Extend the test setup so that all the
  other tests are now run for both C and C++, just in case the behavior could 
ever
  start to diverge for those two languages.)
  
  Differential Revision: https://reviews.llvm.org/D128783


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

https://reviews.llvm.org/D128783

Files:
  clang/test/CodeGen/bounds-checking-fam.c


Index: clang/test/CodeGen/bounds-checking-fam.c
===
--- clang/test/CodeGen/bounds-checking-fam.c
+++ clang/test/CodeGen/bounds-checking-fam.c
@@ -1,5 +1,6 @@
 // REQUIRES: x86-registered-target
 // RUN: %clang_cc1 -emit-llvm -triple x86_64 -fsanitize=array-bounds %s -o - | 
FileCheck %s --check-prefixes=CHECK,CHECK-STRICT-0
+// RUN: %clang_cc1 -emit-llvm -triple x86_64 -fsanitize=array-bounds -x c++ %s 
-o - | FileCheck %s --check-prefixes=CHECK,CHECK-STRICT-0,CXX,CXX-STRICT-0
 
 /// Before flexible array member was added to C99, many projects use a
 /// one-element array as the last emember of a structure as an alternative.
@@ -14,21 +15,48 @@
 struct Three {
   int a[3];
 };
+#define FLEXIBLE 1
+struct Macro {
+  int a[FLEXIBLE];
+};
 
-// CHECK-LABEL: define {{.*}} @test_one(
+// CHECK-LABEL: define {{.*}} @{{.*}}test_one{{.*}}(
 int test_one(struct One *p, int i) {
   // CHECK-STRICT-0-NOT: @__ubsan
   return p->a[i] + (p->a)[i];
 }
 
-// CHECK-LABEL: define {{.*}} @test_two(
+// CHECK-LABEL: define {{.*}} @{{.*}}test_two{{.*}}(
 int test_two(struct Two *p, int i) {
   // CHECK-STRICT-0: call void @__ubsan_handle_out_of_bounds_abort(
   return p->a[i] + (p->a)[i];
 }
 
-// CHECK-LABEL: define {{.*}} @test_three(
+// CHECK-LABEL: define {{.*}} @{{.*}}test_three{{.*}}(
 int test_three(struct Three *p, int i) {
   // CHECK-STRICT-0: call void @__ubsan_handle_out_of_bounds_abort(
   return p->a[i] + (p->a)[i];
 }
+
+// CHECK-LABEL: define {{.*}} @{{.*}}test_macro{{.*}}(
+int test_macro(struct Macro *p, int i) {
+  // CHECK-STRICT-0-NOT: @__ubsan
+  return p->a[i] + (p->a)[i];
+}
+
+#if defined __cplusplus
+
+struct Base {
+  int b;
+};
+struct NoStandardLayout : Base {
+  int a[1];
+};
+
+// CXX-LABEL: define {{.*}} @{{.*}}test_nostandardlayout{{.*}}(
+int test_nostandardlayout(NoStandardLayout *p, int i) {
+  // CXX-STRICT-0-NOT: @__ubsan
+  return p->a[i] + (p->a)[i];
+}
+
+#endif


Index: clang/test/CodeGen/bounds-checking-fam.c
===
--- clang/test/CodeGen/bounds-checking-fam.c
+++ clang/test/CodeGen/bounds-checking-fam.c
@@ -1,5 +1,6 @@
 // REQUIRES: x86-registered-target
 // RUN: %clang_cc1 -emit-llvm -triple x86_64 -fsanitize=array-bounds %s -o - | FileCheck %s --check-prefixes=CHECK,CHECK-STRICT-0
+// RUN: %clang_cc1 -emit-llvm -triple x86_64 -fsanitize=array-bounds -x c++ %s -o - | FileCheck %s --check-prefixes=CHECK,CHECK-STRICT-0,CXX,CXX-STRICT-0
 
 /// Before flexible array member was added to C99, many projects use a
 /// one-element array as the last emember of a structure as an alternative.
@@ -14,21 +15,48 @@
 struct Three {
   int a[3];
 };
+#define FLEXIBLE 1
+struct Macro {
+  int a[FLEXIBLE];
+};
 
-// CHECK-LABEL: define {{.*}} @test_one(
+// CHECK-LABEL: define {{.*}} @{{.*}}test_one{{.*}}(
 int test_one(struct One *p, int i) {
   // CHECK-STRICT-0-NOT: @__ubsan
   return p->a[i] + (p->a)[i];
 }
 
-// CHECK-LABEL: define {{.*}} @test_two(
+// CHECK-LABEL: define {{.*}} @{{.*}}test_two{{.*}}(
 int test_two(struct Two *p, int i) {
   // CHECK-STRICT-0: call void @__ubsan_handle_out_of_bounds_abort(
   return p->a[i] + (p->a)[i];
 }
 

[PATCH] D126864: [clang] Introduce -fstrict-flex-arrays= for stricter handling of flexible arrays

[Stephan Bergmann via Phabricator via cfe-commits]

sberg added a comment.

In D126864#3614297 , @MaskRay wrote:

> Oh, I did not see this when pushing a test 
> efd90ffbfc427ad4c4675ac1fcae9d53cc7f1322 
>  . 
> Consider adding additional tests in 
> `clang/test/CodeGen/bounds-checking-fam.c`.

see https://reviews.llvm.org/D128783 "Check for more -fsanitize=array-bounds 
regressions"

> It's worth bringing up such issue to the project. GCC and Clang have been 
> working around them so far but they should eventually be fixed.

All the issues I encountered were in C++ code, and I'm not sure replacing one 
non-standard behavior (treating trailing length-one arrays as flexible) with 
another (C flexible array members) is something I feel confident to suggest to 
those projects.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126864

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

[PATCH] D128783: Check for more -fsanitize=array-bounds regressions

[Stephan Bergmann via Phabricator via cfe-commits]

sberg created this revision.
sberg added a reviewer: MaskRay.
sberg added a project: clang.
Herald added a subscriber: StephenFan.
Herald added a project: All.
sberg requested review of this revision.

...that had been introduced with (since reverted) 
https://github.com/llvm/llvm-project/commit/886715af962de2c92fac4bd37104450345711e4a
 "[clang] Introduce -fstrict-flex-arrays= for stricter handling of flexible 
arrays", and caused issues in the wild:

For one, the HarfBuzz project has various "fake" flexible array members of the 
form

  TypearrayZ[HB_VAR_ARRAY];

in https://github.com/harfbuzz/harfbuzz/blob/main/src/hb-open-type.hh, where 
`HB_VAR_ARRAY` is a macro defined as

  #ifndef HB_VAR_ARRAY
  #define HB_VAR_ARRAY 1
  #endif

in https://github.com/harfbuzz/harfbuzz/blob/main/src/hb-machinery.hh.

For another, the Firebird project in 
https://github.com/FirebirdSQL/firebird/blob/master/src/lock/lock_proto.h uses 
a trailing member

  srq lhb_hash[1];// Hash table

as a "fake" flexible array, but declared in a

  struct lhb : public Firebird::MemoryHeader

that is not a standard-layout class (because the `Firebird::MemoryHeader` base 
class also declares non-static data members).

(Checking for the second case required changing the test file from C to C++.)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D128783

Files:
  clang/test/CodeGen/bounds-checking-fam.c
  clang/test/CodeGen/bounds-checking-fam.cpp


Index: clang/test/CodeGen/bounds-checking-fam.cpp
===
--- clang/test/CodeGen/bounds-checking-fam.cpp
+++ clang/test/CodeGen/bounds-checking-fam.cpp
@@ -14,21 +14,43 @@
 struct Three {
   int a[3];
 };
+#define FLEXIBLE 1
+struct Macro {
+  int a[FLEXIBLE];
+};
+struct Base {
+  int b;
+};
+struct NoStandardLayout : Base {
+  int a[1];
+};
 
-// CHECK-LABEL: define {{.*}} @test_one(
-int test_one(struct One *p, int i) {
+// CHECK-LABEL: define {{.*}} @{{.*}}test_one{{.*}}(
+int test_one(One *p, int i) {
   // CHECK-STRICT-0-NOT: @__ubsan
   return p->a[i] + (p->a)[i];
 }
 
-// CHECK-LABEL: define {{.*}} @test_two(
-int test_two(struct Two *p, int i) {
+// CHECK-LABEL: define {{.*}} @{{.*}}test_two{{.*}}(
+int test_two(Two *p, int i) {
   // CHECK-STRICT-0: call void @__ubsan_handle_out_of_bounds_abort(
   return p->a[i] + (p->a)[i];
 }
 
-// CHECK-LABEL: define {{.*}} @test_three(
-int test_three(struct Three *p, int i) {
+// CHECK-LABEL: define {{.*}} @{{.*}}test_three{{.*}}(
+int test_three(Three *p, int i) {
   // CHECK-STRICT-0: call void @__ubsan_handle_out_of_bounds_abort(
   return p->a[i] + (p->a)[i];
 }
+
+// CHECK-LABEL: define {{.*}} @{{.*}}test_macro{{.*}}(
+int test_macro(Macro *p, int i) {
+  // CHECK-STRICT-0-NOT: @__ubsan
+  return p->a[i] + (p->a)[i];
+}
+
+// CHECK-LABEL: define {{.*}} @{{.*}}test_nostandardlayout{{.*}}(
+int test_nostandardlayout(NoStandardLayout *p, int i) {
+  // CHECK-STRICT-0-NOT: @__ubsan
+  return p->a[i] + (p->a)[i];
+}


Index: clang/test/CodeGen/bounds-checking-fam.cpp
===
--- clang/test/CodeGen/bounds-checking-fam.cpp
+++ clang/test/CodeGen/bounds-checking-fam.cpp
@@ -14,21 +14,43 @@
 struct Three {
   int a[3];
 };
+#define FLEXIBLE 1
+struct Macro {
+  int a[FLEXIBLE];
+};
+struct Base {
+  int b;
+};
+struct NoStandardLayout : Base {
+  int a[1];
+};
 
-// CHECK-LABEL: define {{.*}} @test_one(
-int test_one(struct One *p, int i) {
+// CHECK-LABEL: define {{.*}} @{{.*}}test_one{{.*}}(
+int test_one(One *p, int i) {
   // CHECK-STRICT-0-NOT: @__ubsan
   return p->a[i] + (p->a)[i];
 }
 
-// CHECK-LABEL: define {{.*}} @test_two(
-int test_two(struct Two *p, int i) {
+// CHECK-LABEL: define {{.*}} @{{.*}}test_two{{.*}}(
+int test_two(Two *p, int i) {
   // CHECK-STRICT-0: call void @__ubsan_handle_out_of_bounds_abort(
   return p->a[i] + (p->a)[i];
 }
 
-// CHECK-LABEL: define {{.*}} @test_three(
-int test_three(struct Three *p, int i) {
+// CHECK-LABEL: define {{.*}} @{{.*}}test_three{{.*}}(
+int test_three(Three *p, int i) {
   // CHECK-STRICT-0: call void @__ubsan_handle_out_of_bounds_abort(
   return p->a[i] + (p->a)[i];
 }
+
+// CHECK-LABEL: define {{.*}} @{{.*}}test_macro{{.*}}(
+int test_macro(Macro *p, int i) {
+  // CHECK-STRICT-0-NOT: @__ubsan
+  return p->a[i] + (p->a)[i];
+}
+
+// CHECK-LABEL: define {{.*}} @{{.*}}test_nostandardlayout{{.*}}(
+int test_nostandardlayout(NoStandardLayout *p, int i) {
+  // CHECK-STRICT-0-NOT: @__ubsan
+  return p->a[i] + (p->a)[i];
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D128643: [clang] -fsanitize=array-bounds: treat all trailing size-1 arrays as flexible

[Stephan Bergmann via Phabricator via cfe-commits]

sberg abandoned this revision.
sberg added a comment.

I'm abandoning this as the underlying https://reviews.llvm.org/D126864 "[clang] 
Introduce -fstrict-flex-arrays= for stricter handling of flexible arrays" 
has been reverted for now.

I documented all my issues with that underlying commit (including, but not 
limited to what was presented here) in my comment at 
https://reviews.llvm.org/D126864#3614235.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128643

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

[PATCH] D126864: [clang] Introduce -fstrict-flex-arrays= for stricter handling of flexible arrays

[Stephan Bergmann via Phabricator via cfe-commits]

sberg added a comment.

In D126864#3612149 , @sberg wrote:

> see https://reviews.llvm.org/D128643 "[clang] -fsanitize=array-bounds: treat 
> all trailing size-1 arrays as flexible" for another regression

For the record (now that the original commit has been reverted anyway for now):

Besides the above regression for `-fsanitize=array-bounds` and macro'ized array 
size in HarfBuzz, I also ran into yet another regression when Firebird is built 
with `-fsanitize=array-bounds`:  That project, in 
https://github.com/FirebirdSQL/firebird/blob/master/src/lock/lock_proto.h uses 
a trailing member

srq lhb_hash[1];// Hash table

as a flexible array, but declared in a

  struct lhb : public Firebird::MemoryHeader

that is not a standard-layout class (because the `Firebird::MemoryHeader` base 
class also declares non-static data members).  And `isFlexibleArrayMember` 
returns `false` if the surrounding class is not a standard-layout class.

In the end, what brought back a successful `-fsanitize=array-bounds` 
LibreOffice (which bundles those HarfBuzz and Firebird, among others) for me 
was to exclude the whole set of blocks

  // Don't consider sizes resulting from macro expansions or template argument
  // substitution to form C89 tail-padded arrays.
  
  TypeSourceInfo *TInfo = FD->getTypeSourceInfo();
  while (TInfo) {
TypeLoc TL = TInfo->getTypeLoc();
// Look through typedefs.
if (TypedefTypeLoc TTL = TL.getAs()) {
  const TypedefNameDecl *TDL = TTL.getTypedefNameDecl();
  TInfo = TDL->getTypeSourceInfo();
  continue;
}
if (ConstantArrayTypeLoc CTL = TL.getAs()) {
  const Expr *SizeExpr = dyn_cast(CTL.getSizeExpr());
  if (!SizeExpr || SizeExpr->getExprLoc().isMacroID())
return false;
}
break;
  }
  
  const ObjCInterfaceDecl *ID =
  dyn_cast(FD->getDeclContext());
  const RecordDecl *RD = dyn_cast(FD->getDeclContext());
  if (RD) {
if (RD->isUnion())
  return false;
if (const CXXRecordDecl *CRD = dyn_cast(RD)) {
  if (!CRD->isStandardLayout())
return false;
}
  } else if (!ID)
return false;

(by means of an additional `bool` parameter) when `isFlexibleArrayMember` is 
called from `getArrayIndexingBound` (in `clang/lib/CodeGen/CGExpr.cpp`).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126864

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

[PATCH] D128643: [clang] -fsanitize=array-bounds: treat all trailing size-1 arrays as flexible

[Stephan Bergmann via Phabricator via cfe-commits]

sberg created this revision.
sberg added a reviewer: serge-sans-paille.
sberg added a project: clang.
Herald added a project: All.
sberg requested review of this revision.

...even if the size resulted from a macro expansion.  This reverts back to the 
behavior prior to
https://github.com/llvm/llvm-project/commit/886715af962de2c92fac4bd37104450345711e4a
 "[clang] Introduce -fstrict-flex-arrays= for stricter handling of flexible 
arrays".  The new behavior caused false out-of-bounds-index reports from e.g. 
HarfBuzz built with `-fsanitize=array-bounds`:  HarfBuzz has various "fake" 
flexible array members of the form

  TypearrayZ[HB_VAR_ARRAY];

in https://github.com/harfbuzz/harfbuzz/blob/main/src/hb-open-type.hh, where 
`HB_VAR_ARRAY` is defined as

  #ifndef HB_VAR_ARRAY
  #define HB_VAR_ARRAY 1
  #endif

in https://github.com/harfbuzz/harfbuzz/blob/main/src/hb-machinery.hh.

Also added a test.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D128643

Files:
  clang/include/clang/AST/Expr.h
  clang/lib/AST/Expr.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/bounds-checking.c


Index: clang/test/CodeGen/bounds-checking.c
===
--- clang/test/CodeGen/bounds-checking.c
+++ clang/test/CodeGen/bounds-checking.c
@@ -58,3 +58,14 @@
// CHECK-NOT: cont:
return b[i];
 }
+
+#define FLEXIBLE 1
+struct Macro {
+  int a[FLEXIBLE];
+};
+
+// CHECK-LABEL: define {{.*}} @f7
+int f7(struct Macro *m, int i) {
+  // CHECK-NOT: call {{.*}} @llvm.ubsantrap
+  return m->a[i];
+}
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -15869,8 +15869,8 @@
   return;
 
 // Also don't warn for flexible array members.
-if (BaseExpr->isFlexibleArrayMember(Context,
-getLangOpts().StrictFlexArrays))
+if (BaseExpr->isFlexibleArrayMember(Context, 
getLangOpts().StrictFlexArrays,
+true))
   return;
 
 // Suppress the warning if the subscript expression (as identified by the
Index: clang/lib/CodeGen/CGExpr.cpp
===
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -932,7 +932,7 @@
   if (const auto *CE = dyn_cast(Base)) {
 if (CE->getCastKind() == CK_ArrayToPointerDecay &&
 !CE->getSubExpr()->IgnoreParens()->isFlexibleArrayMember(
-Context, std::max(StrictFlexArraysLevel, 1))) {
+Context, std::max(StrictFlexArraysLevel, 1), false)) {
   IndexedType = CE->getSubExpr()->getType();
   const ArrayType *AT = IndexedType->castAsArrayTypeUnsafe();
   if (const auto *CAT = dyn_cast(AT))
Index: clang/lib/AST/Expr.cpp
===
--- clang/lib/AST/Expr.cpp
+++ clang/lib/AST/Expr.cpp
@@ -203,8 +203,8 @@
   return false;
 }
 
-bool Expr::isFlexibleArrayMember(ASTContext ,
- int StrictFlexArraysLevel) const {
+bool Expr::isFlexibleArrayMember(ASTContext , int StrictFlexArraysLevel,
+ bool IgnoreSizeFromMacro) const {
   const NamedDecl *ND = nullptr;
   if (const DeclRefExpr *DRE = dyn_cast(this))
 ND = DRE->getDecl();
@@ -260,7 +260,8 @@
 }
 if (ConstantArrayTypeLoc CTL = TL.getAs()) {
   const Expr *SizeExpr = dyn_cast(CTL.getSizeExpr());
-  if (!SizeExpr || SizeExpr->getExprLoc().isMacroID())
+  if (!SizeExpr ||
+  (IgnoreSizeFromMacro && SizeExpr->getExprLoc().isMacroID()))
 return false;
 }
 break;
Index: clang/include/clang/AST/Expr.h
===
--- clang/include/clang/AST/Expr.h
+++ clang/include/clang/AST/Expr.h
@@ -451,7 +451,8 @@
   /// - 1 => [0], [1], [ ]
   /// - 2 => [0], [ ]
   /// - 3 => [ ]
-  bool isFlexibleArrayMember(ASTContext , int StrictFlexArraysLevel) const;
+  bool isFlexibleArrayMember(ASTContext , int StrictFlexArraysLevel,
+ bool IgnoreSizeFromMacro) const;
 
   /// setValueKind - Set the value kind produced by this expression.
   void setValueKind(ExprValueKind Cat) { ExprBits.ValueKind = Cat; }


Index: clang/test/CodeGen/bounds-checking.c
===
--- clang/test/CodeGen/bounds-checking.c
+++ clang/test/CodeGen/bounds-checking.c
@@ -58,3 +58,14 @@
 	// CHECK-NOT: cont:
 	return b[i];
 }
+
+#define FLEXIBLE 1
+struct Macro {
+  int a[FLEXIBLE];
+};
+
+// CHECK-LABEL: define {{.*}} @f7
+int f7(struct Macro *m, int i) {
+  // CHECK-NOT: call {{.*}} @llvm.ubsantrap
+  return m->a[i];
+}
Index: clang/lib/Sema/SemaChecking.cpp
===
--- 

[PATCH] D126864: [clang] Introduce -fstrict-flex-arrays= for stricter handling of flexible arrays

[Stephan Bergmann via Phabricator via cfe-commits]

sberg added a comment.

see https://reviews.llvm.org/D128643 "[clang] -fsanitize=array-bounds: treat 
all trailing size-1 arrays as flexible" for another regression


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

https://reviews.llvm.org/D126864

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

[PATCH] D127593: [clang] Fix trivially copyable for copy constructor and copy assignment operator

[Stephan Bergmann via Phabricator via cfe-commits]

sberg added a comment.

In D127593#3598020 , @royjacobson 
wrote:

> Is it possible to check how often PDFium uses `__has_trivial_assign`? 
> Depending on how large this breakage is we might need to revert and proceed 
> more carefully..

The setup is a bit convoluted:  I ran into this when building LibreOffice with 
clang-cl on Windows.  LibreOffice builds a bundled PDFium (where it obtained 
the PDFium source tarball, including PDFium's bundled `third-party/abseil-cpp`, 
with a recipe documented at 
https://git.libreoffice.org/core/+/71b952340726190d1f178ef0dadfa89677f2c1dd/external/pdfium/README#6).
  That PDFium in turn contains a bundled `third-party/abseil-cpp`, whose 
`absl/meta/type_traits.h` defines an `absl::is_trivially_copy_assignable` 
struct that is implemented via `__has_trivial_assign`, but then in its 
implementation also does a `static_assert` to verify that its behavior (based 
on `__has_trivial_assign`) matches the behavior of 
std::is_trivially_copy_assignable` (which is based on 
`__is_trivially_assignable`).  And that `static_assert` started to fail now 
with clang-cl on Windows against the MSVC standard library:

  In file included from 
C:/lo-clang/core/workdir/UnpackedTarball/pdfium/core/fxge/win32/cgdi_plus_ext.cpp:19:
  In file included from 
C:/lo-clang/core/workdir/UnpackedTarball/pdfium\core/fxcrt/fx_string.h:14:
  In file included from 
C:/lo-clang/core/workdir/UnpackedTarball/pdfium\core/fxcrt/bytestring.h:23:
  In file included from 
C:/lo-clang/core/workdir/UnpackedTarball/pdfium\core/fxcrt/string_view_template.h:18:
  In file included from 
C:/lo-clang/core/workdir/UnpackedTarball/pdfium\third_party/abseil-cpp/absl/types/optional.h:39:
  In file included from 
C:/lo-clang/core/workdir/UnpackedTarball/pdfium/third_party/abseil-cpp\absl/utility/utility.h:51:
  
C:/lo-clang/core/workdir/UnpackedTarball/pdfium/third_party/abseil-cpp\absl/meta/type_traits.h(501,3):
 error: static_assert failed due to requirement 'compliant || 
std::is_trivially_copy_assignable>::value' "Not compliant with std::is_trivially_copy_assignable; Standard: 
false, Implementation: true"
static_assert(compliant || std::is_trivially_copy_assignable::value,
^ 
  
C:/lo-clang/core/workdir/UnpackedTarball/pdfium/third_party/abseil-cpp\absl/types/internal/optional.h(175,21):
 note: in instantiation of template class 
'absl::is_trivially_copy_assignable>' requested here
absl::is_trivially_copy_assignable>' required here
  class optional : private optional_internal::optional_data,
  ^~~~
  
C:/lo-clang/core/workdir/UnpackedTarball/pdfium/core/fxge/win32/cgdi_plus_ext.cpp(384,43):
 note: in instantiation of template class 'absl::optional>' requested here
  absl::optional> IsSmallTriangle(
^

Both the PDFium tarball bundled by LibreOffice, and the `third-party/abseil` in 
turn bundled in that PDFium tarball, may be a bit dated, but I checked that 
PDFium's recent main branch 
https://pdfium.googlesource.com/pdfium/+/2c495300230ed67f87302716c8b262a146ae26af/core/fxge/win32/cgdi_plus_ext.cpp#375
 still uses `absl::optional>`, and Abseil's recent 
master branch 
https://github.com/abseil/abseil-cpp/blob/42f22a28401c952f1fc5942231c7fdac80811bf5/absl/meta/type_traits.h#L264
 definition of `absl::is_trivially_copy_assignable` still works as described 
above.

In general, there's still a handful of `__has_trivial_*` calls across Abseil's 
recent master branch `absl/meta/type_traits.h`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127593

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

[PATCH] D127593: [clang] Fix trivially copyable for copy constructor and copy assignment operator

[Stephan Bergmann via Phabricator via cfe-commits]

sberg added a comment.

In D127593#3596883 , @royjacobson 
wrote:

> In D127593#3596601 , @sberg wrote:
>
>> Is it intended that a deleted copy assignment op as in `struct S { void 
>> operator =(S &) = delete; };` (though, somewhat oddly, not in `struct S { 
>> void operator =(S) = delete; };`) is now marked as trivial?
>>
>> I think that's the root cause why a build of PDFium with clang-cl against 
>> the MSVC standard library started to fail for me now, effectively 
>> complaining that `__is_trivially_assignable(std::pair &, 
>> std::pair const &)` is false (as expected) while 
>> `__has_trivial_assign(std::pair)` is (unexpectedly) true.
>
> I don't see it on godbolt trunk: https://godbolt.org/z/KPfxWqnhd.

You'd only see it when compiling with Clang against MSVC's ``, where 
`std::pair` has a deleted copy assignment op (something I approximated with the 
`struct S` above).  https://godbolt.org/z/bbEqvosvP shows how the behavior 
changed in Clang trunk. (Interestingly, the behavior is different between MSVC 
and GCC, and Clang now changed from matching the MSVC behavior to matching the 
GCC one.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127593

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

[PATCH] D127593: [clang] Fix trivially copyable for copy constructor and copy assignment operator

[Stephan Bergmann via Phabricator via cfe-commits]

sberg added a comment.

Is it intended that a deleted copy assignment op as in `struct S { void 
operator =(S &) = delete; };` (though, somewhat oddly, not in `struct S { void 
operator =(S) = delete; };`) is now marked as trivial?

I think that's the root cause why a build of PDFium with clang-cl against the 
MSVC standard library started to fail for me now, effectively complaining that 
`__is_trivially_assignable(std::pair &, std::pair const &)` 
is false (as expected) while `__has_trivial_assign(std::pair)` is 
(unexpectedly) true.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127593

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

[PATCH] D117977: [cmake] Don't export `LLVM_TOOLS_INSTALL_DIR` anymore

[Stephan Bergmann via Phabricator via cfe-commits]

sberg added a comment.

There might be something fishy with this commit.  My fresh from-scratch `cmake 
--build . && cmake --build . --target install` monorepo build now lacked e.g. 
the `bin/clang++` symlink in the installation dir, and locally reverting this 
commit fixed that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117977

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

[PATCH] D119721: [clang][lex] Use `ConstSearchDirIterator` in lookup cache

[Stephan Bergmann via Phabricator via cfe-commits]

sberg added subscribers: rnk, sberg.
sberg added a comment.
Herald added a project: All.

I started to experience Clang crashing when building Firebird (as part of 
building LibreOffice) in clang-cl mode on Windows, and I think it is due to 
this change in combination with D2733  by @rnk:

When `HeaderSearch::LookupFile` exits through one of the `if 
(checkMSVCHeaderSearch...` branches introduced in D2733 
, it does not update `CacheLookup.HitIt` 
(through one of the calls to `cacheLookupSuccess` or via the "Otherwise, didn't 
find it" step at the end of the function), even though it already may have done 
the "We will fill in our found location below, so prime the start point value" 
step of calling `CacheLookup.reset`.  That means that a later call to 
`HeaderSearch::LookupFile` can go into the `if (!SkipCache && 
CacheLookup.StartIt == NextIt)` true branch, set `It = CacheLookup.HitIt` to an 
invalid iterator (the default `HitIt = nullptr` value), and then assert/crash 
when dereferencing that invalid `It`.

Which was not an issue before this change, as the initial `HitIdx = 0` was not 
an invalid value, so `i = CacheLookup.HitIdx` never caused `i` to become 
invalid.

I locally worked around this in my build with

  -  if (!SkipCache && CacheLookup.StartIt == NextIt) {
  +  if (!SkipCache && CacheLookup.StartIt == NextIt && CacheLookup.HitIt) {

which seems to work well, but I'm not sure what the best real fix would be.  
(D2733  stated "I believe the correct behavior 
here is to avoid updating the cache when we find the header via MSVC's search 
rules.")


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119721

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

[PATCH] D115715: [clang-tidy] Fix llvm-header-guard for Windows paths containing drive letter (e.g. C:).

[Stephan Bergmann via Phabricator via cfe-commits]

sberg added a comment.

> So the check, for a file called e.g. "C:\test\test.h" would suggest the guard 
> C:_TEST_TEST_H being an invalid name due to the presence of the colon.

...though the new suggestion `C__TEST_TEST_H` isn't ideal either, in that it 
uses an identifier that is reserved for the implementation (due to the double 
underscore)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115715

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

[PATCH] D108567: Implement #pragma clang final extension

[Stephan Bergmann via Phabricator via cfe-commits]

sberg added a comment.

> Final macros will always warn on redefinition, including situations with 
> identical bodies and in system headers.

But

  #define NULL syntax error
  #pragma clang final(NULL)
  #include 
  void * p = NULL;

does not produce any warnings/errors for me and indicates that NULL silently 
gets redefined as a null pointer constant in stddef.h.  Is that intended?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108567

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

[PATCH] D110310: State the kind of VerifyDiagnosticConsumer regex syntax (NFC)

[Stephan Bergmann via Phabricator via cfe-commits]

sberg created this revision.
sberg added a project: clang.
sberg requested review of this revision.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D110310

Files:
  clang/include/clang/Frontend/VerifyDiagnosticConsumer.h


Index: clang/include/clang/Frontend/VerifyDiagnosticConsumer.h
===
--- clang/include/clang/Frontend/VerifyDiagnosticConsumer.h
+++ clang/include/clang/Frontend/VerifyDiagnosticConsumer.h
@@ -157,7 +157,8 @@
 /// In this example, the diagnostic may appear only once, if at all.
 ///
 /// Regex matching mode may be selected by appending '-re' to type and
-/// including regexes wrapped in double curly braces in the directive, such as:
+/// including regexes (using POSIX extended regular expression syntax) wrapped
+/// in double curly braces in the directive, such as:
 ///
 /// \code
 ///   expected-error-re {{format specifies type 'wchar_t **' (aka '{{.+}}')}}


Index: clang/include/clang/Frontend/VerifyDiagnosticConsumer.h
===
--- clang/include/clang/Frontend/VerifyDiagnosticConsumer.h
+++ clang/include/clang/Frontend/VerifyDiagnosticConsumer.h
@@ -157,7 +157,8 @@
 /// In this example, the diagnostic may appear only once, if at all.
 ///
 /// Regex matching mode may be selected by appending '-re' to type and
-/// including regexes wrapped in double curly braces in the directive, such as:
+/// including regexes (using POSIX extended regular expression syntax) wrapped
+/// in double curly braces in the directive, such as:
 ///
 /// \code
 ///   expected-error-re {{format specifies type 'wchar_t **' (aka '{{.+}}')}}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D106924: [Preprocessor] -E -P: Ensure newline after 8 skipped lines.

[Stephan Bergmann via Phabricator via cfe-commits]

sberg added a comment.

In D106924#2919406 , @sberg wrote:

> Bisecting shows that this commit causes
>
>   $ echo foo | clang -E -P -
>   
>   foo
>
> to emit a leading newline now.
>
> That breaks e.g. LibreOffice's `configure`, which uses `echo 
> __clang_version__ | clang -E -P -` to obtain the value of that macro, and 
> expects a single line of output.

I now filed https://bugs.llvm.org/show_bug.cgi?id=51616 "Bad newlines in -E -P 
output after https://reviews.llvm.org/D106924 '[Preprocessor] -E -P: Ensure 
newline after 8 skipped lines.'"


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106924

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

[PATCH] D106924: [Preprocessor] -E -P: Ensure newline after 8 skipped lines.

[Stephan Bergmann via Phabricator via cfe-commits]

sberg added a comment.

Bisecting shows that this commit causes

  $ echo foo | clang -E -P -
  
  foo

to emit a leading newline now.

That breaks e.g. LibreOffice's `configure`, which uses `echo __clang_version__ 
| clang -E -P -` to obtain the value of that macro, and expects a single line 
of output.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106924

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

[PATCH] D99005: [clang] Implement P2266 Simpler implicit move

[Stephan Bergmann via Phabricator via cfe-commits]

sberg added a comment.

In D99005#2843201 , @mizvekov wrote:

> I have talked to the MSVC STL maintainers. They would be open to a pull 
> request for fixing this.
> The repo can be found at https://github.com/microsoft/STL
> Be my guest if you want to submit that fix yourself.
> If not, I can probably do it in the near future.

Thanks a lot for the link.  Filed https://github.com/microsoft/STL/pull/2025 
"Adapt some return statements to P2266R1 'Simpler implicit move'" now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99005

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

[PATCH] D99005: [clang] Implement P2266 Simpler implicit move

[Stephan Bergmann via Phabricator via cfe-commits]

sberg added a comment.

And, again for the record, with a build of LibreOffice with clang-cl (and 
`-Xclang -std=c++2b`) on Windows, at least against the C++ standard library 
from Visual Studio 2019 version 16.20.2, I ran into two issues in the standard 
library itself, when using `std::getline` and `std::istream::operator>>`:

  In file included from 
C:/lo-clang/core/setup_native/source/win32/customactions/reg_dlls/reg_dlls.cxx:12:
  
C:/PROGRA~2/MIB055~1/2019/COMMUN~1/VC/Tools/MSVC/1429~1.300/Include\string(66,12):
 error: non-const lvalue reference to type 'basic_istream<...>' cannot bind to 
a temporary of type 'basic_istream<...>'
  return _Istr;
 ^
  
C:/PROGRA~2/MIB055~1/2019/COMMUN~1/VC/Tools/MSVC/1429~1.300/Include\string(78,12):
 note: in instantiation of function template specialization 
'std::getline, std::allocator>' 
requested here
  return getline(_STD move(_Istr), _Str, _Delim);
 ^
  
C:/lo-clang/core/setup_native/source/win32/customactions/reg_dlls/reg_dlls.cxx(197,17):
 note: in instantiation of function template specialization 
'std::getline, std::allocator>' 
requested here
  while (std::getline(ss, sToken, L'|'))
  ^
  1 error generated.
  make[1]: *** [C:/lo-clang/core/solenv/gbuild/LinkTarget.mk:298: 
C:/lo-clang/core/workdir/CxxObject/setup_native/source/win32/customactions/reg_dlls/reg_dlls.o]
 Error 1
  
  c:/Program Files (x86)/Microsoft Visual 
Studio/2019/Community/VC/Tools/MSVC/14.29.30037/include/string

and

  [build CXX] l10ntools/source/merge.cxx
  In file included from C:/lo-clang/core/l10ntools/source/merge.cxx:21:
  In file included from C:/lo-clang/core/include\sal/log.hxx:20:
  In file included from 
C:/PROGRA~2/MIB055~1/2019/COMMUN~1/VC/Tools/MSVC/1429~1.300/Include\sstream:11:
  In file included from 
C:/PROGRA~2/MIB055~1/2019/COMMUN~1/VC/Tools/MSVC/1429~1.300/Include\istream:11:
  In file included from 
C:/PROGRA~2/MIB055~1/2019/COMMUN~1/VC/Tools/MSVC/1429~1.300/Include\ostream:11:
  In file included from 
C:/PROGRA~2/MIB055~1/2019/COMMUN~1/VC/Tools/MSVC/1429~1.300/Include\ios:11:
  In file included from 
C:/PROGRA~2/MIB055~1/2019/COMMUN~1/VC/Tools/MSVC/1429~1.300/Include\xlocnum:16:
  In file included from 
C:/PROGRA~2/MIB055~1/2019/COMMUN~1/VC/Tools/MSVC/1429~1.300/Include\streambuf:11:
  In file included from 
C:/PROGRA~2/MIB055~1/2019/COMMUN~1/VC/Tools/MSVC/1429~1.300/Include\xiosbase:12:
  In file included from 
C:/PROGRA~2/MIB055~1/2019/COMMUN~1/VC/Tools/MSVC/1429~1.300/Include\system_error:14:
  In file included from 
C:/PROGRA~2/MIB055~1/2019/COMMUN~1/VC/Tools/MSVC/1429~1.300/Include\stdexcept:12:
  
C:/PROGRA~2/MIB055~1/2019/COMMUN~1/VC/Tools/MSVC/1429~1.300/Include\xstring(4971,12):
 error: non-const lvalue reference to type 'basic_istream<...>' cannot bind to 
a temporary of type 'basic_istream<...>'
  return _Istr;
 ^
  
C:/PROGRA~2/MIB055~1/2019/COMMUN~1/VC/Tools/MSVC/1429~1.300/Include\xstring(4977,29):
 note: in instantiation of function template specialization 
'std::operator>>, std::allocator>' requested 
here
  return _STD move(_Istr) >> _Str;
  ^
  C:/lo-clang/core/l10ntools/source/merge.cxx(125,18): note: in instantiation 
of function template specialization 'std::operator>>, std::allocator>' requested here
  aInputStream >> sPoFile;
   ^
  1 error generated.
  make[1]: *** [C:/lo-clang/core/solenv/gbuild/LinkTarget.mk:301: 
C:/lo-clang/core/workdir/CxxObject/l10ntools/source/merge.o] Error 1

In both cases, the reason is that library happens to implement the overload 
taking `basic_istream&` by forwarding to the overload taking `basic_istream&&` 
(and not the other way around, as libc++ and libstdc++ happen to do), and the 
fix is to replace

  return _Istr;

in the overloads taking `basic_istream&& _Istr` with

  return static_cast&>(_Istr);

(No idea if any MSVC standard library developers are around here, who might 
want to be CC'ed on this.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99005

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

[PATCH] D99005: [clang] Implement P2266 Simpler implicit move

[Stephan Bergmann via Phabricator via cfe-commits]

sberg added a comment.

For the record, the one other breakage of a LibreOffice build with this is the 
handful of places that needed 
https://git.libreoffice.org/core/+/433ab39b2175bdadb4916373cd2dc8e1aabc08a5%5E%21
 "Adapt implicit OString return value construction to C++23 P2266R1":  In a 
nutshell, LO has an `OString` class with (non-explicit) constructors from any 
kind of `char` pointer or array, where the "array of `N` `const char`" variant 
(no. 3 below) is special, as it determines the string length as `N - 1` 
(whereas all the other variants search for the terminating NUL to determine the 
length).  That requires another constructor overload (no. 2 below) for 
non-`const` arrays, taking a non-`const` lvalue reference, which now causes 
issues when that constructor shall implicitly be used in `return` statements:

  $ cat test.cc
  #include 
  
  template struct CharPtrDetector {};
  template<> struct CharPtrDetector
  { using type = int; };
  template<> struct CharPtrDetector
  { using type = int; };
  
  template struct NonConstCharArrayDetector {};
  template struct NonConstCharArrayDetector
  { using type = int; };
  
  template struct ConstCharArrayDetector {};
  template struct ConstCharArrayDetector
  { using type = int; };
  
  struct OString {
  template
  OString(T const &, typename CharPtrDetector::type = 0); // #1
  template
  OString(T &, typename NonConstCharArrayDetector::type = 0); // #2
  template
  OString(T &, typename ConstCharArrayDetector::type = 0); // #3
  };
  
  OString good1a() {
  char * p;
  //...
  return p; // use #1
  };
  
  OString good1b() {
  char const * p;
  //...
  return p; // use #1
  };
  
  OString bad2() {
  char buf[10];
  //...
  return buf; // use #2
  }
  
  OString good3() {
  return "foo"; // use #3
  }
  
  $ clang++ -std=c++20 -fsyntax-only test.cc
  
  $ clang++ -std=c++2b -fsyntax-only test.cc
  test.cc:41:12: error: no viable conversion from returned value of type 'char 
[10]' to function return type 'OString'
  return buf; // use #2
 ^~~
  test.cc:17:8: note: candidate constructor (the implicit copy constructor) not 
viable: no known conversion from 'char [10]' to 'const OString &' for 1st 
argument
  struct OString {
 ^
  test.cc:17:8: note: candidate constructor (the implicit move constructor) not 
viable: no known conversion from 'char [10]' to 'OString &&' for 1st argument
  struct OString {
 ^
  test.cc:21:5: note: candidate constructor [with T = char [10]] not viable: 
expects an lvalue for 1st argument
  OString(T &, typename NonConstCharArrayDetector::type = 0); // #2
  ^
  test.cc:19:5: note: candidate template ignored: substitution failure [with T 
= char [10]]: no type named 'type' in 'CharPtrDetector'
  OString(T const &, typename CharPtrDetector::type = 0); // #1
  ^   
  test.cc:23:5: note: candidate template ignored: substitution failure [with T 
= char [10]]: no type named 'type' in 'ConstCharArrayDetector'
  OString(T &, typename ConstCharArrayDetector::type = 0); // #3
  ^
  1 error generated.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99005

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

[PATCH] D99005: [clang] Implement P2266 Simpler implicit move

[Stephan Bergmann via Phabricator via cfe-commits]

sberg added a comment.

(In a build prior to 
https://reviews.llvm.org/rGc60dd3b2626a4d9eefd9f82f9a406b0d28d3fd72 "Revert 
'[clang] NRVO: Improvements and handling of more cases.'") I see the following 
(reduced from 
https://git.libreoffice.org/core/+/649313625b94e6b879848fc19b607b74375100bf/o3tl/qa/compile-temporary.cxx)
 started to fail under `-std=c++2b` with this change (and continues to compile 
fine with `-std=c++20`):

  $ cat test.cc
  template  constexpr T& temporary(T&& x) { return x; }
  template  constexpr T& temporary(T&) = delete;
  void f(int*);
  int g();
  void h()
  {
  f((int()));
  f((g()));
  }
  
  $ clang++ -std=c++2b -fsyntax-only test.cc
  test.cc:1:62: error: non-const lvalue reference to type 'int' cannot bind to 
a temporary of type 'int'
  template  constexpr T& temporary(T&& x) { return x; }
   ^
  test.cc:7:8: note: in instantiation of function template specialization 
'temporary' requested here
  f((int()));
 ^
  test.cc:8:8: error: no matching function for call to 'temporary'
  f((g()));
 ^
  test.cc:2:36: note: candidate function [with T = int] not viable: expects an 
lvalue for 1st argument
  template  constexpr T& temporary(T&) = delete;
 ^
  test.cc:1:36: note: candidate template ignored: substitution failure [with T 
= int]
  template  constexpr T& temporary(T&& x) { return x; }
 ^
  2 errors generated.

It is not clear to me whether that is an intended change in behavior according 
to http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2021/p2266r1.html 
"Simpler implicit move", or whether it is a bug in this implementation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99005

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

[PATCH] D103949: Only consider built-in compound assignment operators for -Wunused-but-set-*

[Stephan Bergmann 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 rGb5b9489b2415: Only consider built-in compound assignment 
operators for -Wunused-but-set-* (authored by sberg).

Changed prior to commit:
  https://reviews.llvm.org/D103949?vs=350824=351778#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103949

Files:
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/SemaCXX/warn-unused-but-set-variables-cpp.cpp


Index: clang/test/SemaCXX/warn-unused-but-set-variables-cpp.cpp
===
--- clang/test/SemaCXX/warn-unused-but-set-variables-cpp.cpp
+++ clang/test/SemaCXX/warn-unused-but-set-variables-cpp.cpp
@@ -6,6 +6,7 @@
 
 struct __attribute__((warn_unused)) SWarnUnused {
   int j;
+  void operator +=(int);
 };
 
 int f0() {
@@ -48,3 +49,16 @@
   char a[x];
   char b[y];
 }
+
+void f3(int n) {
+  // Don't warn for overloaded compound assignment operators.
+  SWarnUnused swu;
+  swu += n;
+}
+
+template void f4(T n) {
+  // Don't warn for (potentially) overloaded compound assignment operators in
+  // template code.
+  SWarnUnused swu;
+  swu += n;
+}
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -7808,11 +7808,15 @@
 Expr *E, llvm::DenseMap ) {
   DeclRefExpr *LHS = nullptr;
   if (BinaryOperator *BO = dyn_cast(E)) {
-if (!BO->isAssignmentOp())
+if (BO->getLHS()->getType()->isDependentType() ||
+BO->getRHS()->getType()->isDependentType()) {
+  if (BO->getOpcode() != BO_Assign)
+return;
+} else if (!BO->isAssignmentOp())
   return;
 LHS = dyn_cast(BO->getLHS());
   } else if (CXXOperatorCallExpr *COCE = dyn_cast(E)) {
-if (!COCE->isAssignmentOp())
+if (COCE->getOperator() != OO_Equal)
   return;
 LHS = dyn_cast(COCE->getArg(0));
   }


Index: clang/test/SemaCXX/warn-unused-but-set-variables-cpp.cpp
===
--- clang/test/SemaCXX/warn-unused-but-set-variables-cpp.cpp
+++ clang/test/SemaCXX/warn-unused-but-set-variables-cpp.cpp
@@ -6,6 +6,7 @@
 
 struct __attribute__((warn_unused)) SWarnUnused {
   int j;
+  void operator +=(int);
 };
 
 int f0() {
@@ -48,3 +49,16 @@
   char a[x];
   char b[y];
 }
+
+void f3(int n) {
+  // Don't warn for overloaded compound assignment operators.
+  SWarnUnused swu;
+  swu += n;
+}
+
+template void f4(T n) {
+  // Don't warn for (potentially) overloaded compound assignment operators in
+  // template code.
+  SWarnUnused swu;
+  swu += n;
+}
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -7808,11 +7808,15 @@
 Expr *E, llvm::DenseMap ) {
   DeclRefExpr *LHS = nullptr;
   if (BinaryOperator *BO = dyn_cast(E)) {
-if (!BO->isAssignmentOp())
+if (BO->getLHS()->getType()->isDependentType() ||
+BO->getRHS()->getType()->isDependentType()) {
+  if (BO->getOpcode() != BO_Assign)
+return;
+} else if (!BO->isAssignmentOp())
   return;
 LHS = dyn_cast(BO->getLHS());
   } else if (CXXOperatorCallExpr *COCE = dyn_cast(E)) {
-if (!COCE->isAssignmentOp())
+if (COCE->getOperator() != OO_Equal)
   return;
 LHS = dyn_cast(COCE->getArg(0));
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D103949: Only consider built-in compound assignment operators for -Wunused-but-set-*

[Stephan Bergmann via Phabricator via cfe-commits]

sberg added inline comments.



Comment at: clang/lib/Sema/SemaExprCXX.cpp:7794-7798
+if (BO->getLHS()->getType()->isDependentType() ||
+BO->getRHS()->getType()->isDependentType())
+{
+  if (BO->getOpcode() != BO_Assign)
+return;

mbenfield wrote:
> Would you mind elaborating on the need for this code? IIUC, you're concerned 
> about overloaded operators, but won't such operators always be covered by the 
> `CXXOperatorCallExpr` case below? 
That's what I would initially have thought too (see my unanswered 
 "[llvm-dev] 
BinaryOperator vs. CXXOperatorCallExpr in template code"; but which, I notice 
now, I accidentally sent to llvm-dev rather than cfe-dev).

But e.g. the template `f4` test code in `warn-unused-but-set-variables-cpp.cpp` 
turns the `+=` into a `BinaryOperator`.


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

https://reviews.llvm.org/D103949

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

[PATCH] D103623: [Clang] Test case for -Wunused-but-set-variable, warn for volatile.

[Stephan Bergmann via Phabricator via cfe-commits]

sberg accepted this revision.
sberg 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/D103623/new/

https://reviews.llvm.org/D103623

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

[PATCH] D103949: Only consider built-in compound assignment operators for -Wunused-but-set-*

[Stephan Bergmann via Phabricator via cfe-commits]

sberg added a comment.

In D103949#2807490 , @xbolva00 wrote:

> gcc also ignores it?

For reasons that I never looked into, GCC never warned for any of these cases.  
The Clang implementation appears to be way more aggressive (in a positive 
sense) to begin with.


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

https://reviews.llvm.org/D103949

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

[PATCH] D103949: Only consider built-in compound assignment operators for -Wunused-but-set-*

[Stephan Bergmann via Phabricator via cfe-commits]

sberg created this revision.
sberg added a reviewer: mbenfield.
sberg added a project: clang.
sberg requested review of this revision.

At least LibreOffice has, for mainly historic reasons that would be hard to 
change now, a class `Any` with an overloaded `operator >>=` that semantically 
does not assign to the LHS but rather extracts into the (by-reference) RHS.  
Which thus caused false positive `-Wunused-but-set-parameter` and 
`-Wunused-but-set-variable` after those have been introduced recently.
This change is more conservative about the assumed semantics of overloaded 
operators, excluding compound assignment operators but keeping plain `operator 
=` ones.  At least for LibreOffice, that strikes a good balance of not 
producing false positives but still finding lots of true ones.


https://reviews.llvm.org/D103949

Files:
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/SemaCXX/warn-unused-but-set-variables-cpp.cpp


Index: clang/test/SemaCXX/warn-unused-but-set-variables-cpp.cpp
===
--- clang/test/SemaCXX/warn-unused-but-set-variables-cpp.cpp
+++ clang/test/SemaCXX/warn-unused-but-set-variables-cpp.cpp
@@ -6,6 +6,7 @@
 
 struct __attribute__((warn_unused)) SWarnUnused {
   int j;
+  void operator +=(int);
 };
 
 int f0() {
@@ -48,3 +49,16 @@
   char a[x];
   char b[y];
 }
+
+void f3(int n) {
+  // Don't warn for overloaded compound assignment operators.
+  SWarnUnused swu;
+  swu += n;
+}
+
+template void f4(T n) {
+  // Don't warn for (potentially) overloaded compound assignment operators in
+  // template code.
+  SWarnUnused swu;
+  swu += n;
+}
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -7791,11 +7791,16 @@
 Expr *E, llvm::DenseMap ) {
   DeclRefExpr *LHS = nullptr;
   if (BinaryOperator *BO = dyn_cast(E)) {
-if (!BO->isAssignmentOp())
+if (BO->getLHS()->getType()->isDependentType() ||
+BO->getRHS()->getType()->isDependentType())
+{
+  if (BO->getOpcode() != BO_Assign)
+return;
+} else if (!BO->isAssignmentOp())
   return;
 LHS = dyn_cast(BO->getLHS());
   } else if (CXXOperatorCallExpr *COCE = dyn_cast(E)) {
-if (!COCE->isAssignmentOp())
+if (COCE->getOperator() != OO_Equal)
   return;
 LHS = dyn_cast(COCE->getArg(0));
   }


Index: clang/test/SemaCXX/warn-unused-but-set-variables-cpp.cpp
===
--- clang/test/SemaCXX/warn-unused-but-set-variables-cpp.cpp
+++ clang/test/SemaCXX/warn-unused-but-set-variables-cpp.cpp
@@ -6,6 +6,7 @@
 
 struct __attribute__((warn_unused)) SWarnUnused {
   int j;
+  void operator +=(int);
 };
 
 int f0() {
@@ -48,3 +49,16 @@
   char a[x];
   char b[y];
 }
+
+void f3(int n) {
+  // Don't warn for overloaded compound assignment operators.
+  SWarnUnused swu;
+  swu += n;
+}
+
+template void f4(T n) {
+  // Don't warn for (potentially) overloaded compound assignment operators in
+  // template code.
+  SWarnUnused swu;
+  swu += n;
+}
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -7791,11 +7791,16 @@
 Expr *E, llvm::DenseMap ) {
   DeclRefExpr *LHS = nullptr;
   if (BinaryOperator *BO = dyn_cast(E)) {
-if (!BO->isAssignmentOp())
+if (BO->getLHS()->getType()->isDependentType() ||
+BO->getRHS()->getType()->isDependentType())
+{
+  if (BO->getOpcode() != BO_Assign)
+return;
+} else if (!BO->isAssignmentOp())
   return;
 LHS = dyn_cast(BO->getLHS());
   } else if (CXXOperatorCallExpr *COCE = dyn_cast(E)) {
-if (!COCE->isAssignmentOp())
+if (COCE->getOperator() != OO_Equal)
   return;
 LHS = dyn_cast(COCE->getArg(0));
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

[Stephan Bergmann via Phabricator via cfe-commits]

sberg added a comment.

Is it intentional that this warns about volatile variables as in

  void f(char * p) {
  volatile char c = 0;
  c ^= *p;
  }

(I see that GCC warns about volatile too, at least when you replace the `^=` 
with `=`, so assume the answer is "yes", but would just like to see that 
confirmed (ideally with a test case even?).)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100581

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

[PATCH] D97687: [SEH] Fix capture of this in lambda functions

[Stephan Bergmann via Phabricator via cfe-commits]

sberg added a comment.

Bisecting shows that this started to break

  $ cat test.cc
  void f() noexcept { __try {} __finally {} }
  $ clang-cl -EHs -c test.cc
  clang-cl: ~/github.com/llvm/llvm-project/clang/lib/CodeGen/CGCleanup.h:568: 
void clang::CodeGen::EHScopeStack::popTerminate(): Assertion `!empty() && 
"popping exception stack when not empty"' failed.
  PLEASE submit a bug report to https://bugs.llvm.org/ and include the crash 
backtrace, preprocessed source, and associated run script.
  Stack dump:
  0.Program arguments: ~/llvm/inst/bin/clang-cl -EHs -c test.cc
  1. parser at end of file
  2.test.cc:1:6: LLVM IR generation of declaration 'f'
  3.test.cc:1:6: Generating code for declaration 'f'
   #0 0x7f2fd211624d llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) 
~/github.com/llvm/llvm-project/llvm/lib/Support/Unix/Signals.inc:569:3
   #1 0x7f2fd2114174 llvm::sys::RunSignalHandlers() 
~/github.com/llvm/llvm-project/llvm/lib/Support/Signals.cpp:71:20
   #2 0x7f2fd20368c8 HandleCrash 
~/github.com/llvm/llvm-project/llvm/lib/Support/CrashRecoveryContext.cpp:75:5
   #3 0x7f2fd20368c8 CrashRecoverySignalHandler(int) 
~/github.com/llvm/llvm-project/llvm/lib/Support/CrashRecoveryContext.cpp:388:62
   #4 0x7f2fd68f91e0 __restore_rt sigaction.c:0:0
   #5 0x7f2fd1a4b9d5 raise 
/usr/src/debug/glibc-2.32-37-g760e1d2878/signal/../sysdeps/unix/sysv/linux/raise.c:50:1
   #6 0x7f2fd1a348a4 abort 
/usr/src/debug/glibc-2.32-37-g760e1d2878/stdlib/abort.c:81:7
   #7 0x7f2fd1a34789 get_sysdep_segment_value 
/usr/src/debug/glibc-2.32-37-g760e1d2878/intl/loadmsgcat.c:509:8
   #8 0x7f2fd1a34789 _nl_load_domain.cold 
/usr/src/debug/glibc-2.32-37-g760e1d2878/intl/loadmsgcat.c:970:34
   #9 0x7f2fd1a44026 .annobin___GI___assert_fail.end assert.c:0:0
  #10 0x7f2fd5871973 
(~/llvm/inst/bin/../lib/libclangCodeGen.so.13git+0x44b973)
  #11 0x7f2fd587612a 
(~/llvm/inst/bin/../lib/libclangCodeGen.so.13git+0x45012a)
  #12 0x7f2fd5a85bbc 
clang::CodeGen::CodeGenFunction::FinishFunction(clang::SourceLocation) 
~/github.com/llvm/llvm-project/clang/lib/CodeGen/CodeGenFunction.cpp:405:3
  #13 0x7f2fd587f703 
clang::CodeGen::CodeGenFunction::GenerateSEHFinallyFunction(clang::CodeGen::CodeGenFunction&,
 clang::SEHFinallyStmt const&) 
~/github.com/llvm/llvm-project/clang/lib/CodeGen/CGException.cpp:2005:10
  #14 0x7f2fd587f776 pushCleanup<(anonymous namespace)::PerformSEHFinally, 
llvm::Function*> 
~/github.com/llvm/llvm-project/clang/lib/CodeGen/EHScopeStack.h:273:31
  #15 0x7f2fd587f776 
clang::CodeGen::CodeGenFunction::EnterSEHTryStmt(clang::SEHTryStmt const&) 
~/github.com/llvm/llvm-project/clang/lib/CodeGen/CGException.cpp:2082:43
  #16 0x7f2fd587fa87 
clang::CodeGen::CodeGenFunction::getJumpDestInCurrentScope(llvm::StringRef) 
~/github.com/llvm/llvm-project/clang/lib/CodeGen/CodeGenFunction.h:1096:59
  #17 0x7f2fd587fa87 
clang::CodeGen::CodeGenFunction::EmitSEHTryStmt(clang::SEHTryStmt const&) 
~/github.com/llvm/llvm-project/clang/lib/CodeGen/CGException.cpp:1605:65
  #18 0x7f2fd5a21de4 
clang::CodeGen::CodeGenFunction::EmitCompoundStmtWithoutScope(clang::CompoundStmt
 const&, bool, clang::CodeGen::AggValueSlot) 
~/github.com/llvm/llvm-project/clang/lib/CodeGen/CGStmt.cpp:447:31
  #19 0x7f2fd5a8499b 
clang::CodeGen::CodeGenFunction::EmitFunctionBody(clang::Stmt const*) 
~/github.com/llvm/llvm-project/clang/lib/CodeGen/CodeGenFunction.cpp:1185:36
  #20 0x7f2fd5a9214a clang::CodeGen::CodeGenModule::getLangOpts() const 
~/github.com/llvm/llvm-project/clang/lib/CodeGen/CodeGenModule.h:686:51
  #21 0x7f2fd5a9214a clang::CodeGen::CodeGenFunction::getLangOpts() const 
~/github.com/llvm/llvm-project/clang/lib/CodeGen/CodeGenFunction.h:1921:67
  #22 0x7f2fd5a9214a 
clang::CodeGen::CodeGenFunction::GenerateCode(clang::GlobalDecl, 
llvm::Function*, clang::CodeGen::CGFunctionInfo const&) 
~/github.com/llvm/llvm-project/clang/lib/CodeGen/CodeGenFunction.cpp:1352:19
  #23 0x7f2fd5ad447c 
clang::CodeGen::CodeGenModule::EmitGlobalFunctionDefinition(clang::GlobalDecl, 
llvm::GlobalValue*) 
~/github.com/llvm/llvm-project/clang/lib/CodeGen/CodeGenModule.cpp:4791:3
  #24 0x7f2fd5ad0f7b 
clang::CodeGen::CodeGenModule::EmitGlobalDefinition(clang::GlobalDecl, 
llvm::GlobalValue*) 
~/github.com/llvm/llvm-project/clang/lib/CodeGen/CodeGenModule.cpp:3177:47
  #25 0x7f2fd5ad8c8a 
clang::CodeGen::CodeGenModule::EmitTopLevelDecl(clang::Decl*) (.part.0) 
~/github.com/llvm/llvm-project/clang/lib/CodeGen/CodeGenModule.cpp:5895:1
  #26 0x7f2fd5b4e9a1 (anonymous 
namespace)::CodeGeneratorImpl::HandleTopLevelDecl(clang::DeclGroupRef) 
~/github.com/llvm/llvm-project/clang/lib/CodeGen/ModuleBuilder.cpp:169:67
  #27 0x7f2fd5a70b13 
clang::BackendConsumer::HandleTopLevelDecl(clang::DeclGroupRef) 
~/github.com/llvm/llvm-project/clang/lib/CodeGen/CodeGenAction.cpp:217:7
  #28 0x7f2fcfbbd814 clang::ParseAST(clang::Sema&, bool, bool) 

[PATCH] D98971: [C++20] [P1825] Fix bugs with implicit-move from variables of reference type

[Stephan Bergmann via Phabricator via cfe-commits]

sberg added a comment.

FWIW, LibreOffice `make check` (which started to consistently fail with D88220 
) succeeds with this change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98971

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

[PATCH] D88220: [C++20] P1825R0: More implicit moves

[Stephan Bergmann via Phabricator via cfe-commits]

sberg added a comment.

This change causes

  #include 
  std::shared_ptr x1;
  std::shared_ptr f() {
  std::shared_ptr & r = x1;
  r.reset(new int);
  return r;
  }
  int main() {
  std::shared_ptr x2 = f();
  long n = x2.use_count();
  return n;
  }

when built with `-std=c++20` to erroneously return a `use_count` of 1 instead 
of 2 now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88220

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

[PATCH] D87030: Adapt CastExpr::getSubExprAsWritten to ConstantExpr

[Stephan Bergmann via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rG215ed9b33ccb: Adapt CastExpr::getSubExprAsWritten to 
ConstantExpr (authored by sberg).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87030

Files:
  clang/lib/AST/Expr.cpp
  clang/unittests/Tooling/CastExprTest.cpp


Index: clang/unittests/Tooling/CastExprTest.cpp
===
--- clang/unittests/Tooling/CastExprTest.cpp
+++ clang/unittests/Tooling/CastExprTest.cpp
@@ -34,4 +34,24 @@
 "S1 f(S2 s) { return static_cast(s); }\n");
 }
 
+// Verify that getSubExprAsWritten looks through a ConstantExpr in a scenario
+// like
+//
+//   CXXFunctionalCastExpr functional cast to struct S 
+//   `-ConstantExpr 'S'
+// |-value: Struct
+// `-CXXConstructExpr 'S' 'void (int)'
+//   `-IntegerLiteral 'int' 0
+TEST(CastExprTest, GetSubExprAsWrittenThroughConstantExpr) {
+CastExprVisitor Visitor;
+Visitor.OnExplicitCast = [](ExplicitCastExpr *Expr) {
+  auto *Sub = Expr->getSubExprAsWritten();
+  EXPECT_TRUE(isa(Sub))
+<< "Expected IntegerLiteral, but saw " << Sub->getStmtClassName();
+};
+Visitor.runOver("struct S { consteval S(int) {} };\n"
+"S f() { return S(0); }\n",
+CastExprVisitor::Lang_CXX2a);
+}
+
 }
Index: clang/lib/AST/Expr.cpp
===
--- clang/lib/AST/Expr.cpp
+++ clang/lib/AST/Expr.cpp
@@ -1765,7 +1765,7 @@
 // subexpression describing the call; strip it off.
 if (E->getCastKind() == CK_ConstructorConversion)
   SubExpr =
-skipImplicitTemporary(cast(SubExpr)->getArg(0));
+
skipImplicitTemporary(cast(SubExpr->IgnoreImplicit())->getArg(0));
 else if (E->getCastKind() == CK_UserDefinedConversion) {
   assert((isa(SubExpr) ||
   isa(SubExpr)) &&


Index: clang/unittests/Tooling/CastExprTest.cpp
===
--- clang/unittests/Tooling/CastExprTest.cpp
+++ clang/unittests/Tooling/CastExprTest.cpp
@@ -34,4 +34,24 @@
 "S1 f(S2 s) { return static_cast(s); }\n");
 }
 
+// Verify that getSubExprAsWritten looks through a ConstantExpr in a scenario
+// like
+//
+//   CXXFunctionalCastExpr functional cast to struct S 
+//   `-ConstantExpr 'S'
+// |-value: Struct
+// `-CXXConstructExpr 'S' 'void (int)'
+//   `-IntegerLiteral 'int' 0
+TEST(CastExprTest, GetSubExprAsWrittenThroughConstantExpr) {
+CastExprVisitor Visitor;
+Visitor.OnExplicitCast = [](ExplicitCastExpr *Expr) {
+  auto *Sub = Expr->getSubExprAsWritten();
+  EXPECT_TRUE(isa(Sub))
+<< "Expected IntegerLiteral, but saw " << Sub->getStmtClassName();
+};
+Visitor.runOver("struct S { consteval S(int) {} };\n"
+"S f() { return S(0); }\n",
+CastExprVisitor::Lang_CXX2a);
+}
+
 }
Index: clang/lib/AST/Expr.cpp
===
--- clang/lib/AST/Expr.cpp
+++ clang/lib/AST/Expr.cpp
@@ -1765,7 +1765,7 @@
 // subexpression describing the call; strip it off.
 if (E->getCastKind() == CK_ConstructorConversion)
   SubExpr =
-skipImplicitTemporary(cast(SubExpr)->getArg(0));
+skipImplicitTemporary(cast(SubExpr->IgnoreImplicit())->getArg(0));
 else if (E->getCastKind() == CK_UserDefinedConversion) {
   assert((isa(SubExpr) ||
   isa(SubExpr)) &&
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D87030: Adapt CastExpr::getSubExprAsWritten to ConstantExpr

[Stephan Bergmann via Phabricator via cfe-commits]

sberg added a comment.

Ping; OK to submit?  (I meanwhile also get this when compiling some code with 
plain Clang, not just with the LibreOffice Clang plugin where I saw it 
originally.)


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

https://reviews.llvm.org/D87030

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

[PATCH] D90572: [clang] [MinGW] Allow using the vptr sanitizer

[Stephan Bergmann via Phabricator via cfe-commits]

sberg added a comment.

Smells like this breaks various bots due to a -fsanitize=...,... option now 
listing 18 instead of 17 items, see 
http://lab.llvm.org:8011/#builders/76/builds/363, 
http://lab.llvm.org:8011/#builders/93/builds/430, 
http://lab.llvm.org:8011/#builders/66/builds/315, 
http://lab.llvm.org:8011/#builders/7/builds/303.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90572

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

[PATCH] D87030: Adapt CastExpr::getSubExprAsWritten to ConstantExpr

[Stephan Bergmann via Phabricator via cfe-commits]

sberg added a comment.

ping


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

https://reviews.llvm.org/D87030

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

[PATCH] D89481: [scan-build] Fix clang++ pathname again

[Stephan Bergmann via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG7a5184ed951a: [scan-build] Fix clang++ pathname again 
(authored by sberg).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89481

Files:
  clang/test/Analysis/scan-build/cxx-name.test
  clang/test/Analysis/scan-build/lit.local.cfg
  clang/tools/scan-build/bin/scan-build


Index: clang/tools/scan-build/bin/scan-build
===
--- clang/tools/scan-build/bin/scan-build
+++ clang/tools/scan-build/bin/scan-build
@@ -1925,7 +1925,7 @@
 $ClangCXX =~ s/.exe$/++.exe/;
   }
   else {
-$ClangCXX =~ s/\-\d+\.\d+$//;
+$ClangCXX =~ s/\-\d+(\.\d+)?$//;
 $ClangCXX .= "++";
   }
 }
Index: clang/test/Analysis/scan-build/lit.local.cfg
===
--- clang/test/Analysis/scan-build/lit.local.cfg
+++ clang/test/Analysis/scan-build/lit.local.cfg
@@ -15,4 +15,4 @@
 'tools',
 'scan-build',
 'bin')),
- config.clang)))
+ os.path.realpath(config.clang
Index: clang/test/Analysis/scan-build/cxx-name.test
===
--- /dev/null
+++ clang/test/Analysis/scan-build/cxx-name.test
@@ -0,0 +1,9 @@
+REQUIRES: shell
+
+RUN: %scan-build sh -c 'echo "CLANG_CXX=/$(basename "$CLANG_CXX")/"' | 
FileCheck %s
+
+Check that scan-build sets the CLANG_CXX environment variable (meant to be
+consumed by ccc-analyzer) to an appropriate pathname for the clang++ 
executable,
+derived from the pathname of the clang executable:
+
+CHECK: CLANG_CXX=/clang++{{(\.exe)?}}/


Index: clang/tools/scan-build/bin/scan-build
===
--- clang/tools/scan-build/bin/scan-build
+++ clang/tools/scan-build/bin/scan-build
@@ -1925,7 +1925,7 @@
 $ClangCXX =~ s/.exe$/++.exe/;
   }
   else {
-$ClangCXX =~ s/\-\d+\.\d+$//;
+$ClangCXX =~ s/\-\d+(\.\d+)?$//;
 $ClangCXX .= "++";
   }
 }
Index: clang/test/Analysis/scan-build/lit.local.cfg
===
--- clang/test/Analysis/scan-build/lit.local.cfg
+++ clang/test/Analysis/scan-build/lit.local.cfg
@@ -15,4 +15,4 @@
 'tools',
 'scan-build',
 'bin')),
- config.clang)))
+ os.path.realpath(config.clang
Index: clang/test/Analysis/scan-build/cxx-name.test
===
--- /dev/null
+++ clang/test/Analysis/scan-build/cxx-name.test
@@ -0,0 +1,9 @@
+REQUIRES: shell
+
+RUN: %scan-build sh -c 'echo "CLANG_CXX=/$(basename "$CLANG_CXX")/"' | FileCheck %s
+
+Check that scan-build sets the CLANG_CXX environment variable (meant to be
+consumed by ccc-analyzer) to an appropriate pathname for the clang++ executable,
+derived from the pathname of the clang executable:
+
+CHECK: CLANG_CXX=/clang++{{(\.exe)?}}/
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D89481: [scan-build] Fix clang++ pathname again

[Stephan Bergmann via Phabricator via cfe-commits]

sberg updated this revision to Diff 301273.
sberg edited the summary of this revision.
sberg added a comment.

Is there a reason why "NoQ accepted this revision." kept this at "Needs Review" 
rather than moving it to "This revision is now accepted and ready to land."?

Anyway, I added a test now.  Hope the use of `REQUIRES: shell` is appropriate 
guarding for using `sh` and `basename` (and as such would subsume the `// 
FIXME: Actually, "perl". REQUIRES: shell` found in other test files in that 
directory using `%scan-build`, or should it better also carry that FIXME 
comment for a perl requirement, as scan-build is written in perl?).  (I didn't 
find out how `'shell'` gets added to `available_features` for these tests.  I 
only found it in `clang-tools-extra/test/lit.cfg.py` and 
`compiler-rt/test/lit.common.cfg.py`, but which should not be relevant here?  
Also, I didn't manage to verify this doesn't break anything on Windows; while 
my Linux build dir (at least after once having done `cmake --build . --target 
check-all`) has a `tools/clang/test/Analysis/scan-build` dir that I can test 
with `bin/llvm-lit -v tools/clang/test/Analysis/scan-build`, I don't get that 
directory at all on Windows.)


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

https://reviews.llvm.org/D89481

Files:
  clang/test/Analysis/scan-build/cxx-name.test
  clang/test/Analysis/scan-build/lit.local.cfg
  clang/tools/scan-build/bin/scan-build


Index: clang/tools/scan-build/bin/scan-build
===
--- clang/tools/scan-build/bin/scan-build
+++ clang/tools/scan-build/bin/scan-build
@@ -1925,7 +1925,7 @@
 $ClangCXX =~ s/.exe$/++.exe/;
   }
   else {
-$ClangCXX =~ s/\-\d+\.\d+$//;
+$ClangCXX =~ s/\-\d+(\.\d+)?$//;
 $ClangCXX .= "++";
   }
 }
Index: clang/test/Analysis/scan-build/lit.local.cfg
===
--- clang/test/Analysis/scan-build/lit.local.cfg
+++ clang/test/Analysis/scan-build/lit.local.cfg
@@ -15,4 +15,4 @@
 'tools',
 'scan-build',
 'bin')),
- config.clang)))
+ os.path.realpath(config.clang
Index: clang/test/Analysis/scan-build/cxx-name.test
===
--- /dev/null
+++ clang/test/Analysis/scan-build/cxx-name.test
@@ -0,0 +1,9 @@
+REQUIRES: shell
+
+RUN: %scan-build sh -c 'echo "CLANG_CXX=/$(basename "$CLANG_CXX")/"' | 
FileCheck %s
+
+Check that scan-build sets the CLANG_CXX environment variable (meant to be
+consumed by ccc-analyzer) to an appropriate pathname for the clang++ 
executable,
+derived from the pathname of the clang executable:
+
+CHECK: CLANG_CXX=/clang++{{(\.exe)?}}/


Index: clang/tools/scan-build/bin/scan-build
===
--- clang/tools/scan-build/bin/scan-build
+++ clang/tools/scan-build/bin/scan-build
@@ -1925,7 +1925,7 @@
 $ClangCXX =~ s/.exe$/++.exe/;
   }
   else {
-$ClangCXX =~ s/\-\d+\.\d+$//;
+$ClangCXX =~ s/\-\d+(\.\d+)?$//;
 $ClangCXX .= "++";
   }
 }
Index: clang/test/Analysis/scan-build/lit.local.cfg
===
--- clang/test/Analysis/scan-build/lit.local.cfg
+++ clang/test/Analysis/scan-build/lit.local.cfg
@@ -15,4 +15,4 @@
 'tools',
 'scan-build',
 'bin')),
- config.clang)))
+ os.path.realpath(config.clang
Index: clang/test/Analysis/scan-build/cxx-name.test
===
--- /dev/null
+++ clang/test/Analysis/scan-build/cxx-name.test
@@ -0,0 +1,9 @@
+REQUIRES: shell
+
+RUN: %scan-build sh -c 'echo "CLANG_CXX=/$(basename "$CLANG_CXX")/"' | FileCheck %s
+
+Check that scan-build sets the CLANG_CXX environment variable (meant to be
+consumed by ccc-analyzer) to an appropriate pathname for the clang++ executable,
+derived from the pathname of the clang executable:
+
+CHECK: CLANG_CXX=/clang++{{(\.exe)?}}/
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D87030: Adapt CastExpr::getSubExprAsWritten to ConstantExpr

[Stephan Bergmann via Phabricator via cfe-commits]

sberg added a comment.

ping


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

https://reviews.llvm.org/D87030

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

[PATCH] D89481: [scan-build] Fix clang++ pathname again

[Stephan Bergmann via Phabricator via cfe-commits]

sberg added a comment.

ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89481

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

[PATCH] D89481: [scan-build] Fix clang++ pathname again

[Stephan Bergmann via Phabricator via cfe-commits]

sberg created this revision.
sberg added reviewers: aadg, dcoughlin.
Herald added a subscriber: Charusso.
Herald added a project: clang.
sberg requested review of this revision.

e00629f777d9d62875730f40d266727df300dbb2 "[scan-build] Fix clang++ pathname" had
removed the -MAJOR.MINOR suffix, but since presumably LLVM 7 the suffix is only
-MAJOR, so ClangCXX (i.e., the CLANG_CXX environment variable passed to
clang/tools/scan-build/libexec/ccc-analyzer) now contained a non-existing
/path/to/clang-12++ (which apparently went largely unnoticed as
clang/tools/scan-build/libexec/ccc-analyzer falls back to just 'clang++' if the
executable denoted by CLANG_CXX does not exist).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D89481

Files:
  clang/tools/scan-build/bin/scan-build


Index: clang/tools/scan-build/bin/scan-build
===
--- clang/tools/scan-build/bin/scan-build
+++ clang/tools/scan-build/bin/scan-build
@@ -1925,7 +1925,7 @@
 $ClangCXX =~ s/.exe$/++.exe/;
   }
   else {
-$ClangCXX =~ s/\-\d+\.\d+$//;
+$ClangCXX =~ s/\-\d+(\.\d+)?$//;
 $ClangCXX .= "++";
   }
 }


Index: clang/tools/scan-build/bin/scan-build
===
--- clang/tools/scan-build/bin/scan-build
+++ clang/tools/scan-build/bin/scan-build
@@ -1925,7 +1925,7 @@
 $ClangCXX =~ s/.exe$/++.exe/;
   }
   else {
-$ClangCXX =~ s/\-\d+\.\d+$//;
+$ClangCXX =~ s/\-\d+(\.\d+)?$//;
 $ClangCXX .= "++";
   }
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D87030: Adapt CastExpr::getSubExprAsWritten to ConstantExpr

[Stephan Bergmann via Phabricator via cfe-commits]

sberg added a comment.

friendly ping


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

https://reviews.llvm.org/D87030

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

[PATCH] D87030: Adapt CastExpr::getSubExprAsWritten to ConstantExpr

[Stephan Bergmann via Phabricator via cfe-commits]

sberg added a comment.

ping^2


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

https://reviews.llvm.org/D87030

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

[PATCH] D87030: Adapt CastExpr::getSubExprAsWritten to ConstantExpr

[Stephan Bergmann via Phabricator via cfe-commits]

sberg updated this revision to Diff 292728.
sberg added a comment.

ping

(addressed the clang-tidy nitpick)


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

https://reviews.llvm.org/D87030

Files:
  clang/lib/AST/Expr.cpp
  clang/unittests/Tooling/CastExprTest.cpp


Index: clang/unittests/Tooling/CastExprTest.cpp
===
--- clang/unittests/Tooling/CastExprTest.cpp
+++ clang/unittests/Tooling/CastExprTest.cpp
@@ -34,4 +34,24 @@
 "S1 f(S2 s) { return static_cast(s); }\n");
 }
 
+// Verify that getSubExprAsWritten looks through a ConstantExpr in a scenario
+// like
+//
+//   CXXFunctionalCastExpr functional cast to struct S 
+//   `-ConstantExpr 'S'
+// |-value: Struct
+// `-CXXConstructExpr 'S' 'void (int)'
+//   `-IntegerLiteral 'int' 0
+TEST(CastExprTest, GetSubExprAsWrittenThroughConstantExpr) {
+CastExprVisitor Visitor;
+Visitor.OnExplicitCast = [](ExplicitCastExpr *Expr) {
+  auto *Sub = Expr->getSubExprAsWritten();
+  EXPECT_TRUE(isa(Sub))
+<< "Expected IntegerLiteral, but saw " << Sub->getStmtClassName();
+};
+Visitor.runOver("struct S { consteval S(int) {} };\n"
+"S f() { return S(0); }\n",
+CastExprVisitor::Lang_CXX2a);
+}
+
 }
Index: clang/lib/AST/Expr.cpp
===
--- clang/lib/AST/Expr.cpp
+++ clang/lib/AST/Expr.cpp
@@ -1825,7 +1825,7 @@
 // subexpression describing the call; strip it off.
 if (E->getCastKind() == CK_ConstructorConversion)
   SubExpr =
-skipImplicitTemporary(cast(SubExpr)->getArg(0));
+
skipImplicitTemporary(cast(SubExpr->IgnoreImplicit())->getArg(0));
 else if (E->getCastKind() == CK_UserDefinedConversion) {
   assert((isa(SubExpr) ||
   isa(SubExpr)) &&


Index: clang/unittests/Tooling/CastExprTest.cpp
===
--- clang/unittests/Tooling/CastExprTest.cpp
+++ clang/unittests/Tooling/CastExprTest.cpp
@@ -34,4 +34,24 @@
 "S1 f(S2 s) { return static_cast(s); }\n");
 }
 
+// Verify that getSubExprAsWritten looks through a ConstantExpr in a scenario
+// like
+//
+//   CXXFunctionalCastExpr functional cast to struct S 
+//   `-ConstantExpr 'S'
+// |-value: Struct
+// `-CXXConstructExpr 'S' 'void (int)'
+//   `-IntegerLiteral 'int' 0
+TEST(CastExprTest, GetSubExprAsWrittenThroughConstantExpr) {
+CastExprVisitor Visitor;
+Visitor.OnExplicitCast = [](ExplicitCastExpr *Expr) {
+  auto *Sub = Expr->getSubExprAsWritten();
+  EXPECT_TRUE(isa(Sub))
+<< "Expected IntegerLiteral, but saw " << Sub->getStmtClassName();
+};
+Visitor.runOver("struct S { consteval S(int) {} };\n"
+"S f() { return S(0); }\n",
+CastExprVisitor::Lang_CXX2a);
+}
+
 }
Index: clang/lib/AST/Expr.cpp
===
--- clang/lib/AST/Expr.cpp
+++ clang/lib/AST/Expr.cpp
@@ -1825,7 +1825,7 @@
 // subexpression describing the call; strip it off.
 if (E->getCastKind() == CK_ConstructorConversion)
   SubExpr =
-skipImplicitTemporary(cast(SubExpr)->getArg(0));
+skipImplicitTemporary(cast(SubExpr->IgnoreImplicit())->getArg(0));
 else if (E->getCastKind() == CK_UserDefinedConversion) {
   assert((isa(SubExpr) ||
   isa(SubExpr)) &&
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D87030: Adapt CastExpr::getSubExprAsWritten to ConstantExpr

[Stephan Bergmann via Phabricator via cfe-commits]

sberg added a comment.

I hit this with a call to getSubExprAsWriten from the LibreOffice Clang plugin, 
have no idea whether it can be hit from within the compiler itself.  Not sure 
if clang/unittests/Tooling/CastExprTest.cpp is the best place to add a test for 
it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87030

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

[PATCH] D87030: Adapt CastExpr::getSubExprAsWritten to ConstantExpr

[Stephan Bergmann via Phabricator via cfe-commits]

sberg created this revision.
sberg added a reviewer: rsmith.
Herald added a project: clang.
sberg requested review of this revision.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D87030

Files:
  clang/lib/AST/Expr.cpp
  clang/unittests/Tooling/CastExprTest.cpp


Index: clang/unittests/Tooling/CastExprTest.cpp
===
--- clang/unittests/Tooling/CastExprTest.cpp
+++ clang/unittests/Tooling/CastExprTest.cpp
@@ -34,4 +34,23 @@
 "S1 f(S2 s) { return static_cast(s); }\n");
 }
 
+// Verify that getSubExprAsWritten looks through a ConstantExpr in a scenario
+// like
+//
+//   CXXFunctionalCastExpr functional cast to struct S 
+//   `-ConstantExpr 'S'
+// |-value: Struct
+// `-CXXConstructExpr 'S' 'void (int)'
+//   `-IntegerLiteral 'int' 0
+TEST(CastExprTest, GetSubExprAsWrittenThroughConstantExpr) {
+  CastExprVisitor Visitor;
+  Visitor.OnExplicitCast = [](ExplicitCastExpr *Expr) {
+auto Sub = Expr->getSubExprAsWritten();
+EXPECT_TRUE(isa(Sub))
+<< "Expected IntegerLiteral, but saw " << Sub->getStmtClassName();
+  };
+  Visitor.runOver("struct S { consteval S(int) {} };\n"
+  "S f() { return S(0); }\n",
+  CastExprVisitor::Lang_CXX2a);
+}
 }
Index: clang/lib/AST/Expr.cpp
===
--- clang/lib/AST/Expr.cpp
+++ clang/lib/AST/Expr.cpp
@@ -1823,8 +1823,8 @@
 // Conversions by constructor and conversion functions have a
 // subexpression describing the call; strip it off.
 if (E->getCastKind() == CK_ConstructorConversion)
-  SubExpr =
-skipImplicitTemporary(cast(SubExpr)->getArg(0));
+  SubExpr = skipImplicitTemporary(
+  cast(SubExpr->IgnoreImplicit())->getArg(0));
 else if (E->getCastKind() == CK_UserDefinedConversion) {
   assert((isa(SubExpr) ||
   isa(SubExpr)) &&


Index: clang/unittests/Tooling/CastExprTest.cpp
===
--- clang/unittests/Tooling/CastExprTest.cpp
+++ clang/unittests/Tooling/CastExprTest.cpp
@@ -34,4 +34,23 @@
 "S1 f(S2 s) { return static_cast(s); }\n");
 }
 
+// Verify that getSubExprAsWritten looks through a ConstantExpr in a scenario
+// like
+//
+//   CXXFunctionalCastExpr functional cast to struct S 
+//   `-ConstantExpr 'S'
+// |-value: Struct
+// `-CXXConstructExpr 'S' 'void (int)'
+//   `-IntegerLiteral 'int' 0
+TEST(CastExprTest, GetSubExprAsWrittenThroughConstantExpr) {
+  CastExprVisitor Visitor;
+  Visitor.OnExplicitCast = [](ExplicitCastExpr *Expr) {
+auto Sub = Expr->getSubExprAsWritten();
+EXPECT_TRUE(isa(Sub))
+<< "Expected IntegerLiteral, but saw " << Sub->getStmtClassName();
+  };
+  Visitor.runOver("struct S { consteval S(int) {} };\n"
+  "S f() { return S(0); }\n",
+  CastExprVisitor::Lang_CXX2a);
+}
 }
Index: clang/lib/AST/Expr.cpp
===
--- clang/lib/AST/Expr.cpp
+++ clang/lib/AST/Expr.cpp
@@ -1823,8 +1823,8 @@
 // Conversions by constructor and conversion functions have a
 // subexpression describing the call; strip it off.
 if (E->getCastKind() == CK_ConstructorConversion)
-  SubExpr =
-skipImplicitTemporary(cast(SubExpr)->getArg(0));
+  SubExpr = skipImplicitTemporary(
+  cast(SubExpr->IgnoreImplicit())->getArg(0));
 else if (E->getCastKind() == CK_UserDefinedConversion) {
   assert((isa(SubExpr) ||
   isa(SubExpr)) &&
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68720: Support -fstack-clash-protection for x86

[Stephan Bergmann via Phabricator via cfe-commits]

sberg added a comment.

I filed https://bugs.llvm.org/show_bug.cgi?id=47139 "Misspelled 
-fnostack-clash-protection" now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68720

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

[PATCH] D85778: More accurately compute the ranges of possible values for +, -, *, &, %.

[Stephan Bergmann via Phabricator via cfe-commits]

sberg added a comment.

Fixes all the false positives it had reported for LibreOffice (which had all 
involved expressions containing either ~ or +).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85778

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

[PATCH] D85256: Add -Wtautological-value-range-compare warning.

[Stephan Bergmann via Phabricator via cfe-commits]

sberg added a comment.

I think this generates a false positive with `test.cc`

  enum E { E1 = 1, E2 = 2 };
  bool f(E e) { return ((e & E1) ? 1 : 0) + ((e & E2) ? 1 : 0) > 1; }

and `clang++ -fsyntax-only -Wtautological-value-range-compare test.cc`

  test.cc:2:62: warning: result of comparison of 1-bit unsigned value > 1 is 
always false [-Wtautological-value-range-compare]
  bool f(E e) { return ((e & E1) ? 1 : 0) + ((e & E2) ? 1 : 0) > 1; }
   ~~~ ^ ~


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85256

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

[PATCH] D68720: Support -fstack-clash-protection for x86

[Stephan Bergmann via Phabricator via cfe-commits]

sberg added inline comments.
Herald added a subscriber: dang.



Comment at: clang/include/clang/Driver/Options.td:1778
+  HelpText<"Enable stack clash protection">;
+def fnostack_clash_protection : Flag<["-"], "fnostack-clash-protection">, 
Group,
+  HelpText<"Disable stack clash protection">;

Should this rather spell `"fno-stack-clash-protection"`?  The above change to 
clang/docs/ClangCommandLineReference.rst (but which got overwritten by 

 "[docs] Regenerate ClangCommandLineReference.rst") mentions 
`-fno-stack-clash-protection`, and also GCC calls it like that.  (Though the 
below clang/test/Driver/stack-clash-protection.c does use 
`-fnostack-clash-protection`.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68720

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

[PATCH] D85287: Extend -Wtautological-bitwise-compare "bitwise or with non-zero value" warnings

[Stephan Bergmann via Phabricator via cfe-commits]

sberg added a comment.

In D85287#2199463 , @rtrieu wrote:

> Are you planning to allow this change to other warnings that use the same 
> helper functions?

No, I didn't plan to work on this further.  Just scratching that itch of why 
Clang didn't emit that one warning it "obviously" should have emitted.

> Also, have you tried running warning over a codebase?

As I wrote: "At least building LibreOffice with this change caused no false 
positives."  (Or maybe I misunderstand your question.)


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

https://reviews.llvm.org/D85287

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

[PATCH] D85287: Extend -Wtautological-bitwise-compare "bitwise or with non-zero value" warnings

[Stephan Bergmann via Phabricator via cfe-commits]

sberg added a comment.

My naive assumption was that this warning had initially been restricted to 
integer literals and enumerators to avoid false positives.  Hence my 
conservative approach of extending merely to constant integer expressions for 
now.  Maybe Richard as the original author of the warning (IIUC) can shed some 
light on that original restriction?


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

https://reviews.llvm.org/D85287

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

[PATCH] D85287: Extend -Wtautological-bitwise-compare "bitwise or with non-zero value" warnings

[Stephan Bergmann via Phabricator via cfe-commits]

sberg updated this revision to Diff 283204.

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

https://reviews.llvm.org/D85287

Files:
  clang/lib/Analysis/CFG.cpp
  clang/test/Sema/warn-bitwise-compare.c
  clang/test/SemaCXX/warn-bitwise-compare.cpp

Index: clang/test/SemaCXX/warn-bitwise-compare.cpp
===
--- clang/test/SemaCXX/warn-bitwise-compare.cpp
+++ clang/test/SemaCXX/warn-bitwise-compare.cpp
@@ -1,6 +1,12 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -Wtautological-bitwise-compare %s
 // RUN: %clang_cc1 -fsyntax-only -verify -Wall -Wno-unused %s
 
+int f() { return 3; }
+
+const int const1 = 1;
+constexpr int const2 = 2;
+const int const3 = f();
+
 void test(int x) {
   bool b1 = (8 & x) == 3;
   // expected-warning@-1 {{bitwise comparison always evaluates to false}}
@@ -10,4 +16,15 @@
   // expected-warning@-1 {{bitwise or with non-zero value always evaluates to true}}
   bool b4 = !!(x | 5);
   // expected-warning@-1 {{bitwise or with non-zero value always evaluates to true}}
+  bool b5 = (x | const1);
+  // expected-warning@-1 {{bitwise or with non-zero value always evaluates to true}}
+  bool b6 = (x | const2);
+  // expected-warning@-1 {{bitwise or with non-zero value always evaluates to true}}
+  bool b7 = (x | const3);
+
+  // No warnings at least for now:
+  bool b8 = (x & const1) == 8;
+  bool b9 = (x | const1) == 4;
+  bool b10 = (x & const2) == 8;
+  bool b11 = (x | const2) == 4;
 }
Index: clang/test/Sema/warn-bitwise-compare.c
===
--- clang/test/Sema/warn-bitwise-compare.c
+++ clang/test/Sema/warn-bitwise-compare.c
@@ -8,6 +8,8 @@
   ONE,
 };
 
+const int const1 = 1;
+
 void f(int x) {
   if ((8 & x) == 3) {}  // expected-warning {{bitwise comparison always evaluates to false}}
   if ((x & 8) == 4) {}  // expected-warning {{bitwise comparison always evaluates to false}}
@@ -34,6 +36,9 @@
 
   if ((x & mydefine) == 8) {}
   if ((x | mydefine) == 4) {}
+
+  if ((x & const1) == 8) {}
+  if ((x | const1) == 4) {}
 }
 
 void g(int x) {
@@ -45,4 +50,6 @@
   if (x | ONE) {}  // expected-warning {{bitwise or with non-zero value always evaluates to true}}
 
   if (x | ZERO) {}
+
+  if (x | const1) {}
 }
Index: clang/lib/Analysis/CFG.cpp
===
--- clang/lib/Analysis/CFG.cpp
+++ clang/lib/Analysis/CFG.cpp
@@ -93,29 +93,36 @@
   return isa(E);
 }
 
-/// Helper for tryNormalizeBinaryOperator. Attempts to extract an IntegerLiteral
-/// constant expression or EnumConstantDecl from the given Expr. If it fails,
+/// Helper for tryNormalizeBinaryOperator. Attempts to extract a suitable
+/// integer or enum constant from the given Expr. If it fails,
 /// returns nullptr.
-static const Expr *tryTransformToIntOrEnumConstant(const Expr *E) {
+static const Expr *tryTransformToIntOrEnumConstant(const Expr *E,
+   ASTContext ) {
   E = E->IgnoreParens();
   if (IsIntegerLiteralConstantExpr(E))
 return E;
-  if (auto *DR = dyn_cast(E->IgnoreParenImpCasts()))
-return isa(DR->getDecl()) ? DR : nullptr;
+  if (auto *DR = dyn_cast(E->IgnoreParenImpCasts())) {
+auto *D = DR->getDecl();
+if (isa(D))
+  return DR;
+if (auto *VD = dyn_cast(D))
+  if (VD->isUsableInConstantExpressions(Ctx))
+return DR;
+  }
   return nullptr;
 }
 
 /// Tries to interpret a binary operator into `Expr Op NumExpr` form, if
-/// NumExpr is an integer literal or an enum constant.
+/// NumExpr is a suitable integer or an enum constant.
 ///
 /// If this fails, at least one of the returned DeclRefExpr or Expr will be
 /// null.
 static std::tuple
-tryNormalizeBinaryOperator(const BinaryOperator *B) {
+tryNormalizeBinaryOperator(const BinaryOperator *B, ASTContext ) {
   BinaryOperatorKind Op = B->getOpcode();
 
   const Expr *MaybeDecl = B->getLHS();
-  const Expr *Constant = tryTransformToIntOrEnumConstant(B->getRHS());
+  const Expr *Constant = tryTransformToIntOrEnumConstant(B->getRHS(), Ctx);
   // Expr looked like `0 == Foo` instead of `Foo == 0`
   if (Constant == nullptr) {
 // Flip the operator
@@ -129,7 +136,7 @@
   Op = BO_GE;
 
 MaybeDecl = B->getRHS();
-Constant = tryTransformToIntOrEnumConstant(B->getLHS());
+Constant = tryTransformToIntOrEnumConstant(B->getLHS(), Ctx);
   }
 
   return std::make_tuple(MaybeDecl, Op, Constant);
@@ -140,7 +147,7 @@
 /// literals.
 ///
 /// It's an error to pass this arguments that are not either IntegerLiterals
-/// or DeclRefExprs (that have decls of type EnumConstantDecl)
+/// or DeclRefExprs
 static bool areExprTypesCompatible(const Expr *E1, const Expr *E2) {
   // User intent isn't clear if they're mixing int literals with enum
   // constants.
@@ -151,18 +158,21 @@
   if (!isa(E1))
 return true;
 
-  // IntegerLiterals are handled above and only EnumConstantDecls are expected
+  // 

[PATCH] D85287: Extend -Wtautological-bitwise-compare "bitwise or with non-zero value" warnings

[Stephan Bergmann via Phabricator via cfe-commits]

sberg created this revision.
sberg added reviewers: jordan_rose, rtrieu.
Herald added a project: clang.
sberg requested review of this revision.

...to C++ constant expression operands.

(I had been puzzled why Clang had not found  "cid#1465512 Wrong operator used"
in the LibreOffice source code, only found by Coverity Scan.  At least building
LibreOffice with this change caused no false positives.)

In C, values of const-qualified variables are never constant expressions, so
nothing should change for C code.

To avoid potential further false positives, restrict this change only to the
"bitwise or with non-zero value" warnings while keeping all other
-Wtautological-bitwise-compare warnings as-is, at least for now.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85287

Files:
  clang/lib/Analysis/CFG.cpp
  clang/test/Sema/warn-bitwise-compare.c
  clang/test/SemaCXX/warn-bitwise-compare.cpp

Index: clang/test/SemaCXX/warn-bitwise-compare.cpp
===
--- clang/test/SemaCXX/warn-bitwise-compare.cpp
+++ clang/test/SemaCXX/warn-bitwise-compare.cpp
@@ -1,6 +1,12 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -Wtautological-bitwise-compare %s
 // RUN: %clang_cc1 -fsyntax-only -verify -Wall -Wno-unused %s
 
+int f() { return 3; }
+
+const int const1 = 1;
+constexpr int const2 = 2;
+const int const3 = f();
+
 void test(int x) {
   bool b1 = (8 & x) == 3;
   // expected-warning@-1 {{bitwise comparison always evaluates to false}}
@@ -10,4 +16,15 @@
   // expected-warning@-1 {{bitwise or with non-zero value always evaluates to true}}
   bool b4 = !!(x | 5);
   // expected-warning@-1 {{bitwise or with non-zero value always evaluates to true}}
+  bool b5 = (x | const1);
+  // expected-warning@-1 {{bitwise or with non-zero value always evaluates to true}}
+  bool b6 = (x | const2);
+  // expected-warning@-1 {{bitwise or with non-zero value always evaluates to true}}
+  bool b7 = (x | const3);
+
+  // No warnings at least for now:
+  bool b8 = (x & const1) == 8;
+  bool b9 = (x | const1) == 4;
+  bool b10 = (x & const2) == 8;
+  bool b11 = (x | const2) == 4;
 }
Index: clang/test/Sema/warn-bitwise-compare.c
===
--- clang/test/Sema/warn-bitwise-compare.c
+++ clang/test/Sema/warn-bitwise-compare.c
@@ -8,6 +8,8 @@
   ONE,
 };
 
+const int const1 = 1;
+
 void f(int x) {
   if ((8 & x) == 3) {}  // expected-warning {{bitwise comparison always evaluates to false}}
   if ((x & 8) == 4) {}  // expected-warning {{bitwise comparison always evaluates to false}}
@@ -34,6 +36,9 @@
 
   if ((x & mydefine) == 8) {}
   if ((x | mydefine) == 4) {}
+
+  if ((x & const1) == 8) {}
+  if ((x | const1) == 4) {}
 }
 
 void g(int x) {
@@ -45,4 +50,6 @@
   if (x | ONE) {}  // expected-warning {{bitwise or with non-zero value always evaluates to true}}
 
   if (x | ZERO) {}
+
+  if (x | const1) {}
 }
Index: clang/lib/Analysis/CFG.cpp
===
--- clang/lib/Analysis/CFG.cpp
+++ clang/lib/Analysis/CFG.cpp
@@ -93,15 +93,22 @@
   return isa(E);
 }
 
-/// Helper for tryNormalizeBinaryOperator. Attempts to extract an IntegerLiteral
-/// constant expression or EnumConstantDecl from the given Expr. If it fails,
+/// Helper for tryNormalizeBinaryOperator. Attempts to extract a suitable
+/// integer or enum constant expression from the given Expr. If it fails,
 /// returns nullptr.
-static const Expr *tryTransformToIntOrEnumConstant(const Expr *E) {
+static const Expr *tryTransformToIntOrEnumConstant(const Expr *E,
+   ASTContext ) {
   E = E->IgnoreParens();
   if (IsIntegerLiteralConstantExpr(E))
 return E;
-  if (auto *DR = dyn_cast(E->IgnoreParenImpCasts()))
-return isa(DR->getDecl()) ? DR : nullptr;
+  if (auto *DR = dyn_cast(E->IgnoreParenImpCasts())) {
+auto *D = DR->getDecl();
+if (isa(D))
+  return DR;
+if (auto *VD = dyn_cast(D))
+  if (VD->isUsableInConstantExpressions(Ctx))
+return DR;
+  }
   return nullptr;
 }
 
@@ -111,11 +118,11 @@
 /// If this fails, at least one of the returned DeclRefExpr or Expr will be
 /// null.
 static std::tuple
-tryNormalizeBinaryOperator(const BinaryOperator *B) {
+tryNormalizeBinaryOperator(const BinaryOperator *B, ASTContext ) {
   BinaryOperatorKind Op = B->getOpcode();
 
   const Expr *MaybeDecl = B->getLHS();
-  const Expr *Constant = tryTransformToIntOrEnumConstant(B->getRHS());
+  const Expr *Constant = tryTransformToIntOrEnumConstant(B->getRHS(), Ctx);
   // Expr looked like `0 == Foo` instead of `Foo == 0`
   if (Constant == nullptr) {
 // Flip the operator
@@ -129,7 +136,7 @@
   Op = BO_GE;
 
 MaybeDecl = B->getRHS();
-Constant = tryTransformToIntOrEnumConstant(B->getLHS());
+Constant = 

[PATCH] D76646: Rename/refactor isIntegerConstantExpression to getIntegerConstantExpression

[Stephan Bergmann via Phabricator via cfe-commits]

sberg added inline comments.



Comment at: clang/include/clang/AST/Expr.h:513
 
-  /// isIntegerConstantExpr - Return true if this expression is a valid integer
-  /// constant expression, and, if so, return its value in Result.  If not a
-  /// valid i-c-e, return false and fill in Loc (if specified) with the 
location
-  /// of the invalid expression.
+  /// isIntegerConstantExpr - Return the value if this expression is a valid
+  /// integer constant expression.  If not a valid i-c-e, return None and fill

I think the "Return the value..." description now matches 
getIntegerConstantExpr, not isIntegerConstantExpr


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76646



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

[PATCH] D76572: Replace `T(x)` with `reinterpret_cast(x)` everywhere it means reinterpret_cast. No functional change

[Stephan Bergmann via Phabricator via cfe-commits]

sberg added a comment.

In D76572#1936935 , @grandinj wrote:

> In D76572#1936861 , @Quuxplusone 
> wrote:
>
> > Nice. Does LibreOffice have anything (either in clang-tidy or in a paper 
> > guideline) against `T(x)`-style casts? E.g.
>
>
> No, we don't have very many of those in our codebase, so we have left them 
> alone.
>  Our plugin is designed to convert c-style casts to modern C++ casts.


Heh, I've meanwhile improved LibreOffice's cstylecast check now (after I'd read 
only the first few initial comments here), to also flag function-style casts 
that should be const_/reinterpret_cast, and it indeed only found a handful of 
cases that would better be reinterpret_cast 
(https://gerrit.libreoffice.org/plugins/gitiles/core/+/1ebeacb20ad0165e399629fcfd7795ad0da3edf8%5E!/
 "Extend loplugin:cstylecast to certain function-style casts").  Though in two 
of those places it conveniently unearthed code that should rather do without 
any kind of casting (addressed with follow-up 
https://gerrit.libreoffice.org/plugins/gitiles/core/+/741d30b5e1b0dcdbafb300ed7c7ad46756ffd946%5E!/
 "Simplify pointer equality comparison" and 
https://gerrit.libreoffice.org/plugins/gitiles/core/+/e3196f3dddad6e7825db3b35e8196be35b466fd9%5E!/
 "Fix pointer equality comparision").


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76572



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

[PATCH] D73637: Fix handling of OO_Spaceship in DecodeOperatorCall

[Stephan Bergmann via Phabricator via cfe-commits]

sberg added a comment.

ping^2


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

https://reviews.llvm.org/D73637



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

[PATCH] D73637: Fix handling of OO_Spaceship in DecodeOperatorCall

[Stephan Bergmann via Phabricator via cfe-commits]

sberg added a comment.

ping


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

https://reviews.llvm.org/D73637



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

[PATCH] D73637: Fix handling of OO_Spaceship in DecodeOperatorCall

[Stephan Bergmann via Phabricator via cfe-commits]

sberg updated this revision to Diff 241485.
sberg added a comment.

Minimal reproducer is

  template struct S;
  template void operator <=>(S, S);
  template void f(T x) { decltype(x <=> x) v; }

Added a corresponding test to clang/test/SemaCXX/cxx2a-three-way-comparison.cpp.


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

https://reviews.llvm.org/D73637

Files:
  clang/lib/AST/StmtProfile.cpp
  clang/test/SemaCXX/cxx2a-three-way-comparison.cpp


Index: clang/test/SemaCXX/cxx2a-three-way-comparison.cpp
===
--- clang/test/SemaCXX/cxx2a-three-way-comparison.cpp
+++ clang/test/SemaCXX/cxx2a-three-way-comparison.cpp
@@ -22,3 +22,5 @@
 };
 
 int  = B().operator<=>(0);
+
+template void f(T x) { decltype(x <=> x) v; }
Index: clang/lib/AST/StmtProfile.cpp
===
--- clang/lib/AST/StmtProfile.cpp
+++ clang/lib/AST/StmtProfile.cpp
@@ -1535,8 +1535,8 @@
 return Stmt::BinaryOperatorClass;
 
   case OO_Spaceship:
-// FIXME: Update this once we support <=> expressions.
-llvm_unreachable("<=> expressions not supported yet");
+BinaryOp = BO_Cmp;
+return Stmt::BinaryOperatorClass;
 
   case OO_AmpAmp:
 BinaryOp = BO_LAnd;


Index: clang/test/SemaCXX/cxx2a-three-way-comparison.cpp
===
--- clang/test/SemaCXX/cxx2a-three-way-comparison.cpp
+++ clang/test/SemaCXX/cxx2a-three-way-comparison.cpp
@@ -22,3 +22,5 @@
 };
 
 int  = B().operator<=>(0);
+
+template void f(T x) { decltype(x <=> x) v; }
Index: clang/lib/AST/StmtProfile.cpp
===
--- clang/lib/AST/StmtProfile.cpp
+++ clang/lib/AST/StmtProfile.cpp
@@ -1535,8 +1535,8 @@
 return Stmt::BinaryOperatorClass;
 
   case OO_Spaceship:
-// FIXME: Update this once we support <=> expressions.
-llvm_unreachable("<=> expressions not supported yet");
+BinaryOp = BO_Cmp;
+return Stmt::BinaryOperatorClass;
 
   case OO_AmpAmp:
 BinaryOp = BO_LAnd;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D73637: Fix handling of OO_Spaceship in DecodeOperatorCall

[Stephan Bergmann via Phabricator via cfe-commits]

sberg created this revision.
sberg added reviewers: rsmith, EricWF.
sberg added a project: clang.
Herald added a subscriber: cfe-commits.

This seems to be a leftover from d30b23d6a54b3f0883914f3c2c2318a78edcbe67 
"[c++2a] P0515R3: Support for overloaded operator<=>."  (The corresponding 
llvm_unreachable in clang/lib/Sema/SemaOverload.cpp was e.g. fixed in 
0683c0e68d31d18f6e4fedb270317844a4912882 "[C++2a] Implement operator<=> CodeGen 
and ExprConstant".)
It caused compilation of recent libstdc++ trunk  to crash in -std=c++2a 
mode, hitting the llvm_unreachable.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D73637

Files:
  clang/lib/AST/StmtProfile.cpp


Index: clang/lib/AST/StmtProfile.cpp
===
--- clang/lib/AST/StmtProfile.cpp
+++ clang/lib/AST/StmtProfile.cpp
@@ -1535,8 +1535,8 @@
 return Stmt::BinaryOperatorClass;
 
   case OO_Spaceship:
-// FIXME: Update this once we support <=> expressions.
-llvm_unreachable("<=> expressions not supported yet");
+BinaryOp = BO_Cmp;
+return Stmt::BinaryOperatorClass;
 
   case OO_AmpAmp:
 BinaryOp = BO_LAnd;


Index: clang/lib/AST/StmtProfile.cpp
===
--- clang/lib/AST/StmtProfile.cpp
+++ clang/lib/AST/StmtProfile.cpp
@@ -1535,8 +1535,8 @@
 return Stmt::BinaryOperatorClass;
 
   case OO_Spaceship:
-// FIXME: Update this once we support <=> expressions.
-llvm_unreachable("<=> expressions not supported yet");
+BinaryOp = BO_Cmp;
+return Stmt::BinaryOperatorClass;
 
   case OO_AmpAmp:
 BinaryOp = BO_LAnd;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71393: Default to -fno-use-init-array

[Stephan Bergmann via Phabricator via cfe-commits]

sberg added a comment.

The title should probably read "Default to -fuse-init-array" (dropping the 
"no-")?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71393



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

[PATCH] D58896: Suppress -Wchar-subscripts if the index is a literal char

[Stephan Bergmann via Phabricator via cfe-commits]

sberg added a comment.

In D58896#1738288 , @aaron.ballman 
wrote:

> In D58896#1738263 , @sberg wrote:
>
> > In D58896#1737242 , @edward-jones 
> > wrote:
> >
> > > In D58896#1737113 , @sberg wrote:
> > >
> > > > But how about literals like `'\x80'` where the promoted value depends 
> > > > on whether plain `char` is signed or unsigned?
> > >
> > >
> > > If 'char' is signed and index into an array then this will typically 
> > > trigger an `-Warray-bounds` warning because it references before the 
> > > start of the array.
> >
> >
> > My thought was more that it might be useful as a kind of portability 
> > warning.
>
>
> I'm not opposed to the warning per-se, but do you have evidence that the 
> situation occurs in real-world code?


No.  (My original comment was driven by my, potentially false, assumption that 
this warning was originally, at least in part, meant to flag portability 
issues---along the lines of: why else would the warning trigger at all when 
`char` is unsigned.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58896



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

[PATCH] D58896: Suppress -Wchar-subscripts if the index is a literal char

[Stephan Bergmann via Phabricator via cfe-commits]

sberg added a comment.

In D58896#1737242 , @edward-jones 
wrote:

> In D58896#1737113 , @sberg wrote:
>
> > But how about literals like `'\x80'` where the promoted value depends on 
> > whether plain `char` is signed or unsigned?
>
>
> If 'char' is signed and index into an array then this will typically trigger 
> an `-Warray-bounds` warning because it references before the start of the 
> array.


My thought was more that it might be useful as a kind of portability warning.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58896



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

[PATCH] D58896: Suppress -Wchar-subscripts if the index is a literal char

[Stephan Bergmann via Phabricator via cfe-commits]

sberg added a comment.

But how about literals like `'\x80'` where the promoted value depends on 
whether plain `char` is signed or unsigned?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58896



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

[PATCH] D68581: Include leading attributes in DeclStmt's SourceRange

[Stephan Bergmann via Phabricator via cfe-commits]

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



Comment at: clang/test/AST/sourceranges.cpp:155
+
+#if __cplusplus >= 201703L
+  void cpp17() {

aaron.ballman wrote:
> Why is this guarded on C++17? `[[maybe_unused]]` is supported in every 
> language mode used by this test (it generates a warning, but we're 
> FileCheck'ing so that's fine -- but you could use `[[deprecated("")]]` 
> instead if you want).
> 
> Additionally, I'd appreciate one more test case testing a `__declspec` 
> attribute as well, to make sure we're handling that properly as well.
Oh, didn't know that [[maybe_unused]] would also be accepted in pre-C++17  
modes (I was somehow under the false impression that this file would be 
processed as -std=c++11 by default, where [[deprecated("")]] wouldn't have 
worked either).
Testing both __attribute__ and __declspec at both the start and in the middle 
of the stmt now.  (One caveat with __declspec is that its location resolves to 
, though.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68581



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

[PATCH] D68581: Include leading attributes in DeclStmt's SourceRange

[Stephan Bergmann via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGdc3957ec215d: Include leading attributes in DeclStmts 
SourceRange (authored by sberg).

Changed prior to commit:
  https://reviews.llvm.org/D68581?vs=224823=225397#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68581

Files:
  clang/lib/Parse/ParseStmt.cpp
  clang/test/AST/sourceranges.cpp


Index: clang/test/AST/sourceranges.cpp
===
--- clang/test/AST/sourceranges.cpp
+++ clang/test/AST/sourceranges.cpp
@@ -92,6 +92,22 @@
 
 }; // namespace std
 
+// CHECK: NamespaceDecl {{.*}} attributed_decl
+namespace attributed_decl {
+  void f() {
+// CHECK: DeclStmt {{.*}} 
+[[maybe_unused]] int i1;
+// CHECK: DeclStmt {{.*}} 
+__attribute__((unused)) int i2;
+// CHECK: DeclStmt {{.*}} 
+int __attribute__((unused)) i3;
+// CHECK: DeclStmt {{.*}} <:{{.*}}, {{.*}}:[[@LINE+1]]:40>
+__declspec(dllexport) extern int i4;
+// CHECK: DeclStmt {{.*}} 
+extern int __declspec(dllexport) i5;
+  }
+}
+
 #if __cplusplus >= 201703L
 // CHECK-1Z: FunctionDecl {{.*}} construct_with_init_list
 std::map construct_with_init_list() {
Index: clang/lib/Parse/ParseStmt.cpp
===
--- clang/lib/Parse/ParseStmt.cpp
+++ clang/lib/Parse/ParseStmt.cpp
@@ -220,6 +220,8 @@
 Decl =
 ParseDeclaration(DeclaratorContext::BlockContext, DeclEnd, Attrs);
   }
+  if (Attrs.Range.getBegin().isValid())
+DeclStart = Attrs.Range.getBegin();
   return Actions.ActOnDeclStmt(Decl, DeclStart, DeclEnd);
 }
 


Index: clang/test/AST/sourceranges.cpp
===
--- clang/test/AST/sourceranges.cpp
+++ clang/test/AST/sourceranges.cpp
@@ -92,6 +92,22 @@
 
 }; // namespace std
 
+// CHECK: NamespaceDecl {{.*}} attributed_decl
+namespace attributed_decl {
+  void f() {
+// CHECK: DeclStmt {{.*}} 
+[[maybe_unused]] int i1;
+// CHECK: DeclStmt {{.*}} 
+__attribute__((unused)) int i2;
+// CHECK: DeclStmt {{.*}} 
+int __attribute__((unused)) i3;
+// CHECK: DeclStmt {{.*}} <:{{.*}}, {{.*}}:[[@LINE+1]]:40>
+__declspec(dllexport) extern int i4;
+// CHECK: DeclStmt {{.*}} 
+extern int __declspec(dllexport) i5;
+  }
+}
+
 #if __cplusplus >= 201703L
 // CHECK-1Z: FunctionDecl {{.*}} construct_with_init_list
 std::map construct_with_init_list() {
Index: clang/lib/Parse/ParseStmt.cpp
===
--- clang/lib/Parse/ParseStmt.cpp
+++ clang/lib/Parse/ParseStmt.cpp
@@ -220,6 +220,8 @@
 Decl =
 ParseDeclaration(DeclaratorContext::BlockContext, DeclEnd, Attrs);
   }
+  if (Attrs.Range.getBegin().isValid())
+DeclStart = Attrs.Range.getBegin();
   return Actions.ActOnDeclStmt(Decl, DeclStart, DeclEnd);
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68581: Include leading attributes in DeclStmt's SourceRange

[Stephan Bergmann via Phabricator via cfe-commits]

sberg updated this revision to Diff 224823.

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

https://reviews.llvm.org/D68581

Files:
  clang/lib/Parse/ParseStmt.cpp
  clang/test/AST/sourceranges.cpp


Index: clang/test/AST/sourceranges.cpp
===
--- clang/test/AST/sourceranges.cpp
+++ clang/test/AST/sourceranges.cpp
@@ -144,3 +144,18 @@
   }
 }
 #endif
+
+// CHECK: NamespaceDecl {{.*}} attributed_decl
+namespace attributed_decl {
+  void cpp11() {
+// CHECK: DeclStmt {{.*}} 
+int __attribute__((unused)) i;
+  }
+
+#if __cplusplus >= 201703L
+  void cpp17() {
+// CHECK-1Z: DeclStmt {{.*}} 
+[[maybe_unused]] int i;
+  }
+#endif
+}
Index: clang/lib/Parse/ParseStmt.cpp
===
--- clang/lib/Parse/ParseStmt.cpp
+++ clang/lib/Parse/ParseStmt.cpp
@@ -220,6 +220,8 @@
 Decl =
 ParseDeclaration(DeclaratorContext::BlockContext, DeclEnd, Attrs);
   }
+  if (Attrs.Range.getBegin().isValid())
+DeclStart = Attrs.Range.getBegin();
   return Actions.ActOnDeclStmt(Decl, DeclStart, DeclEnd);
 }
 


Index: clang/test/AST/sourceranges.cpp
===
--- clang/test/AST/sourceranges.cpp
+++ clang/test/AST/sourceranges.cpp
@@ -144,3 +144,18 @@
   }
 }
 #endif
+
+// CHECK: NamespaceDecl {{.*}} attributed_decl
+namespace attributed_decl {
+  void cpp11() {
+// CHECK: DeclStmt {{.*}} 
+int __attribute__((unused)) i;
+  }
+
+#if __cplusplus >= 201703L
+  void cpp17() {
+// CHECK-1Z: DeclStmt {{.*}} 
+[[maybe_unused]] int i;
+  }
+#endif
+}
Index: clang/lib/Parse/ParseStmt.cpp
===
--- clang/lib/Parse/ParseStmt.cpp
+++ clang/lib/Parse/ParseStmt.cpp
@@ -220,6 +220,8 @@
 Decl =
 ParseDeclaration(DeclaratorContext::BlockContext, DeclEnd, Attrs);
   }
+  if (Attrs.Range.getBegin().isValid())
+DeclStart = Attrs.Range.getBegin();
   return Actions.ActOnDeclStmt(Decl, DeclStart, DeclEnd);
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68581: Include leading attributes in DeclStmt's SourceRange

[Stephan Bergmann via Phabricator via cfe-commits]

sberg updated this revision to Diff 224303.
sberg edited the summary of this revision.

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

https://reviews.llvm.org/D68581

Files:
  clang/lib/Parse/ParseStmt.cpp
  clang/test/AST/sourceranges.cpp


Index: clang/test/AST/sourceranges.cpp
===
--- clang/test/AST/sourceranges.cpp
+++ clang/test/AST/sourceranges.cpp
@@ -143,4 +143,14 @@
 new int{0};
   }
 }
+
+// CHECK-1Z: NamespaceDecl {{.*}} attributed_decl
+namespace attributed_decl {
+  void f() {
+// CHECK-1Z: DeclStmt {{.*}} 
+[[maybe_unused]] int i;
+// CHECK-1Z: DeclStmt {{.*}} 
+int __attribute__((unused)) j;
+  }
+}
 #endif
Index: clang/lib/Parse/ParseStmt.cpp
===
--- clang/lib/Parse/ParseStmt.cpp
+++ clang/lib/Parse/ParseStmt.cpp
@@ -220,6 +220,8 @@
 Decl =
 ParseDeclaration(DeclaratorContext::BlockContext, DeclEnd, Attrs);
   }
+  if (Attrs.Range.getBegin().isValid())
+DeclStart = Attrs.Range.getBegin();
   return Actions.ActOnDeclStmt(Decl, DeclStart, DeclEnd);
 }
 


Index: clang/test/AST/sourceranges.cpp
===
--- clang/test/AST/sourceranges.cpp
+++ clang/test/AST/sourceranges.cpp
@@ -143,4 +143,14 @@
 new int{0};
   }
 }
+
+// CHECK-1Z: NamespaceDecl {{.*}} attributed_decl
+namespace attributed_decl {
+  void f() {
+// CHECK-1Z: DeclStmt {{.*}} 
+[[maybe_unused]] int i;
+// CHECK-1Z: DeclStmt {{.*}} 
+int __attribute__((unused)) j;
+  }
+}
 #endif
Index: clang/lib/Parse/ParseStmt.cpp
===
--- clang/lib/Parse/ParseStmt.cpp
+++ clang/lib/Parse/ParseStmt.cpp
@@ -220,6 +220,8 @@
 Decl =
 ParseDeclaration(DeclaratorContext::BlockContext, DeclEnd, Attrs);
   }
+  if (Attrs.Range.getBegin().isValid())
+DeclStart = Attrs.Range.getBegin();
   return Actions.ActOnDeclStmt(Decl, DeclStart, DeclEnd);
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68581: Include leading attributes in DeclStmt's SourceRange

[Stephan Bergmann via Phabricator via cfe-commits]

sberg created this revision.
sberg added reviewers: aaron.ballman, rsmith, Nathan-Huckleberry.
Herald added a project: clang.

For a `DeclStmt` like

  [[maybe_unused]] int i;

`getBeginLoc()` returned the start of `int` instead of the start of `[[` (see 
http://lists.llvm.org/pipermail/cfe-dev/2019-September/063434.html "[cfe-dev] 
Poor DeclStmt source range with leading [[attributes]]?").

Is there a good way to write a test for this?


Repository:
  rC Clang

https://reviews.llvm.org/D68581

Files:
  clang/lib/Parse/ParseStmt.cpp


Index: clang/lib/Parse/ParseStmt.cpp
===
--- clang/lib/Parse/ParseStmt.cpp
+++ clang/lib/Parse/ParseStmt.cpp
@@ -220,6 +220,8 @@
 Decl =
 ParseDeclaration(DeclaratorContext::BlockContext, DeclEnd, Attrs);
   }
+  if (Attrs.Range.getBegin().isValid())
+  DeclStart = Attrs.Range.getBegin();
   return Actions.ActOnDeclStmt(Decl, DeclStart, DeclEnd);
 }
 


Index: clang/lib/Parse/ParseStmt.cpp
===
--- clang/lib/Parse/ParseStmt.cpp
+++ clang/lib/Parse/ParseStmt.cpp
@@ -220,6 +220,8 @@
 Decl =
 ParseDeclaration(DeclaratorContext::BlockContext, DeclEnd, Attrs);
   }
+  if (Attrs.Range.getBegin().isValid())
+  DeclStart = Attrs.Range.getBegin();
   return Actions.ActOnDeclStmt(Decl, DeclStart, DeclEnd);
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66350: Rudimentary support for Doxygen \retval command

[Stephan Bergmann via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL369345: Rudimentary support for Doxygen \retval command 
(authored by sberg, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D66350?vs=215599=216069#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D66350

Files:
  cfe/trunk/include/clang/AST/CommentCommands.td
  cfe/trunk/test/Sema/warn-documentation.cpp


Index: cfe/trunk/include/clang/AST/CommentCommands.td
===
--- cfe/trunk/include/clang/AST/CommentCommands.td
+++ cfe/trunk/include/clang/AST/CommentCommands.td
@@ -139,6 +139,7 @@
 def Pre: BlockCommand<"pre">;
 def Remark : BlockCommand<"remark">;
 def Remarks: BlockCommand<"remarks">;
+def Retval : BlockCommand<"retval">;
 def Sa : BlockCommand<"sa">;
 def See: BlockCommand<"see">;
 def Since  : BlockCommand<"since">;
Index: cfe/trunk/test/Sema/warn-documentation.cpp
===
--- cfe/trunk/test/Sema/warn-documentation.cpp
+++ cfe/trunk/test/Sema/warn-documentation.cpp
@@ -288,6 +288,11 @@
 /// \param x2 Ccc.
 int test_param22(int x1, int x2, int x3);
 
+// expected-warning@+1 {{empty paragraph passed to '\param' command}}
+/// \param a
+/// \retval 0 Blah blah.
+int test_param23(int a);
+
 //===---
 // Test that we treat typedefs to some non-function types as functions for the
 // purposes of documentation comment parsing.


Index: cfe/trunk/include/clang/AST/CommentCommands.td
===
--- cfe/trunk/include/clang/AST/CommentCommands.td
+++ cfe/trunk/include/clang/AST/CommentCommands.td
@@ -139,6 +139,7 @@
 def Pre: BlockCommand<"pre">;
 def Remark : BlockCommand<"remark">;
 def Remarks: BlockCommand<"remarks">;
+def Retval : BlockCommand<"retval">;
 def Sa : BlockCommand<"sa">;
 def See: BlockCommand<"see">;
 def Since  : BlockCommand<"since">;
Index: cfe/trunk/test/Sema/warn-documentation.cpp
===
--- cfe/trunk/test/Sema/warn-documentation.cpp
+++ cfe/trunk/test/Sema/warn-documentation.cpp
@@ -288,6 +288,11 @@
 /// \param x2 Ccc.
 int test_param22(int x1, int x2, int x3);
 
+// expected-warning@+1 {{empty paragraph passed to '\param' command}}
+/// \param a
+/// \retval 0 Blah blah.
+int test_param23(int a);
+
 //===---
 // Test that we treat typedefs to some non-function types as functions for the
 // purposes of documentation comment parsing.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66350: Rudimentary support for Doxygen \retval command

[Stephan Bergmann via Phabricator via cfe-commits]

sberg created this revision.
sberg added a reviewer: gribozavr.
sberg added a project: clang.
Herald added a subscriber: cfe-commits.

...so that at least a preceding \param etc. that lacks a description gets a 
-Wdocumentation warning (instead of erroneously treating the \retval ... text 
as its paragraph).


Repository:
  rC Clang

https://reviews.llvm.org/D66350

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


Index: clang/test/Sema/warn-documentation.cpp
===
--- clang/test/Sema/warn-documentation.cpp
+++ clang/test/Sema/warn-documentation.cpp
@@ -288,6 +288,11 @@
 /// \param x2 Ccc.
 int test_param22(int x1, int x2, int x3);
 
+// expected-warning@+1 {{empty paragraph passed to '\param' command}}
+/// \param a
+/// \retval 0 Blah blah.
+int test_param23(int a);
+
 //===---
 // Test that we treat typedefs to some non-function types as functions for the
 // purposes of documentation comment parsing.
Index: clang/include/clang/AST/CommentCommands.td
===
--- clang/include/clang/AST/CommentCommands.td
+++ clang/include/clang/AST/CommentCommands.td
@@ -139,6 +139,7 @@
 def Pre: BlockCommand<"pre">;
 def Remark : BlockCommand<"remark">;
 def Remarks: BlockCommand<"remarks">;
+def Retval : BlockCommand<"retval">;
 def Sa : BlockCommand<"sa">;
 def See: BlockCommand<"see">;
 def Since  : BlockCommand<"since">;


Index: clang/test/Sema/warn-documentation.cpp
===
--- clang/test/Sema/warn-documentation.cpp
+++ clang/test/Sema/warn-documentation.cpp
@@ -288,6 +288,11 @@
 /// \param x2 Ccc.
 int test_param22(int x1, int x2, int x3);
 
+// expected-warning@+1 {{empty paragraph passed to '\param' command}}
+/// \param a
+/// \retval 0 Blah blah.
+int test_param23(int a);
+
 //===---
 // Test that we treat typedefs to some non-function types as functions for the
 // purposes of documentation comment parsing.
Index: clang/include/clang/AST/CommentCommands.td
===
--- clang/include/clang/AST/CommentCommands.td
+++ clang/include/clang/AST/CommentCommands.td
@@ -139,6 +139,7 @@
 def Pre: BlockCommand<"pre">;
 def Remark : BlockCommand<"remark">;
 def Remarks: BlockCommand<"remarks">;
+def Retval : BlockCommand<"retval">;
 def Sa : BlockCommand<"sa">;
 def See: BlockCommand<"see">;
 def Since  : BlockCommand<"since">;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61479: Finish "Adapt -fsanitize=function to SANITIZER_NON_UNIQUE_TYPEINFO"

[Stephan Bergmann via Phabricator via cfe-commits]

sberg added a comment.

eh, the summary here doesn't get updated from the actual git commit message :(

thanks for the reviews!


Repository:
  rL LLVM

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

https://reviews.llvm.org/D61479



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

[PATCH] D61479: Finish "Adapt -fsanitize=function to SANITIZER_NON_UNIQUE_TYPEINFO"

[Stephan Bergmann via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL366186: Finish Adapt -fsanitize=function to 
SANITIZER_NON_UNIQUE_TYPEINFO (authored by sberg, committed by ).
Herald added a subscriber: delcypher.

Changed prior to commit:
  https://reviews.llvm.org/D61479?vs=209865=210025#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D61479

Files:
  cfe/trunk/docs/UndefinedBehaviorSanitizer.rst
  cfe/trunk/lib/CodeGen/CGExpr.cpp
  cfe/trunk/lib/CodeGen/CodeGenFunction.h
  cfe/trunk/lib/Driver/SanitizerArgs.cpp
  cfe/trunk/test/CodeGen/ubsan-function.cpp
  cfe/trunk/test/Driver/fsanitize.c
  compiler-rt/trunk/lib/ubsan/ubsan_handlers_cxx.cc
  compiler-rt/trunk/lib/ubsan/ubsan_handlers_cxx.h
  compiler-rt/trunk/lib/ubsan/ubsan_interface.inc
  compiler-rt/trunk/test/ubsan/TestCases/TypeCheck/Function/function.cpp

Index: compiler-rt/trunk/test/ubsan/TestCases/TypeCheck/Function/function.cpp
===
--- compiler-rt/trunk/test/ubsan/TestCases/TypeCheck/Function/function.cpp
+++ compiler-rt/trunk/test/ubsan/TestCases/TypeCheck/Function/function.cpp
@@ -1,11 +1,53 @@
-// RUN: %clangxx -std=c++17 -fsanitize=function %s -O3 -g -o %t
-// RUN: %run %t 2>&1 | FileCheck %s
+// RUN: %clangxx -DDETERMINE_UNIQUE %s -o %t-unique
+// RUN: %clangxx -std=c++17 -fsanitize=function %s -O3 -g -DSHARED_LIB -fPIC -shared -o %t-so.so
+// RUN: %clangxx -std=c++17 -fsanitize=function %s -O3 -g -o %t %t-so.so
+// RUN: %run %t 2>&1 | FileCheck %s --check-prefix=CHECK $(%run %t-unique UNIQUE)
 // Verify that we can disable symbolization if needed:
-// RUN: %env_ubsan_opts=symbolize=0 %run %t 2>&1 | FileCheck %s --check-prefix=NOSYM
+// RUN: %env_ubsan_opts=symbolize=0 %run %t 2>&1 | FileCheck %s --check-prefix=NOSYM $(%run %t-unique NOSYM-UNIQUE)
 // XFAIL: windows-msvc
 // Unsupported function flag
 // UNSUPPORTED: openbsd
 
+#ifdef DETERMINE_UNIQUE
+
+#include 
+
+#include "../../../../../lib/sanitizer_common/sanitizer_platform.h"
+
+int main(int, char **argv) {
+  if (!SANITIZER_NON_UNIQUE_TYPEINFO)
+std::cout << "--check-prefix=" << argv[1];
+}
+
+#else
+
+struct Shared {};
+using FnShared = void (*)(Shared *);
+FnShared getShared();
+
+struct __attribute__((visibility("hidden"))) Hidden {};
+using FnHidden = void (*)(Hidden *);
+FnHidden getHidden();
+
+namespace {
+struct Private {};
+} // namespace
+using FnPrivate = void (*)(void *);
+FnPrivate getPrivate();
+
+#ifdef SHARED_LIB
+
+void fnShared(Shared *) {}
+FnShared getShared() { return fnShared; }
+
+void fnHidden(Hidden *) {}
+FnHidden getHidden() { return fnHidden; }
+
+void fnPrivate(Private *) {}
+FnPrivate getPrivate() { return reinterpret_cast(fnPrivate); }
+
+#else
+
 #include 
 
 void f() {}
@@ -64,12 +106,31 @@
   p2(0);
 }
 
+void check_cross_dso() {
+  getShared()(nullptr);
+
+  // UNIQUE: function.cpp:[[@LINE+2]]:3: runtime error: call to function fnHidden(Hidden*) through pointer to incorrect function type 'void (*)(Hidden *)'
+  // NOSYM-UNIQUE: function.cpp:[[@LINE+1]]:3: runtime error: call to function (unknown) through pointer to incorrect function type 'void (*)(Hidden *)'
+  getHidden()(nullptr);
+
+  // TODO: Unlike GCC, Clang fails to prefix the typeinfo name for the function
+  // type with "*", so this erroneously only fails for "*UNIQUE":
+  // UNIQUE: function.cpp:[[@LINE+2]]:3: runtime error: call to function fnPrivate((anonymous namespace)::Private*) through pointer to incorrect function type 'void (*)((anonymous namespace)::Private *)'
+  // NOSYM-UNIQUE: function.cpp:[[@LINE+1]]:3: runtime error: call to function (unknown) through pointer to incorrect function type 'void (*)((anonymous namespace)::Private *)'
+  reinterpret_cast(getPrivate())(nullptr);
+}
+
 int main(void) {
   make_valid_call();
   make_invalid_call();
   check_noexcept_calls();
+  check_cross_dso();
   // Check that no more errors will be printed.
   // CHECK-NOT: runtime error: call to function
   // NOSYM-NOT: runtime error: call to function
   make_invalid_call();
 }
+
+#endif
+
+#endif
Index: compiler-rt/trunk/lib/ubsan/ubsan_interface.inc
===
--- compiler-rt/trunk/lib/ubsan/ubsan_interface.inc
+++ compiler-rt/trunk/lib/ubsan/ubsan_interface.inc
@@ -21,8 +21,8 @@
 INTERFACE_FUNCTION(__ubsan_handle_dynamic_type_cache_miss_abort)
 INTERFACE_FUNCTION(__ubsan_handle_float_cast_overflow)
 INTERFACE_FUNCTION(__ubsan_handle_float_cast_overflow_abort)
-INTERFACE_FUNCTION(__ubsan_handle_function_type_mismatch)
-INTERFACE_FUNCTION(__ubsan_handle_function_type_mismatch_abort)
+INTERFACE_FUNCTION(__ubsan_handle_function_type_mismatch_v1)
+INTERFACE_FUNCTION(__ubsan_handle_function_type_mismatch_v1_abort)
 INTERFACE_FUNCTION(__ubsan_handle_implicit_conversion)
 INTERFACE_FUNCTION(__ubsan_handle_implicit_conversion_abort)
 

[PATCH] D61479: Finish "Adapt -fsanitize=function to SANITIZER_NON_UNIQUE_TYPEINFO"

[Stephan Bergmann via Phabricator via cfe-commits]

sberg updated this revision to Diff 209865.
sberg added a comment.

- Disallowed -fsanitize=function in combination with -fsanitize-minimal-runtime 
now.

- Left the ubsan test's RUN lines and \#include magic as-is, if there's no 
clearly better way to avoid this unfortunate complexity.


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

https://reviews.llvm.org/D61479

Files:
  clang/docs/UndefinedBehaviorSanitizer.rst
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Driver/SanitizerArgs.cpp
  clang/test/CodeGen/ubsan-function.cpp
  clang/test/Driver/fsanitize.c
  compiler-rt/lib/ubsan/ubsan_handlers_cxx.cc
  compiler-rt/lib/ubsan/ubsan_handlers_cxx.h
  compiler-rt/lib/ubsan/ubsan_interface.inc
  compiler-rt/test/ubsan/TestCases/TypeCheck/Function/function.cpp

Index: compiler-rt/test/ubsan/TestCases/TypeCheck/Function/function.cpp
===
--- compiler-rt/test/ubsan/TestCases/TypeCheck/Function/function.cpp
+++ compiler-rt/test/ubsan/TestCases/TypeCheck/Function/function.cpp
@@ -1,11 +1,53 @@
-// RUN: %clangxx -std=c++17 -fsanitize=function %s -O3 -g -o %t
-// RUN: %run %t 2>&1 | FileCheck %s
+// RUN: %clangxx -DDETERMINE_UNIQUE %s -o %t-unique
+// RUN: %clangxx -std=c++17 -fsanitize=function %s -O3 -g -DSHARED_LIB -fPIC -shared -o %t-so.so
+// RUN: %clangxx -std=c++17 -fsanitize=function %s -O3 -g -o %t %t-so.so
+// RUN: %run %t 2>&1 | FileCheck %s --check-prefix=CHECK $(%run %t-unique UNIQUE)
 // Verify that we can disable symbolization if needed:
-// RUN: %env_ubsan_opts=symbolize=0 %run %t 2>&1 | FileCheck %s --check-prefix=NOSYM
+// RUN: %env_ubsan_opts=symbolize=0 %run %t 2>&1 | FileCheck %s --check-prefix=NOSYM $(%run %t-unique NOSYM-UNIQUE)
 // XFAIL: windows-msvc
 // Unsupported function flag
 // UNSUPPORTED: openbsd
 
+#ifdef DETERMINE_UNIQUE
+
+#include 
+
+#include "../../../../../lib/sanitizer_common/sanitizer_platform.h"
+
+int main(int, char **argv) {
+  if (!SANITIZER_NON_UNIQUE_TYPEINFO)
+std::cout << "--check-prefix=" << argv[1];
+}
+
+#else
+
+struct Shared {};
+using FnShared = void (*)(Shared *);
+FnShared getShared();
+
+struct __attribute__((visibility("hidden"))) Hidden {};
+using FnHidden = void (*)(Hidden *);
+FnHidden getHidden();
+
+namespace {
+struct Private {};
+} // namespace
+using FnPrivate = void (*)(void *);
+FnPrivate getPrivate();
+
+#ifdef SHARED_LIB
+
+void fnShared(Shared *) {}
+FnShared getShared() { return fnShared; }
+
+void fnHidden(Hidden *) {}
+FnHidden getHidden() { return fnHidden; }
+
+void fnPrivate(Private *) {}
+FnPrivate getPrivate() { return reinterpret_cast(fnPrivate); }
+
+#else
+
 #include 
 
 void f() {}
@@ -64,12 +106,31 @@
   p2(0);
 }
 
+void check_cross_dso() {
+  getShared()(nullptr);
+
+  // UNIQUE: function.cpp:[[@LINE+2]]:3: runtime error: call to function fnHidden(Hidden*) through pointer to incorrect function type 'void (*)(Hidden *)'
+  // NOSYM-UNIQUE: function.cpp:[[@LINE+1]]:3: runtime error: call to function (unknown) through pointer to incorrect function type 'void (*)(Hidden *)'
+  getHidden()(nullptr);
+
+  // TODO: Unlike GCC, Clang fails to prefix the typeinfo name for the function
+  // type with "*", so this erroneously only fails for "*UNIQUE":
+  // UNIQUE: function.cpp:[[@LINE+2]]:3: runtime error: call to function fnPrivate((anonymous namespace)::Private*) through pointer to incorrect function type 'void (*)((anonymous namespace)::Private *)'
+  // NOSYM-UNIQUE: function.cpp:[[@LINE+1]]:3: runtime error: call to function (unknown) through pointer to incorrect function type 'void (*)((anonymous namespace)::Private *)'
+  reinterpret_cast(getPrivate())(nullptr);
+}
+
 int main(void) {
   make_valid_call();
   make_invalid_call();
   check_noexcept_calls();
+  check_cross_dso();
   // Check that no more errors will be printed.
   // CHECK-NOT: runtime error: call to function
   // NOSYM-NOT: runtime error: call to function
   make_invalid_call();
 }
+
+#endif
+
+#endif
Index: compiler-rt/lib/ubsan/ubsan_interface.inc
===
--- compiler-rt/lib/ubsan/ubsan_interface.inc
+++ compiler-rt/lib/ubsan/ubsan_interface.inc
@@ -21,8 +21,8 @@
 INTERFACE_FUNCTION(__ubsan_handle_dynamic_type_cache_miss_abort)
 INTERFACE_FUNCTION(__ubsan_handle_float_cast_overflow)
 INTERFACE_FUNCTION(__ubsan_handle_float_cast_overflow_abort)
-INTERFACE_FUNCTION(__ubsan_handle_function_type_mismatch)
-INTERFACE_FUNCTION(__ubsan_handle_function_type_mismatch_abort)
+INTERFACE_FUNCTION(__ubsan_handle_function_type_mismatch_v1)
+INTERFACE_FUNCTION(__ubsan_handle_function_type_mismatch_v1_abort)
 INTERFACE_FUNCTION(__ubsan_handle_implicit_conversion)
 INTERFACE_FUNCTION(__ubsan_handle_implicit_conversion_abort)
 INTERFACE_FUNCTION(__ubsan_handle_invalid_builtin)
Index: compiler-rt/lib/ubsan/ubsan_handlers_cxx.h
===
--- 

[PATCH] D61479: Finish "Adapt -fsanitize=function to SANITIZER_NON_UNIQUE_TYPEINFO"

[Stephan Bergmann via Phabricator via cfe-commits]

sberg added a subscriber: cfe-commits.
sberg added a comment.

Any thoughts on this?  (cfe-commits had inadvertently been missing from 
subscribers, it touches clang as well as compiler-rt.)


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

https://reviews.llvm.org/D61479



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

[PATCH] D60760: Adapt -fsanitize=function to SANITIZER_NON_UNIQUE_TYPEINFO

[Stephan Bergmann via Phabricator via cfe-commits]

sberg added a comment.

Added missing tests at https://reviews.llvm.org/D61479 "Add tests for 'Adapt 
-fsanitize=function to SANITIZER_NON_UNIQUE_TYPEINFO'".


Repository:
  rL LLVM

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

https://reviews.llvm.org/D60760



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

[PATCH] D60760: Adapt -fsanitize=function to SANITIZER_NON_UNIQUE_TYPEINFO

[Stephan Bergmann via Phabricator via cfe-commits]

sberg added a comment.

In D60760#1487342 , @lebedev.ri wrote:

> Did this get reviewed?


I didn't get any responses at all, so decided to push it anyway.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D60760



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

[PATCH] D60760: Adapt -fsanitize=function to SANITIZER_NON_UNIQUE_TYPEINFO

[Stephan Bergmann via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL359759: Adapt -fsanitize=function to 
SANITIZER_NON_UNIQUE_TYPEINFO (authored by sberg, committed by ).
Herald added a subscriber: delcypher.

Changed prior to commit:
  https://reviews.llvm.org/D60760?vs=195319=197706#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D60760

Files:
  cfe/trunk/lib/CodeGen/CGExpr.cpp
  compiler-rt/trunk/lib/ubsan/ubsan_handlers.cc
  compiler-rt/trunk/lib/ubsan/ubsan_handlers.h
  compiler-rt/trunk/lib/ubsan/ubsan_handlers_cxx.cc
  compiler-rt/trunk/lib/ubsan/ubsan_handlers_cxx.h
  compiler-rt/trunk/lib/ubsan/ubsan_type_hash.h
  compiler-rt/trunk/lib/ubsan/ubsan_type_hash_itanium.cc
  compiler-rt/trunk/lib/ubsan/ubsan_type_hash_win.cc

Index: cfe/trunk/lib/CodeGen/CGExpr.cpp
===
--- cfe/trunk/lib/CodeGen/CGExpr.cpp
+++ cfe/trunk/lib/CodeGen/CGExpr.cpp
@@ -4672,7 +4672,8 @@
   llvm::Constant *StaticData[] = {EmitCheckSourceLocation(E->getBeginLoc()),
   EmitCheckTypeDescriptor(CalleeType)};
   EmitCheck(std::make_pair(CalleeRTTIMatch, SanitizerKind::Function),
-SanitizerHandler::FunctionTypeMismatch, StaticData, CalleePtr);
+SanitizerHandler::FunctionTypeMismatch, StaticData,
+{CalleePtr, CalleeRTTI, FTRTTIConst});
 
   Builder.CreateBr(Cont);
   EmitBlock(Cont);
Index: compiler-rt/trunk/lib/ubsan/ubsan_type_hash_itanium.cc
===
--- compiler-rt/trunk/lib/ubsan/ubsan_type_hash_itanium.cc
+++ compiler-rt/trunk/lib/ubsan/ubsan_type_hash_itanium.cc
@@ -117,9 +117,7 @@
   const abi::__class_type_info *Base,
   sptr Offset) {
   if (Derived->__type_name == Base->__type_name ||
-  (SANITIZER_NON_UNIQUE_TYPEINFO &&
-   Derived->__type_name[0] != '*' &&
-   !internal_strcmp(Derived->__type_name, Base->__type_name)))
+  __ubsan::checkTypeInfoEquality(Derived, Base))
 return Offset == 0;
 
   if (const abi::__si_class_type_info *SI =
@@ -258,4 +256,13 @@
  ObjectType ? ObjectType->__type_name : "");
 }
 
+bool __ubsan::checkTypeInfoEquality(const void *TypeInfo1,
+const void *TypeInfo2) {
+  auto TI1 = static_cast(TypeInfo1);
+  auto TI2 = static_cast(TypeInfo2);
+  return SANITIZER_NON_UNIQUE_TYPEINFO && TI1->__type_name[0] != '*' &&
+ TI2->__type_name[0] != '*' &&
+ !internal_strcmp(TI1->__type_name, TI2->__type_name);
+}
+
 #endif  // CAN_SANITIZE_UB && !SANITIZER_WINDOWS
Index: compiler-rt/trunk/lib/ubsan/ubsan_handlers.h
===
--- compiler-rt/trunk/lib/ubsan/ubsan_handlers.h
+++ compiler-rt/trunk/lib/ubsan/ubsan_handlers.h
@@ -168,15 +168,6 @@
 /// Handle a builtin called in an invalid way.
 RECOVERABLE(invalid_builtin, InvalidBuiltinData *Data)
 
-struct FunctionTypeMismatchData {
-  SourceLocation Loc;
-  const TypeDescriptor 
-};
-
-RECOVERABLE(function_type_mismatch,
-FunctionTypeMismatchData *Data,
-ValueHandle Val)
-
 struct NonNullReturnData {
   SourceLocation AttrLoc;
 };
Index: compiler-rt/trunk/lib/ubsan/ubsan_type_hash_win.cc
===
--- compiler-rt/trunk/lib/ubsan/ubsan_type_hash_win.cc
+++ compiler-rt/trunk/lib/ubsan/ubsan_type_hash_win.cc
@@ -77,4 +77,9 @@
  "");
 }
 
+bool __ubsan::checkTypeInfoEquality(const std::type_info *,
+const std::type_info *) {
+  return false;
+}
+
 #endif  // CAN_SANITIZE_UB && SANITIZER_WINDOWS
Index: compiler-rt/trunk/lib/ubsan/ubsan_handlers_cxx.cc
===
--- compiler-rt/trunk/lib/ubsan/ubsan_handlers_cxx.cc
+++ compiler-rt/trunk/lib/ubsan/ubsan_handlers_cxx.cc
@@ -156,6 +156,51 @@
 Diag(Loc, DL_Note, ET, "check failed in %0, vtable located in %1")
 << SrcModule << DstModule;
 }
+
+static bool handleFunctionTypeMismatch(FunctionTypeMismatchData *Data,
+   ValueHandle Function,
+   ValueHandle calleeRTTI,
+   ValueHandle fnRTTI, ReportOptions Opts) {
+  if (checkTypeInfoEquality(reinterpret_cast(calleeRTTI),
+reinterpret_cast(fnRTTI)))
+return false;
+
+  SourceLocation CallLoc = Data->Loc.acquire();
+  ErrorType ET = ErrorType::FunctionTypeMismatch;
+
+  if (ignoreReport(CallLoc, Opts, ET))
+return true;
+
+  ScopedReport R(Opts, CallLoc, ET);
+
+  SymbolizedStackHolder 

[PATCH] D60760: Adapt -fsanitize=function to SANITIZER_NON_UNIQUE_TYPEINFO

[Stephan Bergmann via Phabricator via cfe-commits]

sberg added a comment.

friendly ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60760



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

[PATCH] D60760: Adapt -fsanitize=function to SANITIZER_NON_UNIQUE_TYPEINFO

[Stephan Bergmann via Phabricator via cfe-commits]

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



Comment at: compiler-rt/lib/ubsan/ubsan_type_hash_itanium.cc:264
+  return SANITIZER_NON_UNIQUE_TYPEINFO && TI1->__type_name[0] != '*' &&
+ TI2->__type_name[0] != '*' &&
+ !internal_strcmp(TI1->__type_name, TI2->__type_name);

My understanding of that GCC * prefix hack is that would should check 
symmetrically for both type_infos (which is different how isDerivedFromAtOffset 
above did it in the past)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60760



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

[PATCH] D60760: Adapt -fsanitize=function to SANITIZER_NON_UNIQUE_TYPEINFO

[Stephan Bergmann via Phabricator via cfe-commits]

sberg created this revision.
sberg added reviewers: filcab, marxin, rsmith.
Herald added subscribers: llvm-commits, Sanitizers, cfe-commits, kristof.beyls, 
javed.absar, kubamracek.
Herald added projects: clang, Sanitizers, LLVM.

This follows up after b7692bc3e9ad2691fc07261904b88fb15f30696b 
 "[UBSan] 
Fix
isDerivedFromAtOffset on iOS ARM64" fixed the RTTI comparison in
isDerivedFromAtOffset on just one platform and then
a25a2c7c9a7e1e328a5bd8274d2d86b1fadc4692 
 "Always 
compare C++ typeinfo (based on
libstdc++ implementation)" extended that fix to more platforms.

But there is another RTTI comparison for -fsanitize=function generated in
clang's CodeGenFunction::EmitCall as just a pointer comparison.  For
SANITIZER_NON_UNIQUE_TYPEINFO platforms this needs to be extended to also do
string comparison.  For that, __ubsan_handle_function_type_mismatch[_abort]
takes the two std::type_info pointers as additional parameters now, checks them
internally for potential equivalence, and returns without reporting failure if
they turn out to be equivalent after all.  (NORETURN needed to be dropped from
the _abort variant for that.)  Also these functions depend on ABI-specific RTTI
now, so needed to be moved from plain UBSAN_SOURCES (ubsan_handlers.h/cc) to
UBSAN_CXXABI_SOURCES (ubsan_handlers_cxx.h/cc), but as -fsanitize=function is
only supported in C++ mode that's not a problem.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D60760

Files:
  clang/lib/CodeGen/CGExpr.cpp
  compiler-rt/lib/ubsan/ubsan_handlers.cc
  compiler-rt/lib/ubsan/ubsan_handlers.h
  compiler-rt/lib/ubsan/ubsan_handlers_cxx.cc
  compiler-rt/lib/ubsan/ubsan_handlers_cxx.h
  compiler-rt/lib/ubsan/ubsan_type_hash.h
  compiler-rt/lib/ubsan/ubsan_type_hash_itanium.cc
  compiler-rt/lib/ubsan/ubsan_type_hash_win.cc

Index: compiler-rt/lib/ubsan/ubsan_type_hash_win.cc
===
--- compiler-rt/lib/ubsan/ubsan_type_hash_win.cc
+++ compiler-rt/lib/ubsan/ubsan_type_hash_win.cc
@@ -77,4 +77,9 @@
  "");
 }
 
+bool __ubsan::checkTypeInfoEquality(const std::type_info *,
+const std::type_info *) {
+  return false;
+}
+
 #endif  // CAN_SANITIZE_UB && SANITIZER_WINDOWS
Index: compiler-rt/lib/ubsan/ubsan_type_hash_itanium.cc
===
--- compiler-rt/lib/ubsan/ubsan_type_hash_itanium.cc
+++ compiler-rt/lib/ubsan/ubsan_type_hash_itanium.cc
@@ -117,9 +117,7 @@
   const abi::__class_type_info *Base,
   sptr Offset) {
   if (Derived->__type_name == Base->__type_name ||
-  (SANITIZER_NON_UNIQUE_TYPEINFO &&
-   Derived->__type_name[0] != '*' &&
-   !internal_strcmp(Derived->__type_name, Base->__type_name)))
+  __ubsan::checkTypeInfoEquality(Derived, Base))
 return Offset == 0;
 
   if (const abi::__si_class_type_info *SI =
@@ -258,4 +256,13 @@
  ObjectType ? ObjectType->__type_name : "");
 }
 
+bool __ubsan::checkTypeInfoEquality(const void *TypeInfo1,
+const void *TypeInfo2) {
+  auto TI1 = static_cast(TypeInfo1);
+  auto TI2 = static_cast(TypeInfo2);
+  return SANITIZER_NON_UNIQUE_TYPEINFO && TI1->__type_name[0] != '*' &&
+ TI2->__type_name[0] != '*' &&
+ !internal_strcmp(TI1->__type_name, TI2->__type_name);
+}
+
 #endif  // CAN_SANITIZE_UB && !SANITIZER_WINDOWS
Index: compiler-rt/lib/ubsan/ubsan_type_hash.h
===
--- compiler-rt/lib/ubsan/ubsan_type_hash.h
+++ compiler-rt/lib/ubsan/ubsan_type_hash.h
@@ -64,6 +64,10 @@
 extern "C" SANITIZER_INTERFACE_ATTRIBUTE
 HashValue __ubsan_vptr_type_cache[VptrTypeCacheSize];
 
+/// \brief Do whatever is required by the ABI to check for std::type_info
+/// equivalence beyond simple pointer comparison.
+bool checkTypeInfoEquality(const void *TypeInfo1, const void *TypeInfo2);
+
 } // namespace __ubsan
 
 #endif // UBSAN_TYPE_HASH_H
Index: compiler-rt/lib/ubsan/ubsan_handlers_cxx.h
===
--- compiler-rt/lib/ubsan/ubsan_handlers_cxx.h
+++ compiler-rt/lib/ubsan/ubsan_handlers_cxx.h
@@ -33,6 +33,21 @@
 extern "C" SANITIZER_INTERFACE_ATTRIBUTE
 void __ubsan_handle_dynamic_type_cache_miss_abort(
   DynamicTypeCacheMissData *Data, ValueHandle Pointer, ValueHandle Hash);
+
+struct FunctionTypeMismatchData {
+  SourceLocation Loc;
+  const TypeDescriptor 
+};
+
+extern "C" SANITIZER_INTERFACE_ATTRIBUTE void
+__ubsan_handle_function_type_mismatch(FunctionTypeMismatchData *Data,
+  ValueHandle Val, ValueHandle calleeRTTI,
+  ValueHandle fnRTTI);
+extern "C" 

[PATCH] D58056: Look through typedefs in getFunctionTypeWithExceptionSpec

[Stephan Bergmann via Phabricator via cfe-commits]

sberg added a comment.

committed for now to get the crash fixed; if there are issues with the test 
they can be addressed later


Repository:
  rC Clang

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

https://reviews.llvm.org/D58056



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