[PATCH] D76189: [Fuchsia] Use -ffile-prefix-map

2020-03-14 Thread Petr Hosek via Phabricator via cfe-commits 
phosek created this revision.
phosek added a reviewer: leonardchan.
Herald added subscribers: cfe-commits, aprantl, mgorny.
Herald added a project: clang.

This makes toolchain independent of the path it was built in by
rewriting all absolute paths embedded in sources and debug info
into relative ones.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D76189

Files:
  clang/cmake/caches/Fuchsia-stage2.cmake


Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -16,7 +16,7 @@
 set(LLVM_INCLUDE_DOCS OFF CACHE BOOL "")
 set(LLVM_INCLUDE_EXAMPLES OFF CACHE BOOL "")
 set(LLVM_INCLUDE_GO_TESTS OFF CACHE BOOL "")
-set(LLVM_USE_RELATIVE_PATHS_IN_DEBUG_INFO ON CACHE BOOL "")
+set(LLVM_USE_RELATIVE_PATHS_IN_FILES ON CACHE BOOL "")
 
 set(CLANG_DEFAULT_CXX_STDLIB libc++ CACHE STRING "")
 if(NOT APPLE)
@@ -72,6 +72,7 @@
 set(BUILTINS_${target}_CMAKE_SHARED_LINKER_FLAGS "-fuse-ld=lld" CACHE 
STRING "")
 set(BUILTINS_${target}_CMAKE_MODULE_LINKER_FLAGS "-fuse-ld=lld" CACHE 
STRING "")
 set(BUILTINS_${target}_CMAKE_EXE_LINKER_FLAG "-fuse-ld=lld" CACHE STRING 
"")
+set(BUILTINS_${target}_LLVM_USE_RELATIVE_PATHS_IN_FILES ON CACHE BOOL "")
 
 # Set the per-target runtimes options.
 list(APPEND RUNTIME_TARGETS "${target}")
@@ -82,6 +83,7 @@
 set(RUNTIMES_${target}_CMAKE_MODULE_LINKER_FLAGS "-fuse-ld=lld" CACHE 
STRING "")
 set(RUNTIMES_${target}_CMAKE_EXE_LINKER_FLAGS "-fuse-ld=lld" CACHE STRING 
"")
 set(RUNTIMES_${target}_LLVM_ENABLE_ASSERTIONS ON CACHE BOOL "")
+set(RUNTIMES_${target}_LLVM_USE_RELATIVE_PATHS_IN_FILES ON CACHE BOOL "")
 set(RUNTIMES_${target}_LIBUNWIND_ENABLE_SHARED OFF CACHE BOOL "")
 set(RUNTIMES_${target}_LIBUNWIND_USE_COMPILER_RT ON CACHE BOOL "")
 set(RUNTIMES_${target}_LIBUNWIND_INSTALL_LIBRARY OFF CACHE BOOL "")
@@ -125,6 +127,7 @@
 set(BUILTINS_${target}-unknown-fuchsia_CMAKE_MODULE_LINKER_FLAGS 
${FUCHSIA_${target}_LINKER_FLAGS} CACHE STRING "")
 set(BUILTINS_${target}-unknown-fuchsia_CMAKE_EXE_LINKER_FLAGS 
${FUCHSIA_${target}_LINKER_FLAGS} CACHE STRING "")
 set(BUILTINS_${target}-unknown-fuchsia_CMAKE_SYSROOT 
${FUCHSIA_${target}_SYSROOT} CACHE PATH "")
+set(BUILTINS_${target}-unknown-fuchsia_LLVM_USE_RELATIVE_PATHS_IN_FILES ON 
CACHE BOOL "")
   endforeach()
 
   foreach(target x86_64;aarch64)
@@ -141,6 +144,7 @@
 set(RUNTIMES_${target}-unknown-fuchsia_CMAKE_EXE_LINKER_FLAGS 
${FUCHSIA_${target}_LINKER_FLAGS} CACHE STRING "")
 set(RUNTIMES_${target}-unknown-fuchsia_CMAKE_SYSROOT 
${FUCHSIA_${target}_SYSROOT} CACHE PATH "")
 set(RUNTIMES_${target}-unknown-fuchsia_LLVM_ENABLE_ASSERTIONS ON CACHE 
BOOL "")
+set(RUNTIMES_${target}-unknown-fuchsia_LLVM_USE_RELATIVE_PATHS_IN_FILES ON 
CACHE BOOL "")
 set(RUNTIMES_${target}-unknown-fuchsia_LIBUNWIND_USE_COMPILER_RT ON CACHE 
BOOL "")
 set(RUNTIMES_${target}-unknown-fuchsia_LIBUNWIND_HERMETIC_STATIC_LIBRARY 
ON CACHE BOOL "")
 set(RUNTIMES_${target}-unknown-fuchsia_LIBUNWIND_INSTALL_STATIC_LIBRARY 
OFF CACHE BOOL "")


Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -16,7 +16,7 @@
 set(LLVM_INCLUDE_DOCS OFF CACHE BOOL "")
 set(LLVM_INCLUDE_EXAMPLES OFF CACHE BOOL "")
 set(LLVM_INCLUDE_GO_TESTS OFF CACHE BOOL "")
-set(LLVM_USE_RELATIVE_PATHS_IN_DEBUG_INFO ON CACHE BOOL "")
+set(LLVM_USE_RELATIVE_PATHS_IN_FILES ON CACHE BOOL "")
 
 set(CLANG_DEFAULT_CXX_STDLIB libc++ CACHE STRING "")
 if(NOT APPLE)
@@ -72,6 +72,7 @@
 set(BUILTINS_${target}_CMAKE_SHARED_LINKER_FLAGS "-fuse-ld=lld" CACHE STRING "")
 set(BUILTINS_${target}_CMAKE_MODULE_LINKER_FLAGS "-fuse-ld=lld" CACHE STRING "")
 set(BUILTINS_${target}_CMAKE_EXE_LINKER_FLAG "-fuse-ld=lld" CACHE STRING "")
+set(BUILTINS_${target}_LLVM_USE_RELATIVE_PATHS_IN_FILES ON CACHE BOOL "")
 
 # Set the per-target runtimes options.
 list(APPEND RUNTIME_TARGETS "${target}")
@@ -82,6 +83,7 @@
 set(RUNTIMES_${target}_CMAKE_MODULE_LINKER_FLAGS "-fuse-ld=lld" CACHE STRING "")
 set(RUNTIMES_${target}_CMAKE_EXE_LINKER_FLAGS "-fuse-ld=lld" CACHE STRING "")
 set(RUNTIMES_${target}_LLVM_ENABLE_ASSERTIONS ON CACHE BOOL "")
+set(RUNTIMES_${target}_LLVM_USE_RELATIVE_PATHS_IN_FILES ON CACHE BOOL "")
 set(RUNTIMES_${target}_LIBUNWIND_ENABLE_SHARED OFF CACHE BOOL "")
 set(RUNTIMES_${target}_LIBUNWIND_USE_COMPILER_RT ON CACHE BOOL "")
 set(RUNTIMES_${target}_LIBUNWIND_INSTALL_LIBRARY OFF CACHE BOOL "")
@@ -125,6 +127,7 @@
 set(BUILTINS_${target}-unknown-fuchsia_CMAKE_MODULE_LINKER_FLAGS ${FUCHSIA_${target}_LINKER_FLAGS} CACHE STRING "")
 set(BUILTINS_${target}-unknown-fuchsia_CMAKE_EXE_LINKER_FLAGS ${FUCHSIA_${target}_LINKER_FLAGS} CACHE STRING "")

[PATCH] D75591: [OpenMP] Add firstprivate as a default data-sharing attribute to clang

2020-03-14 Thread Atmn Patel via Phabricator via cfe-commits 
atmnpatel added inline comments.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:5102
   Diag(P.second->getExprLoc(), diag::err_omp_no_dsa_for_variable)
   << P.first << P.second->getSourceRange();
   Diag(DSAStack->getDefaultDSALocation(), diag::note_omp_default_dsa_none);

fghanim wrote:
> Why is `firstprivate` throwing this error? isn't the purpose of specifying it 
> as `default` is if a variable is not specified as anything, then it is 
> automatically handled as `firstprivate`? or am I misunderstanding something?
My understanding is that if that line isn't there then errors won't be thrown 
in the special cases where firstprivate explicitly requires a data sharing 
attribute to be specified - such as for static variable within a 
namespace/global scope as per the C/C++ restrictions in the technical report.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75591



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

[PATCH] D76184: [OpenMP][NFC] Remove the need to include `OpenMPClause.h`

2020-03-14 Thread Johannes Doerfert via Phabricator via cfe-commits 
jdoerfert updated this revision to Diff 250396.
jdoerfert added a comment.

Finish changes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76184

Files:
  clang/include/clang/AST/Attr.h
  clang/include/clang/AST/OpenMPClause.h
  clang/include/clang/Basic/OpenMPKinds.h
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/AttrImpl.cpp
  clang/lib/AST/OpenMPClause.cpp
  clang/lib/Parse/ParseOpenMP.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp

Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -6381,7 +6381,7 @@
   Record.push_back(C->getTotalComponentListNum());
   Record.push_back(C->getTotalComponentsNum());
   Record.AddSourceLocation(C->getLParenLoc());
-  for (unsigned I = 0; I < OMPMapClause::NumberOfModifiers; ++I) {
+  for (unsigned I = 0; I < NumberOfOMPMapClauseModifiers; ++I) {
 Record.push_back(C->getMapTypeModifier(I));
 Record.AddSourceLocation(C->getMapTypeModifierLoc(I));
   }
Index: clang/lib/Serialization/ASTReader.cpp
===
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -12292,7 +12292,7 @@
 
 void OMPClauseReader::VisitOMPMapClause(OMPMapClause *C) {
   C->setLParenLoc(Record.readSourceLocation());
-  for (unsigned I = 0; I < OMPMapClause::NumberOfModifiers; ++I) {
+  for (unsigned I = 0; I < NumberOfOMPMapClauseModifiers; ++I) {
 C->setMapTypeModifier(
 I, static_cast(Record.readInt()));
 C->setMapTypeModifierLoc(I, Record.readSourceLocation());
Index: clang/lib/Sema/SemaOpenMP.cpp
===
--- clang/lib/Sema/SemaOpenMP.cpp
+++ clang/lib/Sema/SemaOpenMP.cpp
@@ -16641,7 +16641,7 @@
   OpenMPMapModifierKind Modifiers[] = {OMPC_MAP_MODIFIER_unknown,
OMPC_MAP_MODIFIER_unknown,
OMPC_MAP_MODIFIER_unknown};
-  SourceLocation ModifiersLoc[OMPMapClause::NumberOfModifiers];
+  SourceLocation ModifiersLoc[NumberOfOMPMapClauseModifiers];
 
   // Process map-type-modifiers, flag errors for duplicate modifiers.
   unsigned Count = 0;
@@ -16651,7 +16651,7 @@
   Diag(MapTypeModifiersLoc[I], diag::err_omp_duplicate_map_type_modifier);
   continue;
 }
-assert(Count < OMPMapClause::NumberOfModifiers &&
+assert(Count < NumberOfOMPMapClauseModifiers &&
"Modifiers exceed the allowed number of map type modifiers");
 Modifiers[Count] = MapTypeModifiers[I];
 ModifiersLoc[Count] = MapTypeModifiersLoc[I];
Index: clang/lib/Parse/ParseOpenMP.cpp
===
--- clang/lib/Parse/ParseOpenMP.cpp
+++ clang/lib/Parse/ParseOpenMP.cpp
@@ -11,6 +11,7 @@
 //===--===//
 
 #include "clang/AST/ASTContext.h"
+#include "clang/AST/OpenMPClause.h"
 #include "clang/AST/StmtOpenMP.h"
 #include "clang/Basic/OpenMPKinds.h"
 #include "clang/Parse/ParseDiagnostic.h"
@@ -862,7 +863,7 @@
 } // namespace
 
 void Parser::parseOMPTraitPropertyKind(
-OMPTraitInfo::OMPTraitProperty , llvm::omp::TraitSet Set,
+OMPTraitProperty , llvm::omp::TraitSet Set,
 llvm::omp::TraitSelector Selector, llvm::StringMap ) {
   TIProperty.Kind = TraitProperty::invalid;
 
@@ -931,14 +932,14 @@
   << CONTEXT_TRAIT_LVL << listOpenMPContextTraitProperties(Set, Selector);
 }
 
-void Parser::parseOMPContextProperty(OMPTraitInfo::OMPTraitSelector ,
+void Parser::parseOMPContextProperty(OMPTraitSelector ,
  llvm::omp::TraitSet Set,
  llvm::StringMap ) {
   assert(TISelector.Kind != TraitSelector::user_condition &&
  "User conditions are special properties not handled here!");
 
   SourceLocation PropertyLoc = Tok.getLocation();
-  OMPTraitInfo::OMPTraitProperty TIProperty;
+  OMPTraitProperty TIProperty;
   parseOMPTraitPropertyKind(TIProperty, Set, TISelector.Kind, Seen);
 
   // If we have an invalid property here we already issued a warning.
@@ -972,7 +973,7 @@
 }
 
 void Parser::parseOMPTraitSelectorKind(
-OMPTraitInfo::OMPTraitSelector , llvm::omp::TraitSet Set,
+OMPTraitSelector , llvm::omp::TraitSet Set,
 llvm::StringMap ) {
   TISelector.Kind = TraitSelector::invalid;
 
@@ -1051,7 +1052,7 @@
 ///
 ///  ['('[]  [, ]* ')']
 void Parser::parseOMPContextSelector(
-OMPTraitInfo::OMPTraitSelector , llvm::omp::TraitSet Set,
+OMPTraitSelector , llvm::omp::TraitSet Set,
 llvm::StringMap ) {
   unsigned short OuterPC = ParenCount;
 
@@ -1151,7 +1152,7 @@
   BDT.consumeClose();
 }
 
-void

[PATCH] D74692: [clang-tidy][bugprone-use-after-move] Warn on std::move for consts

2020-03-14 Thread Arthur O'Dwyer via Phabricator via cfe-commits 
Quuxplusone added a comment.

I still think this entire patch is misguided; there's no reason to make the 
note for `const std::string s; std::move(s)` any longer than the note for `int 
i; std::move(i)` or `volatile std::string v; std::move(v)`. People should not 
be using moved-from objects, period; and those who want to use moved-from 
objects, should not enable this clang-tidy check.

However, I have no further comments //besides// philosophical opposition to the 
whole idea.


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

https://reviews.llvm.org/D74692



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

[PATCH] D76181: [AVR] Add support for the -mdouble=x flag

2020-03-14 Thread Fangrui Song via Phabricator via cfe-commits 
MaskRay added a comment.

The GCC side commits can be found on 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92055
So it seems that we will have both `-mlong-double-{64,80,128}` (80 is used by 
x86 fp80) and `-mlong-double={32,64}`... (I actually prefer `=` to `-`)




Comment at: clang/lib/Driver/ToolChains/Clang.cpp:4594
 
+  if (Arg *A = Args.getLastArg(options::OPT_mdouble_EQ)) {
+if (TC.getArch() == llvm::Triple::avr)

Nit: move this before the `-mlong-double-` processing.



Comment at: clang/test/CodeGen/mdouble.c:1
+// RUN: %clang_cc1 %s -emit-llvm -o - -triple=avr-unknown-unknown -mdouble=64 
| \
+// RUN:   FileCheck --check-prefix=AVR-FP64 %s

Maybe name this file `avr-mdouble.c`



Comment at: clang/test/Driver/mdouble.c:1
+// RUN: %clang -target avr-unknown-unknown -c -### %s -mdouble=64 2>&1 | 
FileCheck %s
+

`-unknown-unknown` can be deleted


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76181



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

[PATCH] D76186: Fix compatibility for __builtin_stdarg_start

2020-03-14 Thread Joerg Sonnenberger via Phabricator via cfe-commits 
joerg created this revision.

The `__builtin_stdarg_start` is the legacy spelling of `__builtin_va_start`. It 
should behave exactly the same, but for the last 9 years it would behave subtly 
different for diagnostics. Follow the change from 29ad95b23217 to require 
custom type checking.


https://reviews.llvm.org/D76186

Files:
  clang/include/clang/Basic/Builtins.def
  clang/test/SemaCXX/vararg-non-pod.cpp


Index: clang/test/SemaCXX/vararg-non-pod.cpp
===
--- clang/test/SemaCXX/vararg-non-pod.cpp
+++ clang/test/SemaCXX/vararg-non-pod.cpp
@@ -164,6 +164,13 @@
   __builtin_va_start(list, somearg);
 }
 
+// __builtin_stdarg_start is a compatibility alias for __builtin_va_start,
+// it should behave the same
+void t6b(Foo somearg, ... ) {
+  __builtin_va_list list;
+  __builtin_stdarg_start(list, somearg); // second argument to 'va_start' is 
not the last named parameter [-Wvarargs]
+}
+
 void t7(int n, ...) {
   __builtin_va_list list;
   __builtin_va_start(list, n);
Index: clang/include/clang/Basic/Builtins.def
===
--- clang/include/clang/Basic/Builtins.def
+++ clang/include/clang/Basic/Builtins.def
@@ -471,7 +471,7 @@
 BUILTIN(__builtin_va_start, "vA.", "nt")
 BUILTIN(__builtin_va_end, "vA", "n")
 BUILTIN(__builtin_va_copy, "vAA", "n")
-BUILTIN(__builtin_stdarg_start, "vA.", "n")
+BUILTIN(__builtin_stdarg_start, "vA.", "nt")
 BUILTIN(__builtin_assume_aligned, "v*vC*z.", "nc")
 BUILTIN(__builtin_bcmp, "ivC*vC*z", "Fn")
 BUILTIN(__builtin_bcopy, "vv*v*z", "n")


Index: clang/test/SemaCXX/vararg-non-pod.cpp
===
--- clang/test/SemaCXX/vararg-non-pod.cpp
+++ clang/test/SemaCXX/vararg-non-pod.cpp
@@ -164,6 +164,13 @@
   __builtin_va_start(list, somearg);
 }
 
+// __builtin_stdarg_start is a compatibility alias for __builtin_va_start,
+// it should behave the same
+void t6b(Foo somearg, ... ) {
+  __builtin_va_list list;
+  __builtin_stdarg_start(list, somearg); // second argument to 'va_start' is not the last named parameter [-Wvarargs]
+}
+
 void t7(int n, ...) {
   __builtin_va_list list;
   __builtin_va_start(list, n);
Index: clang/include/clang/Basic/Builtins.def
===
--- clang/include/clang/Basic/Builtins.def
+++ clang/include/clang/Basic/Builtins.def
@@ -471,7 +471,7 @@
 BUILTIN(__builtin_va_start, "vA.", "nt")
 BUILTIN(__builtin_va_end, "vA", "n")
 BUILTIN(__builtin_va_copy, "vAA", "n")
-BUILTIN(__builtin_stdarg_start, "vA.", "n")
+BUILTIN(__builtin_stdarg_start, "vA.", "nt")
 BUILTIN(__builtin_assume_aligned, "v*vC*z.", "nc")
 BUILTIN(__builtin_bcmp, "ivC*vC*z", "Fn")
 BUILTIN(__builtin_bcopy, "vv*v*z", "n")
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D74918: Add method to TargetInfo to get CPU cache line size

2020-03-14 Thread Zoe Carver via Phabricator via cfe-commits 
zoecarver added a comment.
Herald added a subscriber: danielkiss.

There are a lot of different ways we could implement the feature. We may want 
to only enable it when `-march=native`, or maybe only in the unstable ABI, and 
maybe we want to support aligned pairs on some architectures. I think that's an 
important discussion to have but I'm not sure _this_ patch is the best place to 
have that discussion.

Even if we don't use this patch in the implementation I think it would still be 
a good utility to have. Here's what I suggest: I commit this, create another 
patch to add a builtin that exposes this API, and then open a libc++ patch with 
a _possible_ implementation. In that patch, we can discuss how we should 
actually implement the feature and after we have a consensus I can do the work 
to implement it. Any objections to that plan?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74918



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

[clang] 19840a3 - Remove an unnecessary explicit 'WarnDiag'; NFC

2020-03-14 Thread Aaron Ballman via cfe-commits 

Author: Aaron Ballman
Date: 2020-03-14T17:15:59-04:00
New Revision: 19840a307e6d98017b68b14725c53d1ad89d98f9

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

LOG: Remove an unnecessary explicit 'WarnDiag'; NFC

Added: 


Modified: 
clang/include/clang/Basic/Attr.td

Removed: 




diff  --git a/clang/include/clang/Basic/Attr.td 
b/clang/include/clang/Basic/Attr.td
index c2504ba620e8..624995a2d572 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -677,7 +677,7 @@ def AlwaysInline : InheritableAttr {
 
 def Artificial : InheritableAttr {
   let Spellings = [GCC<"artificial">];
-  let Subjects = SubjectList<[InlineFunction], WarnDiag>;
+  let Subjects = SubjectList<[InlineFunction]>;
   let Documentation = [ArtificialDocs];
 }
 



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

[clang] dab43c8 - Remove some explicit calls to getName() when printing diagnostics; NFC

2020-03-14 Thread Aaron Ballman via cfe-commits 

Author: Aaron Ballman
Date: 2020-03-14T17:01:45-04:00
New Revision: dab43c85920cd80b919265936e319f9583c8b4e8

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

LOG: Remove some explicit calls to getName() when printing diagnostics; NFC

Added: 


Modified: 
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaDeclAttr.cpp
clang/test/Sema/no-builtin.cpp
clang/test/SemaCXX/member-pointer-ms.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 4328e322b914..cc815a4993d5 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3461,7 +3461,7 @@ def warn_use_of_temp_in_invalid_state : Warning<
   "invalid invocation of method '%0' on a temporary object while it is in the "
   "'%1' state">, InGroup, DefaultIgnore;
 def warn_attr_on_unconsumable_class : Warning<
-  "consumed analysis attribute is attached to member of class '%0' which isn't 
"
+  "consumed analysis attribute is attached to member of class %0 which isn't "
   "marked as consumable">, InGroup, DefaultIgnore;
 def warn_return_typestate_for_unconsumable_type : Warning<
   "return state set for an unconsumable type '%0'">, InGroup,

diff  --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index 3385717e3138..476c9e635b45 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -1099,7 +1099,7 @@ static void handleNoBuiltinAttr(Sema , Decl *D, const 
ParsedAttr ) {
 AddBuiltinName(BuiltinName);
   else
 S.Diag(LiteralLoc, 
diag::warn_attribute_no_builtin_invalid_builtin_name)
-<< BuiltinName << AL.getAttrName()->getName();
+<< BuiltinName << AL;
 }
 
   // Repeating the same attribute is fine.
@@ -1110,7 +1110,7 @@ static void handleNoBuiltinAttr(Sema , Decl *D, const 
ParsedAttr ) {
   if (HasWildcard && Names.size() > 1)
 S.Diag(D->getLocation(),
diag::err_attribute_no_builtin_wildcard_or_builtin_name)
-<< AL.getAttrName()->getName();
+<< AL;
 
   if (D->hasAttr())
 D->dropAttr();
@@ -1176,8 +1176,7 @@ static bool checkForConsumableClass(Sema , const 
CXXMethodDecl *MD,
 
   if (const CXXRecordDecl *RD = ThisType->getAsCXXRecordDecl()) {
 if (!RD->hasAttr()) {
-  S.Diag(AL.getLoc(), diag::warn_attr_on_unconsumable_class) <<
-RD->getNameAsString();
+  S.Diag(AL.getLoc(), diag::warn_attr_on_unconsumable_class) << RD;
 
   return false;
 }
@@ -3676,7 +3675,7 @@ void Sema::AddAlignValueAttr(Decl *D, const 
AttributeCommonInfo , Expr *E) {
   if (!T->isDependentType() && !T->isAnyPointerType() &&
   !T->isReferenceType() && !T->isMemberPointerType()) {
 Diag(AttrLoc, diag::warn_attribute_pointer_or_reference_only)
-  <<  /*TmpAttr.getName()*/ << T << D->getSourceRange();
+  <<  << T << D->getSourceRange();
 return;
   }
 
@@ -3915,8 +3914,7 @@ bool Sema::checkMSInheritanceAttrOnDefinition(
 
   Diag(Range.getBegin(), diag::err_mismatched_ms_inheritance)
   << 0 /*definition*/;
-  Diag(RD->getDefinition()->getLocation(), diag::note_defined_here)
-  << RD->getNameAsString();
+  Diag(RD->getDefinition()->getLocation(), diag::note_defined_here) << RD;
   return true;
 }
 

diff  --git a/clang/test/Sema/no-builtin.cpp b/clang/test/Sema/no-builtin.cpp
index 8908f38333bd..392d847f98aa 100644
--- a/clang/test/Sema/no-builtin.cpp
+++ b/clang/test/Sema/no-builtin.cpp
@@ -17,11 +17,11 @@ void many_attribute_function_4() 
__attribute__((no_builtin("memcpy", "memcpy")))
 
 /// Invalid builtin name.
 void invalid_builtin() __attribute__((no_builtin("not_a_builtin"))) {}
-// expected-warning@-1 {{'not_a_builtin' is not a valid builtin name for 
no_builtin}}
+// expected-warning@-1 {{'not_a_builtin' is not a valid builtin name for 
'no_builtin'}}
 
 /// Can't use bare no_builtin with a named one.
 void wildcard_and_functionname() __attribute__((no_builtin)) 
__attribute__((no_builtin("memcpy"))) {}
-// expected-error@-1 {{empty no_builtin cannot be composed with named ones}}
+// expected-error@-1 {{empty 'no_builtin' cannot be composed with named ones}}
 
 /// Can't attach attribute to a variable.
 int __attribute__((no_builtin)) variable;

diff  --git a/clang/test/SemaCXX/member-pointer-ms.cpp 
b/clang/test/SemaCXX/member-pointer-ms.cpp
index c8059acd6737..3271ff0c623a 100644
--- a/clang/test/SemaCXX/member-pointer-ms.cpp
+++ b/clang/test/SemaCXX/member-pointer-ms.cpp
@@ -239,11 +239,11 @@ template  struct __multiple_inheritance A;
   // expected-warning@-1 {{inheritance model ignored on partial 
specialization}}
 template <> struct __single_inheritance A;
 
-struct B {}; //

[clang] eda58ac - Improve the attribute language option interface somewhat; NFCi.

2020-03-14 Thread Aaron Ballman via cfe-commits 

Author: Aaron Ballman
Date: 2020-03-14T15:59:14-04:00
New Revision: eda58ac04cfa95298583223ba6779916e4721550

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

LOG: Improve the attribute language option interface somewhat; NFCi.

The name field is optional if the custom code is supplied, so this updates the
documentation for LangOpt and introduces a tablegen warning if both custom code
and a language option name are supplied.

Added: 


Modified: 
clang/include/clang/Basic/Attr.td
clang/utils/TableGen/ClangAttrEmitter.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/Attr.td 
b/clang/include/clang/Basic/Attr.td
index 484691f419c3..c2504ba620e8 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -312,6 +312,7 @@ class SubjectList subjects, SubjectDiag 
diag = WarnDiag,
 }
 
 class LangOpt {
+  // The language option to test; ignored when custom code is supplied.
   string Name = name;
 
   // A custom predicate, written as an expression evaluated in a context with
@@ -323,15 +324,15 @@ def Borland : LangOpt<"Borland">;
 def CUDA : LangOpt<"CUDA">;
 def HIP : LangOpt<"HIP">;
 def SYCL : LangOpt<"SYCLIsDevice">;
-def COnly : LangOpt<"COnly", "!LangOpts.CPlusPlus">;
+def COnly : LangOpt<"", "!LangOpts.CPlusPlus">;
 def CPlusPlus : LangOpt<"CPlusPlus">;
 def OpenCL : LangOpt<"OpenCL">;
 def RenderScript : LangOpt<"RenderScript">;
 def ObjC : LangOpt<"ObjC">;
 def BlocksSupported : LangOpt<"Blocks">;
 def ObjCAutoRefCount : LangOpt<"ObjCAutoRefCount">;
-def ObjCNonFragileRuntime : LangOpt<"ObjCNonFragileRuntime",
-"LangOpts.ObjCRuntime.allowsClassStubs()">;
+def ObjCNonFragileRuntime
+: LangOpt<"", "LangOpts.ObjCRuntime.allowsClassStubs()">;
 
 // Language option for CMSE extensions
 def Cmse : LangOpt<"Cmse">;

diff  --git a/clang/utils/TableGen/ClangAttrEmitter.cpp 
b/clang/utils/TableGen/ClangAttrEmitter.cpp
index 20fb9bc14439..529145efa3d0 100644
--- a/clang/utils/TableGen/ClangAttrEmitter.cpp
+++ b/clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -1980,6 +1980,11 @@ static std::string 
GenerateTestExpression(ArrayRef LangOpts) {
   Test += "(";
   Test += Code;
   Test += ")";
+  if (!E->getValueAsString("Name").empty()) {
+PrintWarning(
+E->getLoc(),
+"non-empty 'Name' field ignored because 'CustomCode' was 
supplied");
+  }
 } else {
   Test += "LangOpts.";
   Test += E->getValueAsString("Name");



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

[PATCH] D75749: [clang-tidy] extend bugprone-signed-char-misuse check.

2020-03-14 Thread Tamás Zolnai via Phabricator via cfe-commits 
ztamas added a comment.

In D75749#1912338 , @njames93 wrote:

> LGTM
>
> However, how does this handle cases when the type is written as `char`. They 
> can be signed/unsigned based on what platform is being targeted. But on a 
> platform where `char` is signed, comparison to an `unsigned char` is 
> dangerous and the complimented case is the same. Surely any `char` comparison 
> to `signed char` or `unsigned char` should be warned. But I'm not sure if 
> that's in the scope of this check.


It uses the default signedness what clang specifies for `char` (which depends 
on the platform). For example, on my Linux x64 system, `char` is equivalent to 
`signed char` and so this clang-tidy check catches also the `char` <-> 
`unsigned char` comparisons. This default behavior can be overridden with 
`-funsigned-char` and `-fsigned-char` compilation options. So for a 
cross-platform C++ code, it's an option to run the clang-tidy twice, using 
these two options and so both `char` <-> `unsigned char` and `char` <-> `signed 
char` comparisons can be caught.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75749



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

[PATCH] D75749: [clang-tidy] extend bugprone-signed-char-misuse check.

2020-03-14 Thread Tamás Zolnai via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rG04410c565aa0: [clang-tidy] extend 
bugprone-signed-char-misuse check. (authored by ztamas).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75749

Files:
  clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.h
  clang-tools-extra/docs/clang-tidy/checks/bugprone-signed-char-misuse.rst
  clang-tools-extra/test/clang-tidy/checkers/bugprone-signed-char-misuse.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-signed-char-misuse.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-signed-char-misuse.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-signed-char-misuse.cpp
@@ -62,6 +62,34 @@
   return NCharacter;
 }
 
+int SignedUnsignedCharEquality(signed char SCharacter) {
+  unsigned char USCharacter = 'a';
+  if (SCharacter == USCharacter) // CHECK-MESSAGES: [[@LINE]]:7: warning: comparison between 'signed char' and 'unsigned char' [bugprone-signed-char-misuse]
+return 1;
+  return 0;
+}
+
+int SignedUnsignedCharIneqiality(signed char SCharacter) {
+  unsigned char USCharacter = 'a';
+  if (SCharacter != USCharacter) // CHECK-MESSAGES: [[@LINE]]:7: warning: comparison between 'signed char' and 'unsigned char' [bugprone-signed-char-misuse]
+return 1;
+  return 0;
+}
+
+int CompareWithNonAsciiConstant(unsigned char USCharacter) {
+  const signed char SCharacter = -5;
+  if (USCharacter == SCharacter) // CHECK-MESSAGES: [[@LINE]]:7: warning: comparison between 'signed char' and 'unsigned char' [bugprone-signed-char-misuse]
+return 1;
+  return 0;
+}
+
+int CompareWithUnsignedNonAsciiConstant(signed char SCharacter) {
+  const unsigned char USCharacter = 128;
+  if (USCharacter == SCharacter) // CHECK-MESSAGES: [[@LINE]]:7: warning: comparison between 'signed char' and 'unsigned char' [bugprone-signed-char-misuse]
+return 1;
+  return 0;
+}
+
 ///
 /// Test cases correctly ignored by the check.
 
@@ -121,3 +149,61 @@
 
   return USCharacter;
 }
+
+int FixComparisonWithSignedCharCast(signed char SCharacter) {
+  unsigned char USCharacter = 'a';
+  if (SCharacter == static_cast(USCharacter))
+return 1;
+  return 0;
+}
+
+int FixComparisonWithUnSignedCharCast(signed char SCharacter) {
+  unsigned char USCharacter = 'a';
+  if (static_cast(SCharacter) == USCharacter)
+return 1;
+  return 0;
+}
+
+// Make sure we don't catch other type of char comparison.
+int SameCharTypeComparison(signed char SCharacter) {
+  signed char SCharacter2 = 'a';
+  if (SCharacter == SCharacter2)
+return 1;
+  return 0;
+}
+
+// Make sure we don't catch other type of char comparison.
+int SameCharTypeComparison2(unsigned char USCharacter) {
+  unsigned char USCharacter2 = 'a';
+  if (USCharacter == USCharacter2)
+return 1;
+  return 0;
+}
+
+// Make sure we don't catch integer - char comparison.
+int CharIntComparison(signed char SCharacter) {
+  int ICharacter = 10;
+  if (SCharacter == ICharacter)
+return 1;
+  return 0;
+}
+
+int CompareWithAsciiLiteral(unsigned char USCharacter) {
+  if (USCharacter == 'x') // no warning
+return 1;
+  return 0;
+}
+
+int CompareWithAsciiConstant(unsigned char USCharacter) {
+  const signed char SCharacter = 'a';
+  if (USCharacter == SCharacter)
+return 1;
+  return 0;
+}
+
+int CompareWithUnsignedAsciiConstant(signed char SCharacter) {
+  const unsigned char USCharacter = 'a';
+  if (USCharacter == SCharacter)
+return 1;
+  return 0;
+}
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone-signed-char-misuse.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/bugprone-signed-char-misuse.rst
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone-signed-char-misuse.rst
@@ -3,27 +3,39 @@
 bugprone-signed-char-misuse
 ===
 
-Finds ``signed char`` -> integer conversions which might indicate a programming
-error. The basic problem with the ``signed char``, that it might store the
-non-ASCII characters as negative values. The human programmer probably
-expects that after an integer conversion the converted value matches with the
+Finds those ``signed char`` -> integer conversions which might indicate a
+programming error. The basic problem with the ``signed char``, that it might
+store the non-ASCII characters as negative values. This behavior can cause a
+misunderstanding of the written code both when an explicit and when an
+implicit conversion happens.
+
+When the code contains an explicit ``signed char`` -> integer conversion, the
+human programmer probably expects that the converted value matches with the
 character code (a value from [0..255]), however, the

[PATCH] D75210: [Attr] Allow ParsedAttr to be constructor for any Attribute

2020-03-14 Thread Johannes Doerfert via Phabricator via cfe-commits 
jdoerfert abandoned this revision.
jdoerfert added a comment.

Code is good and usable but no users anymore. If anyone wants to pick this up, 
feel free!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75210



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

[PATCH] D76184: [OpenMP][NFC] Remove the need to include `OpenMPClause.h`

2020-03-14 Thread Johannes Doerfert via Phabricator via cfe-commits 
jdoerfert created this revision.
jdoerfert added a reviewer: rnk.
Herald added subscribers: guansong, bollu.
Herald added a project: clang.

See rational here: https://reviews.llvm.org/D76173#1922916


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D76184

Files:
  clang/include/clang/AST/Attr.h
  clang/include/clang/AST/OpenMPClause.h
  clang/include/clang/Parse/Parser.h
  clang/lib/AST/OpenMPClause.cpp
  clang/lib/Parse/ParseOpenMP.cpp

Index: clang/lib/Parse/ParseOpenMP.cpp
===
--- clang/lib/Parse/ParseOpenMP.cpp
+++ clang/lib/Parse/ParseOpenMP.cpp
@@ -11,6 +11,7 @@
 //===--===//
 
 #include "clang/AST/ASTContext.h"
+#include "clang/AST/OpenMPClause.h"
 #include "clang/AST/StmtOpenMP.h"
 #include "clang/Basic/OpenMPKinds.h"
 #include "clang/Parse/ParseDiagnostic.h"
@@ -862,7 +863,7 @@
 } // namespace
 
 void Parser::parseOMPTraitPropertyKind(
-OMPTraitInfo::OMPTraitProperty , llvm::omp::TraitSet Set,
+OMPTraitProperty , llvm::omp::TraitSet Set,
 llvm::omp::TraitSelector Selector, llvm::StringMap ) {
   TIProperty.Kind = TraitProperty::invalid;
 
@@ -931,14 +932,14 @@
   << CONTEXT_TRAIT_LVL << listOpenMPContextTraitProperties(Set, Selector);
 }
 
-void Parser::parseOMPContextProperty(OMPTraitInfo::OMPTraitSelector ,
+void Parser::parseOMPContextProperty(OMPTraitSelector ,
  llvm::omp::TraitSet Set,
  llvm::StringMap ) {
   assert(TISelector.Kind != TraitSelector::user_condition &&
  "User conditions are special properties not handled here!");
 
   SourceLocation PropertyLoc = Tok.getLocation();
-  OMPTraitInfo::OMPTraitProperty TIProperty;
+  OMPTraitProperty TIProperty;
   parseOMPTraitPropertyKind(TIProperty, Set, TISelector.Kind, Seen);
 
   // If we have an invalid property here we already issued a warning.
@@ -972,7 +973,7 @@
 }
 
 void Parser::parseOMPTraitSelectorKind(
-OMPTraitInfo::OMPTraitSelector , llvm::omp::TraitSet Set,
+OMPTraitSelector , llvm::omp::TraitSet Set,
 llvm::StringMap ) {
   TISelector.Kind = TraitSelector::invalid;
 
@@ -1051,7 +1052,7 @@
 ///
 ///  ['('[]  [, ]* ')']
 void Parser::parseOMPContextSelector(
-OMPTraitInfo::OMPTraitSelector , llvm::omp::TraitSet Set,
+OMPTraitSelector , llvm::omp::TraitSet Set,
 llvm::StringMap ) {
   unsigned short OuterPC = ParenCount;
 
@@ -1151,7 +1152,7 @@
   BDT.consumeClose();
 }
 
-void Parser::parseOMPTraitSetKind(OMPTraitInfo::OMPTraitSet ,
+void Parser::parseOMPTraitSetKind(OMPTraitSet ,
   llvm::StringMap ) {
   TISet.Kind = TraitSet::invalid;
 
@@ -1215,7 +1216,7 @@
 ///
 ///  '=' '{'  [, ]* '}'
 void Parser::parseOMPContextSelectorSet(
-OMPTraitInfo::OMPTraitSet ,
+OMPTraitSet ,
 llvm::StringMap ) {
   auto OuterBC = BraceCount;
 
@@ -1270,7 +1271,7 @@
 
   llvm::StringMap SeenSelectors;
   do {
-OMPTraitInfo::OMPTraitSelector TISelector;
+OMPTraitSelector TISelector;
 parseOMPContextSelector(TISelector, TISet.Kind, SeenSelectors);
 if (TISelector.Kind != TraitSelector::invalid &&
 !TISelector.Properties.empty())
@@ -1292,10 +1293,10 @@
 /// Parse OpenMP context selectors:
 ///
 ///  [, ]*
-bool Parser::parseOMPContextSelectors(SourceLocation Loc, OMPTraitInfo ) {
+bool Parser::parseOMPContextSelectors(SourceLocation Loc, OMPTraitInfo& TI) {
   llvm::StringMap SeenSets;
   do {
-OMPTraitInfo::OMPTraitSet TISet;
+OMPTraitSet TISet;
 parseOMPContextSelectorSet(TISet, SeenSets);
 if (TISet.Kind != TraitSet::invalid && !TISet.Selectors.empty())
   TI.Sets.push_back(TISet);
Index: clang/lib/AST/OpenMPClause.cpp
===
--- clang/lib/AST/OpenMPClause.cpp
+++ clang/lib/AST/OpenMPClause.cpp
@@ -1841,14 +1841,14 @@
 void OMPTraitInfo::print(llvm::raw_ostream ,
  const PrintingPolicy ) const {
   bool FirstSet = true;
-  for (const OMPTraitInfo::OMPTraitSet  : Sets) {
+  for (const OMPTraitSet  : Sets) {
 if (!FirstSet)
   OS << ", ";
 FirstSet = false;
 OS << llvm::omp::getOpenMPContextTraitSetName(Set.Kind) << "={";
 
 bool FirstSelector = true;
-for (const OMPTraitInfo::OMPTraitSelector  : Set.Selectors) {
+for (const OMPTraitSelector  : Set.Selectors) {
   if (!FirstSelector)
 OS << ", ";
   FirstSelector = false;
@@ -1874,7 +1874,7 @@
 }
 
 bool FirstProperty = true;
-for (const OMPTraitInfo::OMPTraitProperty  :
+for (const OMPTraitProperty  :
  Selector.Properties) {
   if (!FirstProperty)
 OS << ", ";
Index: clang/include/clang/Parse/Parser.h
===
--- clang/include/clang/Parse/Parser.h
+++ clang/include/clang/Parse/Parser.h
@@ -13,7

[PATCH] D74692: [clang-tidy][bugprone-use-after-move] Warn on std::move for consts

2020-03-14 Thread Zinovy Nis via Phabricator via cfe-commits 
zinovy.nis updated this revision to Diff 250375.
zinovy.nis added a comment.

Removed top-level consts.


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

https://reviews.llvm.org/D74692

Files:
  clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp
@@ -129,6 +129,16 @@
   // CHECK-NOTES: [[@LINE-3]]:15: note: move occurred here
 }
 
+void simpleConst() {
+  const A a;
+  a.foo();
+  A other_a = std::move(a);
+  a.foo();
+  // CHECK-NOTES: [[@LINE-1]]:3: warning: 'a' used after it was moved; move of a 'const' argument has no effect
+  // CHECK-NOTES: [[@LINE-3]]:15: note: move occurred here
+  // CHECK-NOTES: [[@LINE-6]]:11: note: variable 'a' declared const here
+}
+
 // A warning should only be emitted for one use-after-move.
 void onlyFlagOneUseAfterMove() {
   A a;
@@ -314,8 +324,21 @@
 auto lambda = [a] {
   std::move(a);
   a.foo();
-  // CHECK-NOTES: [[@LINE-1]]:7: warning: 'a' used after it was moved
+  // CHECK-NOTES: [[@LINE-1]]:7: warning: 'a' used after it was moved; move of a 'const' argument has no effect
   // CHECK-NOTES: [[@LINE-3]]:7: note: move occurred here
+  // CHECK-NOTES: [[@LINE-5]]:20: note: variable 'a' captured const here
+};
+  }
+  // Use-after-moves inside a lambda should be detected.
+  {
+A a;
+// Implicit capture
+auto lambda = [=] {
+  std::move(a);
+  a.foo();
+  // CHECK-NOTES: [[@LINE-1]]:7: warning: 'a' used after it was moved; move of a 'const' argument has no effect
+  // CHECK-NOTES: [[@LINE-3]]:7: note: move occurred here
+  // CHECK-NOTES: [[@LINE-5]]:20: note: variable 'a' implicitly captured const here
 };
   }
   // This is just as true if the variable was declared inside the lambda.
@@ -721,14 +744,16 @@
 const A a;
 std::move(a);
 passByConstPointer();
-// CHECK-NOTES: [[@LINE-1]]:25: warning: 'a' used after it was moved
+// CHECK-NOTES: [[@LINE-1]]:25: warning: 'a' used after it was moved; move of a 'const' argument has no effect
 // CHECK-NOTES: [[@LINE-3]]:5: note: move occurred here
+// CHECK-NOTES: [[@LINE-5]]:13: note: variable 'a' declared const here
   }
   const A a;
   std::move(a);
   passByConstReference(a);
-  // CHECK-NOTES: [[@LINE-1]]:24: warning: 'a' used after it was moved
+  // CHECK-NOTES: [[@LINE-1]]:24: warning: 'a' used after it was moved; move of a 'const' argument has no effect
   // CHECK-NOTES: [[@LINE-3]]:3: note: move occurred here
+  // CHECK-NOTES: [[@LINE-5]]:11: note: variable 'a' declared const here
 }
 
 // Clearing a standard container using clear() is treated as a
Index: clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
@@ -373,14 +373,36 @@
 }
 
 static void emitDiagnostic(const Expr *MovingCall, const DeclRefExpr *MoveArg,
-   const UseAfterMove , ClangTidyCheck *Check,
+   const UseAfterMove ,
+   const LambdaExpr *Lambda, ClangTidyCheck *Check,
ASTContext *Context) {
   SourceLocation UseLoc = Use.DeclRef->getExprLoc();
   SourceLocation MoveLoc = MovingCall->getExprLoc();
+  int MoveArgIsConst = MoveArg->getType().isConstQualified();
 
-  Check->diag(UseLoc, "'%0' used after it was moved")
-  << MoveArg->getDecl()->getName();
+  Check->diag(UseLoc, "'%0' used after it was moved%select{|; move of a "
+  "'const' argument has no effect}1")
+  << MoveArg->getDecl()->getName() << MoveArgIsConst;
   Check->diag(MoveLoc, "move occurred here", DiagnosticIDs::Note);
+  if (MoveArgIsConst) {
+if (Lambda) {
+  for (const auto  : Lambda->captures()) {
+if (MoveArg->getDecl() == Capture.getCapturedVar()) {
+  int IsExplicitCapture = Capture.isExplicit();
+  Check->diag(IsExplicitCapture ? Capture.getLocation()
+: Lambda->getCaptureDefaultLoc(),
+  "variable %0 %select{implicitly|}1 captured const here",
+  DiagnosticIDs::Note)
+  << Capture.getCapturedVar() << IsExplicitCapture;
+  break;
+}
+  }
+} else {
+  Check->diag(MoveArg->getDecl()->getLocation(),
+  "variable '%0' declared const here", DiagnosticIDs::Note);
+}
+  }
+
   if (Use.EvaluationOrderUndefined) {
 Check->diag(UseLoc,
 "the use and move are unsequenced, i.e. there

[PATCH] D76173: [OpenMP][NFC] Minimize memory usage and copying of `OMPTraitInfo`s

2020-03-14 Thread Johannes Doerfert via Phabricator via cfe-commits 
jdoerfert added a comment.

In D76173#1922916 , @rnk wrote:

> lgtm, thanks!
>
> I also noticed that Parser.h includes OpenMPClause.h just for this class. 
> OpenMPClause.h is pretty big. It's for this family of related methods:
>
>   /// Parse a property kind into \p TIProperty for the selector set \p Set and
>   /// selector \p Selector.
>   void parseOMPTraitPropertyKind(OMPTraitInfo::OMPTraitProperty ,
>  llvm::omp::TraitSet Set,
>  llvm::omp::TraitSelector Selector,
>  llvm::StringMap );
>   
>
> The use of an inner class here makes it impossible to forward declare 
> OMPTraitProperty. Would you be opposed to moving it out of line?


Like D76184  ?

> The other popular includer of OpenMPClause.h is Attr.h, which is where I 
> started my investigation. I think after your change, we can forward declare 
> it. I'll look into it.

Took care of that in D76184  as well. I will 
"soonish" redesign OpenMPClause.h completely but that will take more time.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76173



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

[clang-tools-extra] 04410c5 - [clang-tidy] extend bugprone-signed-char-misuse check.

2020-03-14 Thread Tamás Zolnai via cfe-commits 

Author: Tamás Zolnai
Date: 2020-03-14T20:00:00+01:00
New Revision: 04410c565aa08b703ef5d11b454e7fba47163e3c

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

LOG: [clang-tidy] extend bugprone-signed-char-misuse check.

Summary:
Cover a new use case when using a 'signed char' as an integer
might lead to issue with non-ASCII characters. Comparing
a 'signed char' with an 'unsigned char' using equality / unequality
operator produces an unexpected result for non-ASCII characters.

Reviewers: aaron.ballman, alexfh, hokein, njames93

Reviewed By: njames93

Subscribers: xazax.hun, cfe-commits

Tags: #clang, #clang-tools-extra

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

Added: 


Modified: 
clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.cpp
clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.h
clang-tools-extra/docs/clang-tidy/checks/bugprone-signed-char-misuse.rst
clang-tools-extra/test/clang-tidy/checkers/bugprone-signed-char-misuse.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.cpp 
b/clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.cpp
index 32949a878497..732ccbc9dd2a 100644
--- a/clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.cpp
@@ -18,6 +18,8 @@ namespace clang {
 namespace tidy {
 namespace bugprone {
 
+static constexpr int UnsignedASCIIUpperBound = 127;
+
 static Matcher hasAnyListedName(const std::string ) {
   const std::vector NameList =
   utils::options::parseStringList(Names);
@@ -33,25 +35,29 @@ void 
SignedCharMisuseCheck::storeOptions(ClangTidyOptions::OptionMap ) {
   Options.store(Opts, "CharTypdefsToIgnore", CharTypdefsToIgnoreList);
 }
 
-void SignedCharMisuseCheck::registerMatchers(MatchFinder *Finder) {
+// Create a matcher for char -> integer cast.
+BindableMatcher SignedCharMisuseCheck::charCastExpression(
+bool IsSigned, const Matcher ,
+const std::string ) const {
   // We can ignore typedefs which are some kind of integer types
   // (e.g. typedef char sal_Int8). In this case, we don't need to
   // worry about the misinterpretation of char values.
   const auto IntTypedef = qualType(
   hasDeclaration(typedefDecl(hasAnyListedName(CharTypdefsToIgnoreList;
 
-  const auto SignedCharType = expr(hasType(qualType(
-  allOf(isAnyCharacter(), isSignedInteger(), unless(IntTypedef);
-
-  const auto IntegerType = qualType(allOf(isInteger(), 
unless(isAnyCharacter()),
-  unless(booleanType(
-   .bind("integerType");
+  auto CharTypeExpr = expr();
+  if (IsSigned) {
+CharTypeExpr = expr(hasType(
+qualType(isAnyCharacter(), isSignedInteger(), unless(IntTypedef;
+  } else {
+CharTypeExpr = expr(hasType(qualType(
+isAnyCharacter(), unless(isSignedInteger()), unless(IntTypedef;
+  }
 
-  // We are interested in signed char -> integer conversion.
   const auto ImplicitCastExpr =
-  implicitCastExpr(hasSourceExpression(SignedCharType),
+  implicitCastExpr(hasSourceExpression(CharTypeExpr),
hasImplicitDestinationType(IntegerType))
-  .bind("castExpression");
+  .bind(CastBindName);
 
   const auto CStyleCastExpr = cStyleCastExpr(has(ImplicitCastExpr));
   const auto StaticCastExpr = cxxStaticCastExpr(has(ImplicitCastExpr));
@@ -59,44 +65,84 @@ void SignedCharMisuseCheck::registerMatchers(MatchFinder 
*Finder) {
 
   // We catch any type of casts to an integer. We need to have these cast
   // expressions explicitly to catch only those casts which are direct children
-  // of an assignment/declaration.
-  const auto CastExpr = expr(anyOf(ImplicitCastExpr, CStyleCastExpr,
-   StaticCastExpr, FunctionalCastExpr));
+  // of the checked expressions. (e.g. assignment, declaration).
+  return expr(anyOf(ImplicitCastExpr, CStyleCastExpr, StaticCastExpr,
+FunctionalCastExpr));
+}
 
-  // Catch assignments with the suspicious type conversion.
-  const auto AssignmentOperatorExpr = expr(binaryOperator(
-  hasOperatorName("="), hasLHS(hasType(IntegerType)), hasRHS(CastExpr)));
+void SignedCharMisuseCheck::registerMatchers(MatchFinder *Finder) {
+  const auto IntegerType =
+  qualType(isInteger(), unless(isAnyCharacter()), unless(booleanType()))
+  .bind("integerType");
+  const auto SignedCharCastExpr =
+  charCastExpression(true, IntegerType, "signedCastExpression");
+  const auto UnSignedCharCastExpr =
+  charCastExpression(false, IntegerType, "unsignedCastExpression");
+
+  // Catch assignments with singed char -> integer conversion.
+  const auto

[PATCH] D76182: [AVR] Support aliases in non-zero address space

2020-03-14 Thread Ayke via Phabricator via cfe-commits 
aykevl created this revision.
aykevl added reviewers: dylanmckay, rsmith, rjmccall.
Herald added subscribers: cfe-commits, Jim.
Herald added a project: clang.

This fixes code like the following on AVR:

  void foo(void) {
  }
  void bar(void) __attribute__((alias("foo")));

Code like this is present in compiler-rt, which I'm trying to build.

---

I'm not sure how to add a test for this, are there any examples I can look at? 
And does this need a test at all, considering how trivial the change is?


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D76182

Files:
  clang/lib/CodeGen/CodeGenModule.cpp


Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -4547,8 +4547,9 @@
   }
 
   // Create the new alias itself, but don't set a name yet.
+  unsigned AS = cast(Aliasee->getType())->getAddressSpace();
   auto *GA =
-  llvm::GlobalAlias::create(DeclTy, 0, LT, "", Aliasee, ());
+  llvm::GlobalAlias::create(DeclTy, AS, LT, "", Aliasee, ());
 
   if (Entry) {
 if (GA->getAliasee() == Entry) {


Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -4547,8 +4547,9 @@
   }
 
   // Create the new alias itself, but don't set a name yet.
+  unsigned AS = cast(Aliasee->getType())->getAddressSpace();
   auto *GA =
-  llvm::GlobalAlias::create(DeclTy, 0, LT, "", Aliasee, ());
+  llvm::GlobalAlias::create(DeclTy, AS, LT, "", Aliasee, ());
 
   if (Entry) {
 if (GA->getAliasee() == Entry) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D76181: [AVR] Add support for the -mdouble=x flag

2020-03-14 Thread Ayke via Phabricator via cfe-commits 
aykevl created this revision.
aykevl added reviewers: dylanmckay, MaskRay, hfinkel, rnk, rsmith.
Herald added subscribers: cfe-commits, Jim.
Herald added a project: clang.

This flag is used by avr-gcc (starting with v10) to set the width of the double 
type. The double type is by default interpreted as a 32-bit floating point 
number in avr-gcc instead of a 64-bit floating point number as is common on 
other architectures. Starting with GCC 10, a new option has been added to 
control this behavior:
https://gcc.gnu.org/wiki/avr-gcc#Deviations_from_the_Standard

This commit keeps the default double at 32 bits but adds support for the 
`-mdouble` flag (`-mdouble=32` and `-mdouble=64`) to control this behavior.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D76181

Files:
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/Basic/TargetInfo.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/mdouble.c
  clang/test/Driver/mdouble.c

Index: clang/test/Driver/mdouble.c
===
--- /dev/null
+++ clang/test/Driver/mdouble.c
@@ -0,0 +1,7 @@
+// RUN: %clang -target avr-unknown-unknown -c -### %s -mdouble=64 2>&1 | FileCheck %s
+
+// CHECK: "-mdouble=64"
+
+// RUN: %clang -target aarch64 -c -### %s -mdouble=64 2>&1 | FileCheck --check-prefix=ERR %s
+
+// ERR: error: unsupported option '-mdouble=64' for target 'aarch64'
Index: clang/test/CodeGen/mdouble.c
===
--- /dev/null
+++ clang/test/CodeGen/mdouble.c
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 %s -emit-llvm -o - -triple=avr-unknown-unknown -mdouble=64 | \
+// RUN:   FileCheck --check-prefix=AVR-FP64 %s
+// RUN: %clang_cc1 %s -emit-llvm -o - -triple=avr-unknown-unknown -mdouble=32 | \
+// RUN:   FileCheck --check-prefix=AVR-FP32 %s
+
+double x = 0;
+int size = sizeof(x);
+
+// FIXME: the double should have an alignment of 1 on AVR, not 4 or 8.
+// AVR-FP64: @x = global double {{.*}}, align 8
+// AVR-FP64: @size = global i16 8
+// AVR-FP32: @x = global float {{.*}}, align 4
+// AVR-FP32: @size = global i16 4
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -2937,6 +2937,7 @@
   Opts.PackStruct = getLastArgIntValue(Args, OPT_fpack_struct_EQ, 0, Diags);
   Opts.MaxTypeAlign = getLastArgIntValue(Args, OPT_fmax_type_align_EQ, 0, Diags);
   Opts.AlignDouble = Args.hasArg(OPT_malign_double);
+  Opts.DoubleSize = getLastArgIntValue(Args, OPT_mdouble_EQ, 0, Diags);
   Opts.LongDoubleSize = Args.hasArg(OPT_mlong_double_128)
 ? 128
 : Args.hasArg(OPT_mlong_double_64) ? 64 : 0;
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4591,6 +4591,14 @@
   << A->getAsString(Args) << TripleStr;
   }
 
+  if (Arg *A = Args.getLastArg(options::OPT_mdouble_EQ)) {
+if (TC.getArch() == llvm::Triple::avr)
+  A->render(Args, CmdArgs);
+else
+  D.Diag(diag::err_drv_unsupported_opt_for_target)
+  << A->getAsString(Args) << TripleStr;
+  }
+
   // Decide whether to use verbose asm. Verbose assembly is the default on
   // toolchains which have the integrated assembler on by default.
   bool IsIntegratedAssemblerDefault = TC.IsIntegratedAssemblerDefault();
Index: clang/lib/Basic/TargetInfo.cpp
===
--- clang/lib/Basic/TargetInfo.cpp
+++ clang/lib/Basic/TargetInfo.cpp
@@ -380,6 +380,20 @@
 LongDoubleFormat = ::APFloat::IEEEquad();
   }
 
+  if (Opts.DoubleSize) {
+if (Opts.DoubleSize == 32) {
+  DoubleWidth = 32;
+  LongDoubleWidth = 32;
+  DoubleFormat = ::APFloat::IEEEsingle();
+  LongDoubleFormat = ::APFloat::IEEEsingle();
+} else if (Opts.DoubleSize == 64) {
+  DoubleWidth = 64;
+  LongDoubleWidth = 64;
+  DoubleFormat = ::APFloat::IEEEdouble();
+  LongDoubleFormat = ::APFloat::IEEEdouble();
+}
+  }
+
   if (Opts.LongDoubleSize) {
 if (Opts.LongDoubleSize == DoubleWidth) {
   LongDoubleWidth = DoubleWidth;
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -2179,6 +2179,8 @@
 def mfancy_math_387 : Flag<["-"], "mfancy-math-387">, Group;
 def mlong_calls : Flag<["-"], "mlong-calls">, Group,
   HelpText<"Generate branches with extended addressability, usually via indirect jumps.">;
+def mdouble_EQ : Joined<["-"], "mdouble=">, Group, Values<"32,64">, Flags<[CC1Option]>,
+  HelpText<"Force double to be 32

[PATCH] D74692: [clang-tidy][bugprone-use-after-move] Warn on std::move for consts

2020-03-14 Thread Aaron Ballman via Phabricator via cfe-commits 
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM aside from the `const` nits. You should wait for @Quuxplusone before 
committing in case there are any additional comments.




Comment at: clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp:383
   SourceLocation MoveLoc = MovingCall->getExprLoc();
+  const bool MoveArgIsConst = MoveArg->getType().isConstQualified();
 

aaron.ballman wrote:
> Drop the top-level `const` and make it an integer type so that you don't need 
> to use the `?:` below. The implicit conversion will do the right thing in 
> terms of the integer value.
Top-level `const` is still here on the declaration.



Comment at: clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp:393
+if (MoveArg->getDecl() == Capture.getCapturedVar()) {
+  const int IsExplicitCapture = Capture.isExplicit();
+  Check->diag(IsExplicitCapture ? Capture.getLocation()

Top-level `const` is still here as well.


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

https://reviews.llvm.org/D74692



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

[PATCH] D75210: [Attr] Allow ParsedAttr to be constructor for any Attribute

2020-03-14 Thread Aaron Ballman via Phabricator via cfe-commits 
aaron.ballman added a comment.

In D75210#1920206 , @jdoerfert wrote:

> The reason for this patch is not there anymore. I'm fine with postponing this 
> patch until there is a user again, thoughts?


If we don't have a use for it currently, I think we should postpone it until 
there is a need.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75210



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

[PATCH] D75844: [clang] Set begin loc on GNU attribute parsed attrs

2020-03-14 Thread Aaron Ballman via Phabricator via cfe-commits 
aaron.ballman added a reviewer: rsmith.
aaron.ballman added a subscriber: rsmith.
aaron.ballman added a comment.

I think this is a reasonable approach (including moving 
`ParsedAttributesWithRange` into `Sema`), but we should have test coverage for 
the places where we're now using a range where we weren't previously (to ensure 
we're okay with the positional changes we're introducing).

Adding @rsmith to ensure I've not misunderstood layering concerns.




Comment at: clang/test/SemaCXX/switch-implicit-fallthrough.cpp:110-114
+case 26:
+  return 0;
+__attribute__((fallthrough)); // expected-warning{{fallthrough annotation 
in unreachable code}}
+case 27:
+  break;

This test seems unrelated to the patch, or am I misunderstanding something?


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

https://reviews.llvm.org/D75844



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

[PATCH] D31342: Add ParsedAttrInfo::handleDeclAttribute

2020-03-14 Thread Aaron Ballman via Phabricator via cfe-commits 
aaron.ballman added inline comments.



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:6701-6707
 if (!AL.isStmtAttr()) {
   // Type attributes are handled elsewhere; silently move on.
   assert(AL.isTypeAttr() && "Non-type attribute not handled");
   break;
 }
 S.Diag(AL.getLoc(), diag::err_stmt_attribute_invalid_on_decl)
 << AL << D->getLocation();

john.brawn wrote:
> aaron.ballman wrote:
> > john.brawn wrote:
> > > aaron.ballman wrote:
> > > > If we're in the situation where we parse a plugin-based attribute (so 
> > > > `AL.getKind()` falls into the `default` label) and its 
> > > > `handleDeclAttributes()` (incorrectly) returns false rather than true, 
> > > > it seems like we would not want to trigger either of these tests, 
> > > > correct? I'm thinking about a case where the plugin knows about that 
> > > > attribute but cannot apply it because of some other issues (maybe the 
> > > > subject is wrong, or args are incorrect, etc) and returns `false` here. 
> > > > We wouldn't want to assert or claim that this is an invalid statement 
> > > > attribute applied to a decl in that case.
> > > > 
> > > > Rather than requiring the user to return true from 
> > > > `handleDeclAttribute()`, it seems that what we really want is something 
> > > > more like:
> > > > ```
> > > > if (AL.getInfo().hasHandlerFunc()) {
> > > >   AL.getInfo().handleDeclAttribute(S, D, AL);
> > > >   break;
> > > > }
> > > > if (!AL.isStmtAttr()) {
> > > >   ...
> > > > }
> > > > S.Diag(...);
> > > > ```
> > > > where an unknown attribute does not have a handler, but simple and 
> > > > plugin attributes do have a handler. This way the user doesn't have the 
> > > > chance to accidentally get confused about what it means to handle an 
> > > > attribute. WDYT?
> > > That sounds a little overly complex, as instead of defining one thing you 
> > > now have to define two things so you have the possibility of defining one 
> > > but not the other which won't do anything useful.
> > I was hoping we could do it in such a way that the plugin user defines 
> > `handleDeclAttribute()` and the `hasHandlerFunc()` logic is automatic. This 
> > way, the user just has to define their function and "everything just works".
> > 
> > My big concern here is that we have a function with a useless return value 
> > (from the plugin author's perspective) but they're likely to get the 
> > behavior wrong through simple misunderstanding. Unless the plugin author is 
> > careful with their testing, this will lead to failed assertions when we go 
> > to check for whether it's type attribute or not.
> I don't think there's any straightforward way that hasHanderFunc could be 
> implemented automatically. The obvious choice of
> 
> ```
>   return  == ::handleDeclAttribute;
> ```
> doesn't work as  is only allowed if X.Y is an lvalue, which isn't the 
> case for a member function.
> 
> We could do something by use of a template:
> 
> ```
> class ParsedAttrInfo {
> public:
>   virtual bool hasHandlerFunc() = 0;
>   virtual void handleDeclAttribute() { }
> };
> 
> template class ParsedAttrInfoTemplate : public ParsedAttrInfo {
> public:
>   virtual bool hasHandlerFunc() {
> return ::handleDeclAttribute != ::handleDeclAttribute;
>   }
> };
> 
> class ExampleAttribute : public ParsedAttrInfoTemplate {
> public:
>   virtual void handleDeclAttribute() { }
> };
> 
> void call_fn(ParsedAttrInfo *p) {
>   if (p->hasHandlerFunc()) {
> p->handleDeclAttribute();
>   }
> }
> ```
> This works, but now we have to be careful to inherit from 
> ParsedAttrInfoTemplate in the right way.
Thank you for investigating that as a possible solution -- I agree that it 
doesn't seem too feasible. However, my concern still stands that this 
particular bit of code is too fragile and can lead to mysterious behavior for 
plugin authors pretty easily.

What about a tri-state boolean return value (aka, an enum)? The default 
`ParsedAttrInfo::handleDeclAttribute` function could return "attribute not 
known", and the other two states could be "attribute applied" or "attribute not 
applied". The logic here would then become something like:
```
if (AL.getInfo().handleDeclAttribute(S, D, AL) != AttributeNotKnown)
  break;
if (!AL.isStmtAttr()) {
  // Type attributes are handled elsewhere; silently move on.
  assert(AL.isTypeAttr() && "Non-type attribute not handled");
  break;
}
S.Diag(AL.getLoc(), diag::err_stmt_attribute_invalid_on_decl)
<< AL << D->getLocation();
```


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

https://reviews.llvm.org/D31342



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

[PATCH] D75745: [clang-tidy] Added AllowMissingMoveFunctionsWhenCopyIsDeleted flag to cppcoreguidelines-special-member-functions

2020-03-14 Thread Aaron Ballman via Phabricator via cfe-commits 
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75745



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

[PATCH] D76173: [OpenMP][NFC] Minimize memory usage and copying of `OMPTraitInfo`s

2020-03-14 Thread Reid Kleckner via Phabricator via cfe-commits 
rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.

lgtm, thanks!

I also noticed that Parser.h includes OpenMPClause.h just for this class. 
OpenMPClause.h is pretty big. It's for this family of related methods:

  /// Parse a property kind into \p TIProperty for the selector set \p Set and
  /// selector \p Selector.
  void parseOMPTraitPropertyKind(OMPTraitInfo::OMPTraitProperty ,
 llvm::omp::TraitSet Set,
 llvm::omp::TraitSelector Selector,
 llvm::StringMap );

The use of an inner class here makes it impossible to forward declare 
OMPTraitProperty. Would you be opposed to moving it out of line?

The other popular includer of OpenMPClause.h is Attr.h, which is where I 
started my investigation. I think after your change, we can forward declare it. 
I'll look into it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76173



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

[PATCH] D75001: [OpenMP][cmake] ignore warning on unknown CUDA version

2020-03-14 Thread Kelvin Li via Phabricator via cfe-commits 
kkwli0 added a comment.

In D75001#1922789 , @jdoerfert wrote:

> In D75001#1891284 , @Hahnfeld wrote:
>
> > In D75001#1887876 , @jdoerfert 
> > wrote:
> >
> > > I like this way better. I was hoping we could do it in our cmake only :)
> > >
> > > Give others a day or so to comment before your commit but I'm fine with 
> > > this.
> >
> >
> > Well, that doesn't really work if `openmp-commits` is only subscribed on 
> > commit. That said, the solution is a bit ugly but I don't have an 
> > alternative right now.
>
>
> What is the problem with `openmp-commits` here? I got the emails, didn't you?
>
> @kkwli0 Where are we with this?


@jdoerfert  In terms of the scope of this issue, it is done.  It is to ignore 
the warning in order to avoid build blockage in the future when newer CUDA 
toolkit is available and installed on the build system.  Another related issue 
that blocked the other part of the tests (related to python binding) was 
resolved in https://reviews.llvm.org/D76030.  As far as I know, the remaining 
one (not quite an issue, it is by design) is warning issued when 
-fopenm-targets=nvptx* is specified, users can suppress it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75001



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

[PATCH] D73534: [DebugInfo] Enable the debug entry values feature by default

2020-03-14 Thread Nico Weber via Phabricator via cfe-commits 
thakis added a comment.

This makes clang crash, repro at 
https://bugs.chromium.org/p/chromium/issues/detail?id=1061533#c4 I'll revert 
for now.

I'm also seeing timeouts and use-after-frees on other bots; maybe they're 
related (or maybe not).


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

https://reviews.llvm.org/D73534



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

[PATCH] D73534: [DebugInfo] Enable the debug entry values feature by default

2020-03-14 Thread Djordje Todorovic via Phabricator via cfe-commits 
djtodoro added a comment.

Why reverting this one? Is it a different assertion?

The D75036  was the problem with the previous 
issue reported.


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

https://reviews.llvm.org/D73534



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

[PATCH] D73534: [DebugInfo] Enable the debug entry values feature by default

2020-03-14 Thread Adrian Prantl via Phabricator via cfe-commits 
aprantl added a comment.

FYI: See also https://reviews.llvm.org/D76164


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

https://reviews.llvm.org/D73534



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

[PATCH] D73534: [DebugInfo] Enable the debug entry values feature by default

2020-03-14 Thread Nico Weber via Phabricator via cfe-commits 
thakis added a comment.

Reduced repro at 
https://bugs.chromium.org/p/chromium/issues/detail?id=1061533#c7

Scrolling up, it looks like this was reverted yesterday already. I'm confused 
why this wasn't reverted yesterday; all work I did in bisecting and creducing 
was a duplicate of what happened yesterday :/


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

https://reviews.llvm.org/D73534



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

[PATCH] D75791: [clang-format] Added new option IndentExternBlock

2020-03-14 Thread Marcus Johnson via Phabricator via cfe-commits 
MarcusJohnson91 updated this revision to Diff 250354.
MarcusJohnson91 added a comment.

Fixed Format.h comments, and rebased on master.

it's perfect on my end now.


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

https://reviews.llvm.org/D75791

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -2460,14 +2460,14 @@
 }
 
 TEST_F(FormatTest, FormatsExternC) {
-  verifyFormat("extern \"C\" {\nint a;");
+  verifyFormat("extern \"C\" {\nint a; /*2.1*/");
   verifyFormat("extern \"C\" {}");
   verifyFormat("extern \"C\" {\n"
-   "int foo();\n"
+   "int FormatsExternC_1();\n"
"}");
-  verifyFormat("extern \"C\" int foo() {}");
-  verifyFormat("extern \"C\" int foo();");
-  verifyFormat("extern \"C\" int foo() {\n"
+  verifyFormat("extern \"C\" int FormatsExternC_2() {}");
+  verifyFormat("extern \"C\" int FormatsExternC_3();");
+  verifyFormat("extern \"C\" int FormatsExternC_4() {\n"
"  int i = 42;\n"
"  return i;\n"
"}");
@@ -2475,9 +2475,9 @@
   FormatStyle Style = getLLVMStyle();
   Style.BreakBeforeBraces = FormatStyle::BS_Custom;
   Style.BraceWrapping.AfterFunction = true;
-  verifyFormat("extern \"C\" int foo() {}", Style);
-  verifyFormat("extern \"C\" int foo();", Style);
-  verifyFormat("extern \"C\" int foo()\n"
+  verifyFormat("extern \"C\" int FormatsExternC_5() {}", Style);
+  verifyFormat("extern \"C\" int FormatsExternC_6();", Style);
+  verifyFormat("extern \"C\" int FormatsExternC_7()\n"
"{\n"
"  int i = 42;\n"
"  return i;\n"
@@ -2486,16 +2486,41 @@
 
   Style.BraceWrapping.AfterExternBlock = true;
   Style.BraceWrapping.SplitEmptyRecord = false;
-  verifyFormat("extern \"C\"\n"
-   "{}",
-   Style);
-  verifyFormat("extern \"C\"\n"
-   "{\n"
-   "  int foo();\n"
+  verifyFormat("extern \"C\"\n{}", Style);
+  verifyFormat("extern \"C\"\n{\nint FormatsExternC_8();\n}", Style);
+
+  Style.BraceWrapping.AfterExternBlock = false;
+  verifyFormat("extern \"C\" {}", Style);
+  verifyFormat("extern \"C\" {\n"
+   "int FormatsExternC_9();\n"
"}",
Style);
 }
 
+TEST_F(FormatTest, FormatsExternBlock) {
+  FormatStyle Style = getLLVMStyle();
+  Style.IndentWidth = 2;
+  Style.BraceWrapping.AfterExternBlock = true;
+  Style.IndentExternBlock = true;
+  verifyFormat("extern \"C\" {}", Style);
+  verifyFormat("extern \"C\" {\n  int FormatsExternBlock_1();\n}", Style);
+
+  Style.BraceWrapping.AfterExternBlock = false;
+  Style.IndentExternBlock = true;
+  verifyFormat("extern \"C\" {}", Style);
+  verifyFormat("extern \"C\" {\n  int FormatsExternBlock_2();\n}", Style);
+
+  Style.BraceWrapping.AfterExternBlock = true;
+  Style.IndentExternBlock = false;
+  verifyFormat("extern \"C\" {}", Style);
+  verifyFormat("extern \"C\" {\nint FormatsExternBlock_3();\n}", Style);
+
+  Style.BraceWrapping.AfterExternBlock = false;
+  Style.IndentExternBlock = false;
+  verifyFormat("extern \"C\" {}", Style);
+  verifyFormat("extern \"C\" {\nint FormatsExternBlock_4();\n}", Style);
+}
+
 TEST_F(FormatTest, FormatsInlineASM) {
   verifyFormat("asm(\"xyz\" : \"=a\"(a), \"=d\"(b) : \"a\"(data));");
   verifyFormat("asm(\"nop\" ::: \"memory\");");
@@ -12660,6 +12685,7 @@
   CHECK_PARSE_BOOL(IndentCaseBlocks);
   CHECK_PARSE_BOOL(IndentGotoLabels);
   CHECK_PARSE_BOOL(IndentWrappedFunctionNames);
+  CHECK_PARSE_BOOL(IndentExternBlock);
   CHECK_PARSE_BOOL(KeepEmptyLinesAtTheStartOfBlocks);
   CHECK_PARSE_BOOL(ObjCSpaceAfterProperty);
   CHECK_PARSE_BOOL(ObjCSpaceBeforeProtocolList);
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1094,11 +1094,21 @@
 if (FormatTok->Tok.is(tok::string_literal)) {
   nextToken();
   if (FormatTok->Tok.is(tok::l_brace)) {
-if (Style.BraceWrapping.AfterExternBlock) {
+if (Style.BraceWrapping.AfterExternBlock == true &&
+Style.IndentExternBlock == true) {
   addUnwrappedLine();
-  parseBlock(/*MustBeDeclaration=*/true);
-} else {
+  parseBlock(/*MustBeDeclaration=*/true, /*AddLevel=*/true);
+} else if (Style.BraceWrapping.AfterExternBlock == false &&
+   Style.IndentExternBlock == false) {
   parseBlock(/*MustBeDeclaration=*/true, /*AddLevel=*/false);
+} else if (Style.BraceWrapping.AfterExternBlock == false &&
+

[PATCH] D52773: clang-cl: Add /showFilenames option (PR31957)

2020-03-14 Thread Asuka Inori via Phabricator via cfe-commits 
asukainori added a comment.

I'm sorry to disturb.

But I found this option is conflict with -flto. e.g.

  clang-cl -fuse-ld=lld -flto=thin /showFilenames .\test.cpp

will output:

  clang-cl: warning: argument unused during compilation: '/showFilenames' 
[-Wunused-command-line-argument]


Repository:
  rL LLVM

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

https://reviews.llvm.org/D52773



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

[PATCH] D75001: [OpenMP][cmake] ignore warning on unknown CUDA version

2020-03-14 Thread Jonas Hahnfeld via Phabricator via cfe-commits 
Hahnfeld added a comment.

In D75001#1922789 , @jdoerfert wrote:

> In D75001#1891284 , @Hahnfeld wrote:
>
> > In D75001#1887876 , @jdoerfert 
> > wrote:
> >
> > > I like this way better. I was hoping we could do it in our cmake only :)
> > >
> > > Give others a day or so to comment before your commit but I'm fine with 
> > > this.
> >
> >
> > Well, that doesn't really work if `openmp-commits` is only subscribed on 
> > commit. That said, the solution is a bit ugly but I don't have an 
> > alternative right now.
>
>
> What is the problem with `openmp-commits` here? I got the emails, didn't you?


There's nothing wrong with `openmp-commits`. I wanted to say that you have to 
subscribe the ML **before** committing. Otherwise only the reviewers are 
notified and there cannot be "others" chiming in.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75001



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

[PATCH] D71830: [OpenMP][Part 2] Use reusable OpenMP context/traits handling

2020-03-14 Thread Johannes Doerfert via Phabricator via cfe-commits 
jdoerfert marked an inline comment as done.
jdoerfert added inline comments.



Comment at: clang/include/clang/AST/OpenMPClause.h:6682
+  /// The outermost level of selector sets.
+  llvm::SmallVector Sets;
+

jdoerfert wrote:
> rnk wrote:
> > This is not a good data structure choice. You have three levels of small 
> > vector nesting, so sizeof OMPTraitInfo is 880 bytes, and then you are 
> > passing it by value in in some of the attribute classes. Are you sure you 
> > wanted to do that?
> I could make them all 0 elements long, reducing the size quite a bit for the 
> common case of very few trait sets/selectors/properties. I could also try to 
> dynamically allocate them once. WDYT?
One way of fixing this is D76173


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71830



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