[PATCH] D29827: [AVR] Add -mmcu option to the driver

[Leslie Zhai via Phabricator via cfe-commits]

xiangzhai added a comment.

Hi Jonathan,

Peter might do not have commit access, please commit it on his behalf, thanks a 
lot!

Regards,
Leslie Zhai


https://reviews.llvm.org/D29827



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

r300667 - Add support for editor placeholders to Clang

[Alex Lorenz via cfe-commits]

Author: arphaman
Date: Wed Apr 19 03:58:56 2017
New Revision: 300667

URL: http://llvm.org/viewvc/llvm-project?rev=300667&view=rev
Log:
Add support for editor placeholders to Clang

This commit teaches Clang to recognize editor placeholders that are produced
when an IDE like Xcode inserts a code-completion result that includes a
placeholder. Now when the lexer sees a placeholder token, it emits an
'editor placeholder in source file' error and creates an identifier token
that represents the placeholder. The parser/sema can now recognize the
placeholders and can suppress the diagnostics related to the placeholders. This
ensures that live issues in an IDE like Xcode won't get spurious diagnostics
related to placeholders.

This commit also adds a new compiler option named '-fallow-editor-placeholders'
that silences the 'editor placeholder in source file' error. This is useful
for an IDE like Xcode as we don't want to display those errors in live issues.

rdar://31581400

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

Added:
cfe/trunk/test/Parser/editor-placeholder-recovery.cpp
Modified:
cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td
cfe/trunk/include/clang/Basic/IdentifierTable.h
cfe/trunk/include/clang/Basic/LangOptions.def
cfe/trunk/include/clang/Driver/Options.td
cfe/trunk/include/clang/Lex/Lexer.h
cfe/trunk/include/clang/Lex/Token.h
cfe/trunk/lib/Driver/ToolChains/Clang.cpp
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/lib/Lex/Lexer.cpp
cfe/trunk/lib/Parse/Parser.cpp
cfe/trunk/lib/Sema/SemaCXXScopeSpec.cpp
cfe/trunk/lib/Sema/SemaDecl.cpp
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/test/Driver/clang_f_opts.c
cfe/trunk/test/Parser/placeholder-recovery.m

Modified: cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td?rev=300667&r1=300666&r2=300667&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td Wed Apr 19 03:58:56 2017
@@ -242,6 +242,7 @@ def warn_bad_character_encoding : ExtWar
   "illegal character encoding in character literal">,
   InGroup;
 def err_lexing_string : Error<"failure when lexing a string">;
+def err_placeholder_in_source : Error<"editor placeholder in source file">;
 
 
 
//===--===//

Modified: cfe/trunk/include/clang/Basic/IdentifierTable.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/IdentifierTable.h?rev=300667&r1=300666&r2=300667&view=diff
==
--- cfe/trunk/include/clang/Basic/IdentifierTable.h (original)
+++ cfe/trunk/include/clang/Basic/IdentifierTable.h Wed Apr 19 03:58:56 2017
@@ -355,6 +355,19 @@ public:
   RecomputeNeedsHandleIdentifier();
   }
 
+  /// Return true if this identifier is an editor placeholder.
+  ///
+  /// Editor placeholders are produced by the code-completion engine and are
+  /// represented as characters between '<#' and '#>' in the source code. An
+  /// example of auto-completed call with a placeholder parameter is shown
+  /// below:
+  /// \code
+  ///   function(<#int x#>);
+  /// \endcode
+  bool isEditorPlaceholder() const {
+return getName().startswith("<#") && getName().endswith("#>");
+  }
+
   /// \brief Provide less than operator for lexicographical sorting.
   bool operator<(const IdentifierInfo &RHS) const {
 return getName() < RHS.getName();

Modified: cfe/trunk/include/clang/Basic/LangOptions.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/LangOptions.def?rev=300667&r1=300666&r2=300667&view=diff
==
--- cfe/trunk/include/clang/Basic/LangOptions.def (original)
+++ cfe/trunk/include/clang/Basic/LangOptions.def Wed Apr 19 03:58:56 2017
@@ -266,6 +266,8 @@ LANGOPT(SanitizeAddressFieldPadding, 2,
 
 LANGOPT(XRayInstrument, 1, 0, "controls whether to do XRay instrumentation")
 
+LANGOPT(AllowEditorPlaceholders, 1, 0, "allow editor placeholders in source")
+
 #undef LANGOPT
 #undef COMPATIBLE_LANGOPT
 #undef BENIGN_LANGOPT

Modified: cfe/trunk/include/clang/Driver/Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=300667&r1=300666&r2=300667&view=diff
==
--- cfe/trunk/include/clang/Driver/Options.td (original)
+++ cfe/trunk/include/clang/Driver/Options.td Wed Apr 19 03:58:56 2017
@@ -1487,6 +1487,12 @@ def fstrict_return : Flag<["-"], "fstric
 def fno_strict_return : Flag<["-"], "fno-strict-return">, Group,
   Flags<[CC1Option]>;
 
+def fallow_editor_placeholders : Flag<["-"], "fallow-editor-placeholders">,
+  Group

[PATCH] D32081: Add support for editor placeholders to Clang's lexer

[Alex Lorenz via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL300667: Add support for editor placeholders to Clang 
(authored by arphaman).

Changed prior to commit:
  https://reviews.llvm.org/D32081?vs=95596&id=95696#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D32081

Files:
  cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td
  cfe/trunk/include/clang/Basic/IdentifierTable.h
  cfe/trunk/include/clang/Basic/LangOptions.def
  cfe/trunk/include/clang/Driver/Options.td
  cfe/trunk/include/clang/Lex/Lexer.h
  cfe/trunk/include/clang/Lex/Token.h
  cfe/trunk/lib/Driver/ToolChains/Clang.cpp
  cfe/trunk/lib/Frontend/CompilerInvocation.cpp
  cfe/trunk/lib/Lex/Lexer.cpp
  cfe/trunk/lib/Parse/Parser.cpp
  cfe/trunk/lib/Sema/SemaCXXScopeSpec.cpp
  cfe/trunk/lib/Sema/SemaDecl.cpp
  cfe/trunk/lib/Sema/SemaExpr.cpp
  cfe/trunk/test/Driver/clang_f_opts.c
  cfe/trunk/test/Parser/editor-placeholder-recovery.cpp
  cfe/trunk/test/Parser/placeholder-recovery.m

Index: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
===
--- cfe/trunk/lib/Frontend/CompilerInvocation.cpp
+++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp
@@ -2322,6 +2322,9 @@
   Args.getAllArgValues(OPT_fxray_always_instrument);
   Opts.XRayNeverInstrumentFiles =
   Args.getAllArgValues(OPT_fxray_never_instrument);
+
+  // -fallow-editor-placeholders
+  Opts.AllowEditorPlaceholders = Args.hasArg(OPT_fallow_editor_placeholders);
 }
 
 static void ParsePreprocessorArgs(PreprocessorOptions &Opts, ArgList &Args,
Index: cfe/trunk/lib/Parse/Parser.cpp
===
--- cfe/trunk/lib/Parse/Parser.cpp
+++ cfe/trunk/lib/Parse/Parser.cpp
@@ -851,6 +851,10 @@
 
   default:
   dont_know:
+if (Tok.isEditorPlaceholder()) {
+  ConsumeToken();
+  return nullptr;
+}
 // We can't tell whether this is a function-definition or declaration yet.
 return ParseDeclarationOrFunctionDefinition(attrs, DS);
   }
@@ -1679,6 +1683,8 @@
   return false;
 }
   }
+  if (Tok.isEditorPlaceholder())
+return true;
 
   Diag(Tok.getLocation(), diag::err_expected_qualified_after_typename);
   return true;
Index: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp
@@ -2306,6 +2306,9 @@
   if (!Args.hasFlag(options::OPT_fstrict_return, options::OPT_fno_strict_return,
 true))
 CmdArgs.push_back("-fno-strict-return");
+  if (Args.hasFlag(options::OPT_fallow_editor_placeholders,
+   options::OPT_fno_allow_editor_placeholders, false))
+CmdArgs.push_back("-fallow-editor-placeholders");
   if (Args.hasFlag(options::OPT_fstrict_vtable_pointers,
options::OPT_fno_strict_vtable_pointers,
false))
Index: cfe/trunk/lib/Lex/Lexer.cpp
===
--- cfe/trunk/lib/Lex/Lexer.cpp
+++ cfe/trunk/lib/Lex/Lexer.cpp
@@ -2716,6 +2716,37 @@
   return false;
 }
 
+static const char *findPlaceholderEnd(const char *CurPtr,
+  const char *BufferEnd) {
+  if (CurPtr == BufferEnd)
+return nullptr;
+  BufferEnd -= 1; // Scan until the second last character.
+  for (; CurPtr != BufferEnd; ++CurPtr) {
+if (CurPtr[0] == '#' && CurPtr[1] == '>')
+  return CurPtr + 2;
+  }
+  return nullptr;
+}
+
+bool Lexer::lexEditorPlaceholder(Token &Result, const char *CurPtr) {
+  assert(CurPtr[-1] == '<' && CurPtr[0] == '#' && "Not a placeholder!");
+  if (!PP || LexingRawMode)
+return false;
+  const char *End = findPlaceholderEnd(CurPtr + 1, BufferEnd);
+  if (!End)
+return false;
+  const char *Start = CurPtr - 1;
+  if (!LangOpts.AllowEditorPlaceholders)
+Diag(Start, diag::err_placeholder_in_source);
+  Result.startToken();
+  FormTokenWithChars(Result, End, tok::raw_identifier);
+  Result.setRawIdentifierData(Start);
+  PP->LookUpIdentifierInfo(Result);
+  Result.setFlag(Token::IsEditorPlaceholder);
+  BufferPtr = End;
+  return true;
+}
+
 bool Lexer::isCodeCompletionPoint(const char *CurPtr) const {
   if (PP && PP->isCodeCompletionEnabled()) {
 SourceLocation Loc = FileLoc.getLocWithOffset(CurPtr-BufferStart);
@@ -3473,6 +3504,8 @@
 } else if (LangOpts.Digraphs && Char == '%') { // '<%' -> '{'
   CurPtr = ConsumeChar(CurPtr, SizeTmp, Result);
   Kind = tok::l_brace;
+} else if (Char == '#' && lexEditorPlaceholder(Result, CurPtr)) {
+  return true;
 } else {
   Kind = tok::less;
 }
Index: cfe/trunk/lib/Sema/SemaCXXScopeSpec.cpp
===
--- cfe/trunk/lib/Sema/SemaCXXScopeSpec.cpp
+++ cfe/trunk/lib/Sema/SemaCXXScopeSpec.cpp
@@ -480,6 +480,8 @@
   

[PATCH] D27251: [PPC] some bugs mainly about sign problem fixed in altivec.h

[ZiXuan Wu via Phabricator via cfe-commits]

Zeson abandoned this revision.
Zeson added a comment.

I think this revision is out-of-date. I'd like to abandon it.


https://reviews.llvm.org/D27251



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

[PATCH] D29827: [AVR] Add -mmcu option to the driver

[Peter Wu via Phabricator via cfe-commits]

Lekensteyn updated this revision to Diff 95707.
Lekensteyn added a comment.

v2: rebase on commit ad25f8b712f1ef99020fcb7b5e31dd95b39c6112 (trunk@300661)

Based on the context, the change from lib/Driver/Tools.cpp -> 
lib/Driver/ToolChains/CommonArgs.cpp seems most appropriate (Gnu.cpp seems to 
be about mapping LLVM options to GCC?)


https://reviews.llvm.org/D29827

Files:
  include/clang/Driver/Options.td
  lib/Driver/ToolChains/CommonArgs.cpp
  test/Driver/avr-mmcu.c


Index: test/Driver/avr-mmcu.c
===
--- /dev/null
+++ test/Driver/avr-mmcu.c
@@ -0,0 +1,5 @@
+// A test for the propagation of the -mmcu option to -cc1 and -cc1as
+
+// RUN: %clang -### -target avr -mmcu=atmega328p -save-temps %s 2>&1 | 
FileCheck %s
+// CHECK: clang{{.*}} "-cc1" {{.*}} "-target-cpu" "atmega328p"
+// CHECK: clang{{.*}} "-cc1as" {{.*}} "-target-cpu" "atmega328p"
Index: lib/Driver/ToolChains/CommonArgs.cpp
===
--- lib/Driver/ToolChains/CommonArgs.cpp
+++ lib/Driver/ToolChains/CommonArgs.cpp
@@ -261,6 +261,12 @@
 arm::getARMArchCPUFromArgs(Args, MArch, MCPU, FromAs);
 return arm::getARMTargetCPU(MCPU, MArch, T);
   }
+
+  case llvm::Triple::avr:
+if (const Arg *A = Args.getLastArg(options::OPT_mmcu_EQ))
+  return A->getValue();
+return "";
+
   case llvm::Triple::mips:
   case llvm::Triple::mipsel:
   case llvm::Triple::mips64:
Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -1655,6 +1655,7 @@
 def municode : Joined<["-"], "municode">, Group, 
Flags<[DriverOption]>;
 def mthreads : Joined<["-"], "mthreads">, Group, 
Flags<[DriverOption]>;
 def mcpu_EQ : Joined<["-"], "mcpu=">, Group;
+def mmcu_EQ : Joined<["-"], "mmcu=">, Group;
 def mdynamic_no_pic : Joined<["-"], "mdynamic-no-pic">, Group;
 def mfix_and_continue : Flag<["-"], "mfix-and-continue">, 
Group;
 def mieee_fp : Flag<["-"], "mieee-fp">, Group;


Index: test/Driver/avr-mmcu.c
===
--- /dev/null
+++ test/Driver/avr-mmcu.c
@@ -0,0 +1,5 @@
+// A test for the propagation of the -mmcu option to -cc1 and -cc1as
+
+// RUN: %clang -### -target avr -mmcu=atmega328p -save-temps %s 2>&1 | FileCheck %s
+// CHECK: clang{{.*}} "-cc1" {{.*}} "-target-cpu" "atmega328p"
+// CHECK: clang{{.*}} "-cc1as" {{.*}} "-target-cpu" "atmega328p"
Index: lib/Driver/ToolChains/CommonArgs.cpp
===
--- lib/Driver/ToolChains/CommonArgs.cpp
+++ lib/Driver/ToolChains/CommonArgs.cpp
@@ -261,6 +261,12 @@
 arm::getARMArchCPUFromArgs(Args, MArch, MCPU, FromAs);
 return arm::getARMTargetCPU(MCPU, MArch, T);
   }
+
+  case llvm::Triple::avr:
+if (const Arg *A = Args.getLastArg(options::OPT_mmcu_EQ))
+  return A->getValue();
+return "";
+
   case llvm::Triple::mips:
   case llvm::Triple::mipsel:
   case llvm::Triple::mips64:
Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -1655,6 +1655,7 @@
 def municode : Joined<["-"], "municode">, Group, Flags<[DriverOption]>;
 def mthreads : Joined<["-"], "mthreads">, Group, Flags<[DriverOption]>;
 def mcpu_EQ : Joined<["-"], "mcpu=">, Group;
+def mmcu_EQ : Joined<["-"], "mmcu=">, Group;
 def mdynamic_no_pic : Joined<["-"], "mdynamic-no-pic">, Group;
 def mfix_and_continue : Flag<["-"], "mfix-and-continue">, Group;
 def mieee_fp : Flag<["-"], "mieee-fp">, Group;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32176: Add #pragma clang attribute support for the external_source_symbol attribute

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: lib/Sema/SemaDeclAttr.cpp:5769
+// might include a subject list even when it has custom parsing.
+if (!Attr.diagnoseAppertainsTo(S, D))
+  return true;

Instead of duplicating this check in the if statement, why not hoist the 
current check above the if statement?



Comment at: utils/TableGen/ClangAttrEmitter.cpp:1620
+   bool IsRule)
+: Rules(Rules.begin(), Rules.end()), IsRule(IsRule) {}
+

ArrayRef automatically converts to a std::vector, so you should just be able to 
use `Rules(Rules)`.



Comment at: utils/TableGen/ClangAttrEmitter.cpp:1625
+
+AttributeSubjectMatchRule &getRule() {
+  assert(IsRule && "not a rule!");

Any reason not to return this rule by const ref and make the function const?



Comment at: utils/TableGen/ClangAttrEmitter.cpp:1701
+  std::vector DeclNodes = Records.getAllDerivedDefinitions("DDecl");
+  for (const Record *Aggregate : Aggregates) {
+Record *SubjectDecl = Aggregate->getValueAsDef("Subject");

Can use `const auto *` here


Repository:
  rL LLVM

https://reviews.llvm.org/D32176



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

r300686 - Avoid assert when a non-static member function is qualified with __unaligned

[Roger Ferrer Ibanez via cfe-commits]

Author: rogfer01
Date: Wed Apr 19 07:23:28 2017
New Revision: 300686

URL: http://llvm.org/viewvc/llvm-project?rev=300686&view=rev
Log:
Avoid assert when a non-static member function is qualified with __unaligned

Under -fms-extensions __unaligned is a type-qualifier that can be applied to a
non-static member function declaration.

This causes an assertion when mangling the name under Itanium, where that
qualifier is not mangled.

This patch justs makes the minimal change to avoid the crash and avoid mangling
__unaligned, as it currently happens with non-member functions.

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


Added:
cfe/trunk/test/CodeGenCXX/unaligned-duplicated-mangle-name.cpp
Modified:
cfe/trunk/lib/AST/ItaniumMangle.cpp

Modified: cfe/trunk/lib/AST/ItaniumMangle.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ItaniumMangle.cpp?rev=300686&r1=300685&r2=300686&view=diff
==
--- cfe/trunk/lib/AST/ItaniumMangle.cpp (original)
+++ cfe/trunk/lib/AST/ItaniumMangle.cpp Wed Apr 19 07:23:28 2017
@@ -1455,10 +1455,12 @@ void CXXNameMangler::mangleNestedName(co
   Out << 'N';
   if (const CXXMethodDecl *Method = dyn_cast(ND)) {
 Qualifiers MethodQuals =
-Qualifiers::fromCVRMask(Method->getTypeQualifiers());
+Qualifiers::fromCVRUMask(Method->getTypeQualifiers());
 // We do not consider restrict a distinguishing attribute for overloading
 // purposes so we must not mangle it.
 MethodQuals.removeRestrict();
+// __unaligned is not currently mangled in any way, so remove it.
+MethodQuals.removeUnaligned();
 mangleQualifiers(MethodQuals);
 mangleRefQualifier(Method->getRefQualifier());
   }

Added: cfe/trunk/test/CodeGenCXX/unaligned-duplicated-mangle-name.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/unaligned-duplicated-mangle-name.cpp?rev=300686&view=auto
==
--- cfe/trunk/test/CodeGenCXX/unaligned-duplicated-mangle-name.cpp (added)
+++ cfe/trunk/test/CodeGenCXX/unaligned-duplicated-mangle-name.cpp Wed Apr 19 
07:23:28 2017
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -triple %itanium_abi_triple -fms-extensions -emit-llvm-only 
%s -verify
+
+struct A
+{
+int x;
+void foo() __unaligned;
+void foo();
+};
+
+void A::foo() __unaligned
+{
+this->x++;
+}
+
+void A::foo() // expected-error {{definition with same mangled name as another 
definition}}
+  // expected-note@-6 {{previous definition is here}}
+{
+this->x++;
+}
+


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

[PATCH] D31976: Avoid assert when a non-static member function is qualified with __unaligned

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL300686: Avoid assert when a non-static member function is 
qualified with __unaligned (authored by rogfer01).

Changed prior to commit:
  https://reviews.llvm.org/D31976?vs=94962&id=95723#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D31976

Files:
  cfe/trunk/lib/AST/ItaniumMangle.cpp
  cfe/trunk/test/CodeGenCXX/unaligned-duplicated-mangle-name.cpp


Index: cfe/trunk/test/CodeGenCXX/unaligned-duplicated-mangle-name.cpp
===
--- cfe/trunk/test/CodeGenCXX/unaligned-duplicated-mangle-name.cpp
+++ cfe/trunk/test/CodeGenCXX/unaligned-duplicated-mangle-name.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -triple %itanium_abi_triple -fms-extensions -emit-llvm-only 
%s -verify
+
+struct A
+{
+int x;
+void foo() __unaligned;
+void foo();
+};
+
+void A::foo() __unaligned
+{
+this->x++;
+}
+
+void A::foo() // expected-error {{definition with same mangled name as another 
definition}}
+  // expected-note@-6 {{previous definition is here}}
+{
+this->x++;
+}
+
Index: cfe/trunk/lib/AST/ItaniumMangle.cpp
===
--- cfe/trunk/lib/AST/ItaniumMangle.cpp
+++ cfe/trunk/lib/AST/ItaniumMangle.cpp
@@ -1455,10 +1455,12 @@
   Out << 'N';
   if (const CXXMethodDecl *Method = dyn_cast(ND)) {
 Qualifiers MethodQuals =
-Qualifiers::fromCVRMask(Method->getTypeQualifiers());
+Qualifiers::fromCVRUMask(Method->getTypeQualifiers());
 // We do not consider restrict a distinguishing attribute for overloading
 // purposes so we must not mangle it.
 MethodQuals.removeRestrict();
+// __unaligned is not currently mangled in any way, so remove it.
+MethodQuals.removeUnaligned();
 mangleQualifiers(MethodQuals);
 mangleRefQualifier(Method->getRefQualifier());
   }


Index: cfe/trunk/test/CodeGenCXX/unaligned-duplicated-mangle-name.cpp
===
--- cfe/trunk/test/CodeGenCXX/unaligned-duplicated-mangle-name.cpp
+++ cfe/trunk/test/CodeGenCXX/unaligned-duplicated-mangle-name.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -triple %itanium_abi_triple -fms-extensions -emit-llvm-only %s -verify
+
+struct A
+{
+int x;
+void foo() __unaligned;
+void foo();
+};
+
+void A::foo() __unaligned
+{
+this->x++;
+}
+
+void A::foo() // expected-error {{definition with same mangled name as another definition}}
+  // expected-note@-6 {{previous definition is here}}
+{
+this->x++;
+}
+
Index: cfe/trunk/lib/AST/ItaniumMangle.cpp
===
--- cfe/trunk/lib/AST/ItaniumMangle.cpp
+++ cfe/trunk/lib/AST/ItaniumMangle.cpp
@@ -1455,10 +1455,12 @@
   Out << 'N';
   if (const CXXMethodDecl *Method = dyn_cast(ND)) {
 Qualifiers MethodQuals =
-Qualifiers::fromCVRMask(Method->getTypeQualifiers());
+Qualifiers::fromCVRUMask(Method->getTypeQualifiers());
 // We do not consider restrict a distinguishing attribute for overloading
 // purposes so we must not mangle it.
 MethodQuals.removeRestrict();
+// __unaligned is not currently mangled in any way, so remove it.
+MethodQuals.removeUnaligned();
 mangleQualifiers(MethodQuals);
 mangleRefQualifier(Method->getRefQualifier());
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32199: [TBAASan] A TBAA Sanitizer (Clang)

[Hal Finkel via Phabricator via cfe-commits]

hfinkel added a comment.

As a note: As follow-up work, we might want to extend Clang to add TBAA 
metadata to allocas and lifetime.start intrinsics so that the instrumentation 
pass can mark the types of memory upon declaration/construction instead of only 
upon first access.


https://reviews.llvm.org/D32199



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

[PATCH] D32207: Corrrect warn_unused_result attribute

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: lib/AST/Decl.cpp:3007
 const CXXRecordDecl *Ret = RetType->getAsCXXRecordDecl();
-const auto *MD = dyn_cast(this);
-if (Ret && !(MD && MD->getCorrespondingMethodInClass(Ret, true))) {
+auto OpCode = getOverloadedOperator();
+bool IsPostfix =

Please don't use `auto` here.



Comment at: lib/AST/Decl.cpp:3010
+(OpCode == OO_PlusPlus || OpCode == OO_MinusMinus) &&
+(this->getNumParams() + (isa(this) ? 1 : 0)) == 2;
+if (Ret && !IsPostfix) {

CXXMethodDecl represents a static or instance method, so I'm not certain this 
logic is quite correct.



Comment at: test/SemaCXX/warn-unused-result.cpp:166
+// are special-cased to not warn for return-type due to ubiquitousness.
+struct[[clang::warn_unused_result]] S {
+  S DoThing() { return {}; };

Can you also add a test with [[nodiscard]] and verify that the uses of 
increment/decrement *are* diagnosed?


https://reviews.llvm.org/D32207



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

[PATCH] D31097: [clang-tidy] don't warn about implicit widening casts in function calls

[Daniel Marjamäki via Phabricator via cfe-commits]

danielmarjamaki abandoned this revision.
danielmarjamaki added a comment.

I believe https://reviews.llvm.org/D32164 is better


Repository:
  rL LLVM

https://reviews.llvm.org/D31097



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

[PATCH] D30087: [Driver] Unify linking of OpenMP runtime. NFCI.

[Alexey Bataev via Phabricator via cfe-commits]

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

LG


https://reviews.llvm.org/D30087



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

r300689 - [Driver] Unify linking of OpenMP runtime. NFCI.

[Jonas Hahnfeld via cfe-commits]

Author: hahnfeld
Date: Wed Apr 19 08:55:39 2017
New Revision: 300689

URL: http://llvm.org/viewvc/llvm-project?rev=300689&view=rev
Log:
[Driver] Unify linking of OpenMP runtime. NFCI.

While at it, extend test for FreeBSD and check for -lrt iff on Linux.

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

Modified:
cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
cfe/trunk/lib/Driver/ToolChains/CommonArgs.h
cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
cfe/trunk/test/Driver/fopenmp.c

Modified: cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp?rev=300689&r1=300688&r2=300689&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp Wed Apr 19 08:55:39 2017
@@ -426,11 +426,12 @@ void tools::addArchSpecificRPath(const T
   }
 }
 
-void tools::addOpenMPRuntime(ArgStringList &CmdArgs, const ToolChain &TC,
- const ArgList &Args) {
+bool tools::addOpenMPRuntime(ArgStringList &CmdArgs, const ToolChain &TC,
+ const ArgList &Args, bool IsOffloadingHost,
+ bool GompNeedsRT) {
   if (!Args.hasFlag(options::OPT_fopenmp, options::OPT_fopenmp_EQ,
 options::OPT_fno_openmp, false))
-return;
+return false;
 
   switch (TC.getDriver().getOpenMPRuntime(Args)) {
   case Driver::OMPRT_OMP:
@@ -438,16 +439,24 @@ void tools::addOpenMPRuntime(ArgStringLi
 break;
   case Driver::OMPRT_GOMP:
 CmdArgs.push_back("-lgomp");
+
+if (GompNeedsRT)
+  CmdArgs.push_back("-lrt");
 break;
   case Driver::OMPRT_IOMP5:
 CmdArgs.push_back("-liomp5");
 break;
   case Driver::OMPRT_Unknown:
 // Already diagnosed.
-break;
+return false;
   }
 
+  if (IsOffloadingHost)
+CmdArgs.push_back("-lomptarget");
+
   addArchSpecificRPath(TC, Args, CmdArgs);
+
+  return true;
 }
 
 static void addSanitizerRuntime(const ToolChain &TC, const ArgList &Args,

Modified: cfe/trunk/lib/Driver/ToolChains/CommonArgs.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/CommonArgs.h?rev=300689&r1=300688&r2=300689&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/CommonArgs.h (original)
+++ cfe/trunk/lib/Driver/ToolChains/CommonArgs.h Wed Apr 19 08:55:39 2017
@@ -59,8 +59,10 @@ void AddAssemblerKPIC(const ToolChain &T
 
 void addArchSpecificRPath(const ToolChain &TC, const llvm::opt::ArgList &Args,
   llvm::opt::ArgStringList &CmdArgs);
-void addOpenMPRuntime(llvm::opt::ArgStringList &CmdArgs, const ToolChain &TC,
-  const llvm::opt::ArgList &Args);
+/// Returns true, if an OpenMP runtime has been added.
+bool addOpenMPRuntime(llvm::opt::ArgStringList &CmdArgs, const ToolChain &TC,
+  const llvm::opt::ArgList &Args,
+  bool IsOffloadingHost = false, bool GompNeedsRT = false);
 
 llvm::opt::Arg *getLastProfileUseArg(const llvm::opt::ArgList &Args);
 llvm::opt::Arg *getLastProfileSampleUseArg(const llvm::opt::ArgList &Args);

Modified: cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Gnu.cpp?rev=300689&r1=300688&r2=300689&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Gnu.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Gnu.cpp Wed Apr 19 08:55:39 2017
@@ -586,37 +586,15 @@ void tools::gnutools::Linker::ConstructJ
   bool WantPthread = Args.hasArg(options::OPT_pthread) ||
  Args.hasArg(options::OPT_pthreads);
 
-  if (Args.hasFlag(options::OPT_fopenmp, options::OPT_fopenmp_EQ,
-   options::OPT_fno_openmp, false)) {
+  // FIXME: Only pass GompNeedsRT = true for platforms with libgomp that
+  // require librt. Most modern Linux platforms do, but some may not.
+  if (addOpenMPRuntime(CmdArgs, ToolChain, Args,
+   JA.isHostOffloading(Action::OFK_OpenMP),
+   /* GompNeedsRT= */ true))
 // OpenMP runtimes implies pthreads when using the GNU toolchain.
 // FIXME: Does this really make sense for all GNU toolchains?
 WantPthread = true;
 
-// Also link the particular OpenMP runtimes.
-switch (ToolChain.getDriver().getOpenMPRuntime(Args)) {
-case Driver::OMPRT_OMP:
-  CmdArgs.push_back("-lomp");
-  break;
-case Driver::OMPRT_GOMP:
-  CmdArgs.push_back("-lgomp");
-
-  // FIXME: Exclude this for platforms with libgomp that don't require
-  // librt. Most modern Linux platforms require it, but some may not.
-  CmdArgs.push_back("-lrt");
-  break;
-   

[PATCH] D30087: [Driver] Unify linking of OpenMP runtime. NFCI.

[Jonas Hahnfeld via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL300689: [Driver] Unify linking of OpenMP runtime. NFCI. 
(authored by Hahnfeld).

Changed prior to commit:
  https://reviews.llvm.org/D30087?vs=94372&id=95733#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D30087

Files:
  cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
  cfe/trunk/lib/Driver/ToolChains/CommonArgs.h
  cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
  cfe/trunk/test/Driver/fopenmp.c

Index: cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
@@ -586,37 +586,15 @@
   bool WantPthread = Args.hasArg(options::OPT_pthread) ||
  Args.hasArg(options::OPT_pthreads);
 
-  if (Args.hasFlag(options::OPT_fopenmp, options::OPT_fopenmp_EQ,
-   options::OPT_fno_openmp, false)) {
+  // FIXME: Only pass GompNeedsRT = true for platforms with libgomp that
+  // require librt. Most modern Linux platforms do, but some may not.
+  if (addOpenMPRuntime(CmdArgs, ToolChain, Args,
+   JA.isHostOffloading(Action::OFK_OpenMP),
+   /* GompNeedsRT= */ true))
 // OpenMP runtimes implies pthreads when using the GNU toolchain.
 // FIXME: Does this really make sense for all GNU toolchains?
 WantPthread = true;
 
-// Also link the particular OpenMP runtimes.
-switch (ToolChain.getDriver().getOpenMPRuntime(Args)) {
-case Driver::OMPRT_OMP:
-  CmdArgs.push_back("-lomp");
-  break;
-case Driver::OMPRT_GOMP:
-  CmdArgs.push_back("-lgomp");
-
-  // FIXME: Exclude this for platforms with libgomp that don't require
-  // librt. Most modern Linux platforms require it, but some may not.
-  CmdArgs.push_back("-lrt");
-  break;
-case Driver::OMPRT_IOMP5:
-  CmdArgs.push_back("-liomp5");
-  break;
-case Driver::OMPRT_Unknown:
-  // Already diagnosed.
-  break;
-}
-if (JA.isHostOffloading(Action::OFK_OpenMP))
-  CmdArgs.push_back("-lomptarget");
-
-addArchSpecificRPath(ToolChain, Args, CmdArgs);
-  }
-
   AddRunTimeLibs(ToolChain, D, CmdArgs, Args);
 
   if (WantPthread && !isAndroid)
Index: cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
+++ cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
@@ -426,28 +426,37 @@
   }
 }
 
-void tools::addOpenMPRuntime(ArgStringList &CmdArgs, const ToolChain &TC,
- const ArgList &Args) {
+bool tools::addOpenMPRuntime(ArgStringList &CmdArgs, const ToolChain &TC,
+ const ArgList &Args, bool IsOffloadingHost,
+ bool GompNeedsRT) {
   if (!Args.hasFlag(options::OPT_fopenmp, options::OPT_fopenmp_EQ,
 options::OPT_fno_openmp, false))
-return;
+return false;
 
   switch (TC.getDriver().getOpenMPRuntime(Args)) {
   case Driver::OMPRT_OMP:
 CmdArgs.push_back("-lomp");
 break;
   case Driver::OMPRT_GOMP:
 CmdArgs.push_back("-lgomp");
+
+if (GompNeedsRT)
+  CmdArgs.push_back("-lrt");
 break;
   case Driver::OMPRT_IOMP5:
 CmdArgs.push_back("-liomp5");
 break;
   case Driver::OMPRT_Unknown:
 // Already diagnosed.
-break;
+return false;
   }
 
+  if (IsOffloadingHost)
+CmdArgs.push_back("-lomptarget");
+
   addArchSpecificRPath(TC, Args, CmdArgs);
+
+  return true;
 }
 
 static void addSanitizerRuntime(const ToolChain &TC, const ArgList &Args,
Index: cfe/trunk/lib/Driver/ToolChains/CommonArgs.h
===
--- cfe/trunk/lib/Driver/ToolChains/CommonArgs.h
+++ cfe/trunk/lib/Driver/ToolChains/CommonArgs.h
@@ -59,8 +59,10 @@
 
 void addArchSpecificRPath(const ToolChain &TC, const llvm::opt::ArgList &Args,
   llvm::opt::ArgStringList &CmdArgs);
-void addOpenMPRuntime(llvm::opt::ArgStringList &CmdArgs, const ToolChain &TC,
-  const llvm::opt::ArgList &Args);
+/// Returns true, if an OpenMP runtime has been added.
+bool addOpenMPRuntime(llvm::opt::ArgStringList &CmdArgs, const ToolChain &TC,
+  const llvm::opt::ArgList &Args,
+  bool IsOffloadingHost = false, bool GompNeedsRT = false);
 
 llvm::opt::Arg *getLastProfileUseArg(const llvm::opt::ArgList &Args);
 llvm::opt::Arg *getLastProfileSampleUseArg(const llvm::opt::ArgList &Args);
Index: cfe/trunk/test/Driver/fopenmp.c
===
--- cfe/trunk/test/Driver/fopenmp.c
+++ cfe/trunk/test/Driver/fopenmp.c
@@ -18,29 +18,33 @@
 // CHECK-CC1-NO-OPENMP-NOT: "-fopenmp"
 //
 // RUN

[PATCH] D29827: [AVR] Add -mmcu option to the driver

[Leslie Zhai via Phabricator via cfe-commits]

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

Hi Peter,

Thanks for your rebase! LGTM!

> Based on the context, the change from lib/Driver/Tools.cpp -> 
> lib/Driver/ToolChains/CommonArgs.cpp seems most appropriate (Gnu.cpp seems to 
> be about mapping LLVM options to GCC?)

Sorry for my mistake! yes, I am maintaining similar patch 

 for GCC toolchain triples :) also rebased to 
lib/Driver/ToolChains/CommonArgs.cpp for packaging llvm-4.0.0 
.

> And please do commit on my behalf, I haven't requested an account yet. Thanks!

I will commit it for you tomorrow if no **Need Review**!

Regards,
Leslie Zhai


https://reviews.llvm.org/D29827



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

[PATCH] D30295: [analyzer] clarify undef shift result when shift count is negative or exceeds the bit width

[Daniel Marjamäki via Phabricator via cfe-commits]

danielmarjamaki updated this revision to Diff 95740.
danielmarjamaki added a comment.

Fix review comments

- renamed
- reorder function arguments (CheckerContext last)


Repository:
  rL LLVM

https://reviews.llvm.org/D30295

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h
  lib/StaticAnalyzer/Checkers/ConversionChecker.cpp
  lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
  lib/StaticAnalyzer/Core/CheckerHelpers.cpp
  test/Analysis/bitwise-ops.c

Index: test/Analysis/bitwise-ops.c
===
--- test/Analysis/bitwise-ops.c
+++ test/Analysis/bitwise-ops.c
@@ -29,4 +29,18 @@
   default:
 return 0;
   }
-}
\ No newline at end of file
+}
+
+int testOverflowShift(int a) {
+  if (a == 323) {
+return 1 << a; // expected-warning{{The result of the '<<' expression is undefined due to shift count >= width of type}}
+  }
+  return 0;
+}
+
+int testNegativeShift(int a) {
+  if (a == -5) {
+return 1 << a; // expected-warning{{The result of the '<<' expression is undefined due to negative value on the right side of operand}}
+  }
+  return 0;
+}
Index: lib/StaticAnalyzer/Core/CheckerHelpers.cpp
===
--- lib/StaticAnalyzer/Core/CheckerHelpers.cpp
+++ lib/StaticAnalyzer/Core/CheckerHelpers.cpp
@@ -94,3 +94,41 @@
 
   return std::make_pair(VD, RHS);
 }
+
+bool clang::ento::exprComparesTo(SVal LHSVal, BinaryOperatorKind ComparisonOp,
+ SVal RHSVal, ProgramStateRef State) {
+
+  if (LHSVal.isUnknownOrUndef())
+return false;
+  ProgramStateManager &Mgr = State->getStateManager();
+  if (!LHSVal.getAs() && LHSVal.getAs()) {
+LHSVal = Mgr.getStoreManager().getBinding(State->getStore(),
+  LHSVal.castAs());
+  }
+  if (LHSVal.isUnknownOrUndef() || !LHSVal.getAs())
+return false;
+
+  SValBuilder &Bldr = Mgr.getSValBuilder();
+  SVal Eval = Bldr.evalBinOp(State, ComparisonOp, LHSVal, RHSVal,
+ Bldr.getConditionType());
+  if (Eval.isUnknownOrUndef())
+return false;
+  ConstraintManager &CM = State->getConstraintManager();
+  ProgramStateRef StTrue, StFalse;
+  std::tie(StTrue, StFalse) = CM.assumeDual(State, Eval.castAs());
+  return StTrue && !StFalse;
+}
+
+// Is E value greater or equal than Val?
+bool clang::ento::isGreaterOrEqual(const Expr *E, unsigned long long Val,
+   CheckerContext &C) {
+  DefinedSVal V =
+  C.getSValBuilder().makeIntVal(Val, C.getASTContext().LongLongTy);
+  return exprComparesTo(C.getSVal(E), BO_GE, V, C.getState());
+}
+
+// Is E value negative?
+bool clang::ento::isNegative(const Expr *E, CheckerContext &C) {
+  DefinedSVal V = C.getSValBuilder().makeIntVal(0, false);
+  return exprComparesTo(C.getSVal(E), BO_LT, V, C.getState());
+}
Index: lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
+++ lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
@@ -17,6 +17,7 @@
 #include "clang/StaticAnalyzer/Core/Checker.h"
 #include "clang/StaticAnalyzer/Core/CheckerManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/Support/raw_ostream.h"
@@ -59,6 +60,11 @@
   return StOutBound && !StInBound;
 }
 
+static bool isShiftOverflow(const BinaryOperator *B, CheckerContext &C) {
+  return isGreaterOrEqual(
+  B->getRHS(), C.getASTContext().getIntWidth(B->getLHS()->getType()), C);
+}
+
 void UndefResultChecker::checkPostStmt(const BinaryOperator *B,
CheckerContext &C) const {
   ProgramStateRef state = C.getState();
@@ -106,9 +112,24 @@
 }
 else {
   // Neither operand was undefined, but the result is undefined.
-  OS << "The result of the '"
- << BinaryOperator::getOpcodeStr(B->getOpcode())
- << "' expression is undefined";
+  if ((B->getOpcode() == BinaryOperatorKind::BO_Shl ||
+   B->getOpcode() == BinaryOperatorKind::BO_Shr) &&
+  isNegative(B->getRHS(), C)) {
+OS << "The result of the '"
+   << BinaryOperator::getOpcodeStr(B->getOpcode())
+   << "' expression is undefined due to negative value on the right "
+  "side of operand";
+  } else if ((B->getOpcode() == BinaryOperatorKind::BO_Shl ||
+  B->getOpcode() == BinaryOperatorKind::BO_Shr) &&
+ isShiftOverflow(B, C)) {
+OS << "The result of the '"
+   << BinaryOperator::getOpcodeStr(B->getOpcode())
+   << "' expression is undefined due to shift count >= width of type";
+  } else {
+OS << "The result of the '"
+   <

[PATCH] D32210: [Sema][ObjC] Add support for attribute "noescape"

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: include/clang/Basic/Attr.td:1222
+def NoEscape : InheritableAttr {
+  let Spellings = [GCC<"noescape">, CXX11<"clang", "noescape">];
+  let Subjects = SubjectList<[ParmVar], WarnDiag, "ExpectedParameter">;

I do not think GCC supports this attribute, so that should be using a GNU 
spelling rather than a GCC spelling.



Comment at: include/clang/Basic/Attr.td:1223
+  let Spellings = [GCC<"noescape">, CXX11<"clang", "noescape">];
+  let Subjects = SubjectList<[ParmVar], WarnDiag, "ExpectedParameter">;
+  let Documentation = [NoEscapeDocs];

No need to specify the last two arguments, they should be handled by default 
already.



Comment at: include/clang/Basic/AttrDocs.td:118
+  let Content = [{
+``noescape`` placed on a block parameter is used to inform the compiler that 
the block passed to a function cannot escape: that is, the block will not be 
invoked after the function returns. To ensure that the block does not escape, 
clang imposes the following restrictions on usage of non-escaping blocks:
+

Please wrap to 80 columns.

"block parameter" is a bit strange -- I assumed that meant parameter to a 
block, not a function parameter of block type. May want to clarify.

Should also clarify "the block will not be invoked after the function returns." 
Does this code produce undefined behavior?
```
  typedef void (^BlockTy)();
  void nonescapingFunc(__attribute__((noescape)) BlockTy);
  void escapingFunc(BlockTy);

  void callerFunc(__attribute__((noescape)) BlockTy block) {
nonescapingFunc(block); // OK
escapingFunc(block);// error: parameter is not annotated with noescape
  }

  void f(void) {
BlockTy Blk = ^{};
callerFunc(Blk);
Blk();
  }
```



Comment at: include/clang/Basic/DiagnosticSemaKinds.td:3094
+def err_noescape_passed_to_call : Error<
+  "cannot pass non-escaping parameter '%0' to function or method expecting 
escaping "
+  "argument">;

expecting an escaping argument

Also, someday we should make non-foo vs nonfoo a consistent decision in our 
diagnostics. We seem to use both forms.



Comment at: include/clang/Basic/DiagnosticSemaKinds.td:3106
+def note_noescape_parameter : Note<
+  "parameter is %select{annotated|not annotated}0 with noescape">;
+

Should quote `noescape`.



Comment at: lib/Sema/SemaChecking.cpp:2545
+   SourceLocation CallSiteLoc) {
+  ArrayRef Params = FDecl->parameters();
+

Space before the asterisk.



Comment at: lib/Sema/SemaChecking.cpp:2548
+  for (unsigned I = 0, E = Args.size(); I < E ; ++I) {
+const auto *CalleePD = I < Params.size() ? Params[I] : nullptr;
+

Please don't use `auto` here.



Comment at: lib/Sema/SemaChecking.cpp:2559
+if (const auto *DRE = dyn_cast(Args[I]->IgnoreImpCasts())) {
+  const auto *CallerPD = DRE->getDecl();
+  if (CallerPD->hasAttr()) {

Don't use `auto` here.



Comment at: lib/Sema/SemaChecking.cpp:2562
+S.Diag(Args[I]->getExprLoc(), diag::err_noescape_passed_to_call)
+<< CallerPD->getName();
+S.Diag(CallerPD->getLocation(), diag::note_noescape_parameter) << 0;

No need to use `getName()` here, you can just pass in the declaration directly. 
That means you should also remove the quotes from the diagnostic so it does not 
get double-quoted.



Comment at: lib/Sema/SemaChecking.cpp:2567
+else
+  assert(FDecl->isVariadic() &&
+ "Called function expected to be variadic");

What about functions that have no prototype (they're not variadic, but they 
accept any number of arguments)? Should include a test for this.



Comment at: lib/Sema/SemaDeclAttr.cpp:1520-1522
+  ParmVarDecl *PD = dyn_cast(D);
+  if (!PD)
+return;

Is there ever a case where this if statement will early return? I think you can 
just use `cast(D)->getType()` below.



Comment at: lib/Sema/SemaExpr.cpp:11180
+  if (const auto *DRE =
+  dyn_cast_or_null(RHS.get()->IgnoreImpCasts())) {
+const auto *D = DRE->getDecl();

Why `dyn_cast_or_null`? I didn't think `IgnoreImpCasts()` could return null.



Comment at: lib/Sema/SemaExpr.cpp:11181
+  dyn_cast_or_null(RHS.get()->IgnoreImpCasts())) {
+const auto *D = DRE->getDecl();
+if (D->hasAttr()) {

Don't use `auto`.



Comment at: lib/Sema/SemaExpr.cpp:11184
+  Diag(DRE->getLocation(), diag::err_noescape_assignment)
+  << D->getName();
+  Diag(D->getLocation(), diag::note_noescape_parameter) << 0;

Can drop the `getName()` cal

[PATCH] D29654: [OpenMP] Integrate OpenMP target region cubin into host binary

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

gtbercea updated this revision to Diff 95737.
gtbercea marked an inline comment as done.
gtbercea added a comment.

Avoid renaming by enabling PTXAS to generate an output file with the 
appropriate extension, in this case a cubin extension.


Repository:
  rL LLVM

https://reviews.llvm.org/D29654

Files:
  lib/Driver/Driver.cpp
  lib/Driver/ToolChains/CommonArgs.cpp
  lib/Driver/ToolChains/CommonArgs.h
  lib/Driver/ToolChains/Cuda.cpp
  lib/Driver/ToolChains/Cuda.h
  lib/Driver/ToolChains/Gnu.cpp
  test/Driver/openmp-offload.c

Index: test/Driver/openmp-offload.c
===
--- test/Driver/openmp-offload.c
+++ test/Driver/openmp-offload.c
@@ -590,6 +590,16 @@
 
 /// ###
 
+/// Check cubin file generation and usage by nvlink
+// RUN:   %clang -### -fopenmp=libomp -fopenmp-targets=nvptx64-nvidia-cuda -save-temps -no-canonical-prefixes %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHK-CUBIN %s
+
+// CHK-CUBIN: clang{{.*}}" "-o" "{{.*}}-openmp-nvptx64-nvidia-cuda.s"
+// CHK-CUBIN-NEXT: ptxas{{.*}}" "--output-file" "{{.*}}-openmp-nvptx64-nvidia-cuda.cubin" "{{.*}}-openmp-nvptx64-nvidia-cuda.s"
+// CHK-CUBIN-NEXT: nvlink" "-o" "{{.*}}-openmp-nvptx64-nvidia-cuda" {{.*}} "openmp-offload-openmp-nvptx64-nvidia-cuda.cubin"
+
+/// ###
+
 /// Check PTXAS is passed -c flag when offloading to an NVIDIA device using OpenMP.
 // RUN:   %clang -### -fopenmp=libomp -fopenmp-targets=nvptx64-nvidia-cuda -save-temps -no-canonical-prefixes %s 2>&1 \
 // RUN:   | FileCheck -check-prefix=CHK-PTXAS-DEFAULT %s
Index: lib/Driver/ToolChains/Gnu.cpp
===
--- lib/Driver/ToolChains/Gnu.cpp
+++ lib/Driver/ToolChains/Gnu.cpp
@@ -203,131 +203,6 @@
   // The types are (hopefully) good enough.
 }
 
-/// Add OpenMP linker script arguments at the end of the argument list so that
-/// the fat binary is built by embedding each of the device images into the
-/// host. The linker script also defines a few symbols required by the code
-/// generation so that the images can be easily retrieved at runtime by the
-/// offloading library. This should be used only in tool chains that support
-/// linker scripts.
-static void AddOpenMPLinkerScript(const ToolChain &TC, Compilation &C,
-  const InputInfo &Output,
-  const InputInfoList &Inputs,
-  const ArgList &Args, ArgStringList &CmdArgs,
-  const JobAction &JA) {
-
-  // If this is not an OpenMP host toolchain, we don't need to do anything.
-  if (!JA.isHostOffloading(Action::OFK_OpenMP))
-return;
-
-  // Create temporary linker script. Keep it if save-temps is enabled.
-  const char *LKS;
-  SmallString<256> Name = llvm::sys::path::filename(Output.getFilename());
-  if (C.getDriver().isSaveTempsEnabled()) {
-llvm::sys::path::replace_extension(Name, "lk");
-LKS = C.getArgs().MakeArgString(Name.c_str());
-  } else {
-llvm::sys::path::replace_extension(Name, "");
-Name = C.getDriver().GetTemporaryPath(Name, "lk");
-LKS = C.addTempFile(C.getArgs().MakeArgString(Name.c_str()));
-  }
-
-  // Add linker script option to the command.
-  CmdArgs.push_back("-T");
-  CmdArgs.push_back(LKS);
-
-  // Create a buffer to write the contents of the linker script.
-  std::string LksBuffer;
-  llvm::raw_string_ostream LksStream(LksBuffer);
-
-  // Get the OpenMP offload tool chains so that we can extract the triple
-  // associated with each device input.
-  auto OpenMPToolChains = C.getOffloadToolChains();
-  assert(OpenMPToolChains.first != OpenMPToolChains.second &&
- "No OpenMP toolchains??");
-
-  // Track the input file name and device triple in order to build the script,
-  // inserting binaries in the designated sections.
-  SmallVector, 8> InputBinaryInfo;
-
-  // Add commands to embed target binaries. We ensure that each section and
-  // image is 16-byte aligned. This is not mandatory, but increases the
-  // likelihood of data to be aligned with a cache block in several main host
-  // machines.
-  LksStream << "/*\n";
-  LksStream << "   OpenMP Offload Linker Script\n";
-  LksStream << " *** Automatically generated by Clang ***\n";
-  LksStream << "*/\n";
-  LksStream << "TARGET(binary)\n";
-  auto DTC = OpenMPToolChains.first;
-  for (auto &II : Inputs) {
-const Action *A = II.getAction();
-// Is this a device linking action?
-if (A && isa(A) &&
-A->isDeviceOffloading(Action::OFK_OpenMP)) {
-  assert(DTC != OpenMPToolChains.second &&
- "More device inputs than device toolchains??");
-  InputBinaryInfo.push_back(std::make_pair(
-  DTC->second->getTriple().normalize(), II.getFilename()));
-  ++DTC;
-  LksStream << "INPUT(" << II.getF

[PATCH] D30295: [analyzer] clarify undef shift result when shift count is negative or exceeds the bit width

[Daniel Marjamäki via Phabricator via cfe-commits]

danielmarjamaki marked 4 inline comments as done.
danielmarjamaki added inline comments.



Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h:46
 
-} // end GR namespace
+bool isExprResultConformsComparisonRule(CheckerContext &C,
+BinaryOperatorKind CompRule,

zaks.anna wrote:
> NoQ wrote:
> > Because you are making these functions public, i think it's worth it to 
> > make it obvious what they do without looking at the particular checker 
> > code. Comments are definitely worth it. I think function names could be 
> > worded better; i suggest `exprComparesTo(const Expr *LHSExpr, 
> > BinaryOperatorKind ComparisonOp, SVal RHSVal, CheckerContext &C);` and 
> > `isGreaterOrEqual(...)`; i'm personally fond of having CheckerContext at 
> > the back because that's where we have it in checker callbacks, but that's 
> > not important.
> + 1 on Artem's comment of making the names more clear and providing 
> documentation. Also, should these be part of CheckerContext?
> Also, should these be part of CheckerContext?

I chose not to. Then as NoQ suggested it's not possible to use this when 
CheckerContext is not available:

"The good thing about requiring only State is that we'd be able to use these 
functions in checker callbacks that don't provide the CheckerContext object, 
eg. checkEndAnalysis or checkRegionChanges."


Repository:
  rL LLVM

https://reviews.llvm.org/D30295



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

[PATCH] D32178: Delete unstable integration tests

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs added a comment.

I think you'd have to check for a maximum host SDK version, and then bump it 
upstream every time there's a new one thats known to work with the then-current 
trunk.

It can't be done based on language features because by definition, one cannot 
know what features are going to be added in the future.

All of that being said, we'd still have a situation where the test means 
different things on different host systems purely based on what's installed on 
that system (which is not a good thing IMO).


https://reviews.llvm.org/D32178



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

[PATCH] D30771: [analyzer] Teach the MallocChecker about Glib API for two arguments

[Daniel Marjamäki via Phabricator via cfe-commits]

danielmarjamaki requested changes to this revision.
danielmarjamaki added inline comments.
This revision now requires changes to proceed.



Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:788
 
+SVal MallocChecker::SValBinMulOp(CheckerContext &C, const Expr *Blocks,
+ const Expr *BlockBytes, ProgramStateRef 
State) {

I have the feeling this should be renamed. Since its purpose is to calculate 
the total size maybe MallocChecker::calculateBufferSize()



Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:790
+ const Expr *BlockBytes, ProgramStateRef 
State) {
+  SValBuilder &svalBuilder = C.getSValBuilder();
+  SVal nBlocks = C.getSVal(Blocks);

Please begin variables with Capitals. svalBuilder,nBlocks,nBlockBytes



Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:911
+  if (FunI == II_g_malloc0_n || FunI == II_g_try_malloc0_n) {
+SValBuilder &svalBuilder = C.getSValBuilder();
+Init = svalBuilder.makeZeroVal(svalBuilder.getContext().CharTy);

Capital. svalBuilder



Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2040
 
   const Expr *arg0Expr = CE->getArg(0);
   const LocationContext *LCtx = C.getLocationContext();

Capital variable: arg0Expr, arg0Val



Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2054
   const Expr *Arg1 = CE->getArg(1);
   if (!Arg1)
 return nullptr;

is this "if (!Arg1)" needed? It seems to me that if CE->getNumArgs() returns >= 
2 then CE->getArg(1) should be non-NULL. Does it crash/assert if you remove 
this condition?



Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2061
+const Expr *Arg2 = CE->getArg(2);
+if (!Arg2)
+  return nullptr;

hmm.. seems weird if CE->getArg(2) returns nullptr even though CE->getNumArgs() 
>= 3 .. is it possible to remove this check or does that cause some crash etc?

> Generally, i'd prefer the code for computing the buffer size to be structured 
> as

I would also prefer such code.



Repository:
  rL LLVM

https://reviews.llvm.org/D30771



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

[clang-tools-extra] r300699 - [clang-tidy] misc-misplaced-widening-cast: Disable checking of implicit widening casts by default.

[Gabor Horvath via cfe-commits]

Author: xazax
Date: Wed Apr 19 09:55:58 2017
New Revision: 300699

URL: http://llvm.org/viewvc/llvm-project?rev=300699&view=rev
Log:
[clang-tidy] misc-misplaced-widening-cast: Disable checking of implicit 
widening casts by default.

Patch by Ádám Balogh!

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

Added:

clang-tools-extra/trunk/test/clang-tidy/misc-misplaced-widening-cast-implicit-enabled.cpp
Removed:
clang-tools-extra/trunk/test/clang-tidy/misc-misplaced-widening-cast.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/misc/MisplacedWideningCastCheck.cpp

Modified: clang-tools-extra/trunk/clang-tidy/misc/MisplacedWideningCastCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/MisplacedWideningCastCheck.cpp?rev=300699&r1=300698&r2=300699&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/misc/MisplacedWideningCastCheck.cpp 
(original)
+++ clang-tools-extra/trunk/clang-tidy/misc/MisplacedWideningCastCheck.cpp Wed 
Apr 19 09:55:58 2017
@@ -21,7 +21,7 @@ namespace misc {
 MisplacedWideningCastCheck::MisplacedWideningCastCheck(
 StringRef Name, ClangTidyContext *Context)
 : ClangTidyCheck(Name, Context),
-  CheckImplicitCasts(Options.get("CheckImplicitCasts", true)) {}
+  CheckImplicitCasts(Options.get("CheckImplicitCasts", false)) {}
 
 void MisplacedWideningCastCheck::storeOptions(
 ClangTidyOptions::OptionMap &Opts) {

Added: 
clang-tools-extra/trunk/test/clang-tidy/misc-misplaced-widening-cast-implicit-enabled.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/misc-misplaced-widening-cast-implicit-enabled.cpp?rev=300699&view=auto
==
--- 
clang-tools-extra/trunk/test/clang-tidy/misc-misplaced-widening-cast-implicit-enabled.cpp
 (added)
+++ 
clang-tools-extra/trunk/test/clang-tidy/misc-misplaced-widening-cast-implicit-enabled.cpp
 Wed Apr 19 09:55:58 2017
@@ -0,0 +1,101 @@
+// RUN: %check_clang_tidy %s misc-misplaced-widening-cast %t -- 
-config="{CheckOptions: [{key: misc-misplaced-widening-cast.CheckImplicitCasts, 
value: 1}]}" --
+
+void func(long arg) {}
+
+void assign(int a, int b) {
+  long l;
+
+  l = a * b;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: either cast from 'int' to 'long' 
is ineffective, or there is loss of precision before the conversion 
[misc-misplaced-widening-cast]
+  l = (long)(a * b);
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: either cast from 'int' to 'long'
+  l = (long)a * b;
+
+  l = a << 8;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: either cast from 'int' to 'long'
+  l = (long)(a << 8);
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: either cast from 'int' to 'long'
+  l = (long)b << 8;
+
+  l = static_cast(a * b);
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: either cast from 'int' to 'long'
+}
+
+void compare(int a, int b, long c) {
+  bool l;
+
+  l = a * b == c;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: either cast from 'int' to 'long'
+  l = c == a * b;
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: either cast from 'int' to 'long'
+  l = (long)(a * b) == c;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: either cast from 'int' to 'long'
+  l = c == (long)(a * b);
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: either cast from 'int' to 'long'
+  l = (long)a * b == c;
+  l = c == (long)a * b;
+}
+
+void init(unsigned int n) {
+  long l1 = n << 8;
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: either cast from 'unsigned int' 
to 'long'
+  long l2 = (long)(n << 8);
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: either cast from 'unsigned int' 
to 'long'
+  long l3 = (long)n << 8;
+}
+
+void call(unsigned int n) {
+  func(n << 8);
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: either cast from 'unsigned int' 
to 'long'
+  func((long)(n << 8));
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: either cast from 'unsigned int' 
to 'long'
+  func((long)n << 8);
+}
+
+long ret(int a) {
+  if (a < 0) {
+return a * 1000;
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: either cast from 'int' to 
'long'
+  } else if (a > 0) {
+return (long)(a * 1000);
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: either cast from 'int' to 
'long'
+  } else {
+return (long)a * 1000;
+  }
+}
+
+void dontwarn1(unsigned char a, int i, unsigned char *p) {
+  long l;
+  // The result is a 9 bit value, there is no truncation in the implicit cast.
+  l = (long)(a + 15);
+  // The result is a 12 bit value, there is no truncation in the implicit cast.
+  l = (long)(a << 4);
+  // The result is a 3 bit value, there is no truncation in the implicit cast.
+  l = (long)((i % 5) + 1);
+  // The result is a 16 bit value, there is no truncation in the implicit cast.
+  l = (long)(((*p) << 8) + *(p + 1));
+}
+
+template  struct DontWarn2 {
+  void assign(T a, T b) {
+long l;
+l = (long)(a * b);
+  }
+};
+DontW

[PATCH] D32164: [clang-tidy] misc-misplaced-widening-cast: Disable checking of implicit widening casts by default

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL300699: [clang-tidy] misc-misplaced-widening-cast: Disable 
checking of implicit… (authored by xazax).

Changed prior to commit:
  https://reviews.llvm.org/D32164?vs=95563&id=95751#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D32164

Files:
  clang-tools-extra/trunk/clang-tidy/misc/MisplacedWideningCastCheck.cpp
  
clang-tools-extra/trunk/test/clang-tidy/misc-misplaced-widening-cast-implicit-enabled.cpp
  clang-tools-extra/trunk/test/clang-tidy/misc-misplaced-widening-cast.cpp

Index: clang-tools-extra/trunk/clang-tidy/misc/MisplacedWideningCastCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/misc/MisplacedWideningCastCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/misc/MisplacedWideningCastCheck.cpp
@@ -21,7 +21,7 @@
 MisplacedWideningCastCheck::MisplacedWideningCastCheck(
 StringRef Name, ClangTidyContext *Context)
 : ClangTidyCheck(Name, Context),
-  CheckImplicitCasts(Options.get("CheckImplicitCasts", true)) {}
+  CheckImplicitCasts(Options.get("CheckImplicitCasts", false)) {}
 
 void MisplacedWideningCastCheck::storeOptions(
 ClangTidyOptions::OptionMap &Opts) {
Index: clang-tools-extra/trunk/test/clang-tidy/misc-misplaced-widening-cast-implicit-enabled.cpp
===
--- clang-tools-extra/trunk/test/clang-tidy/misc-misplaced-widening-cast-implicit-enabled.cpp
+++ clang-tools-extra/trunk/test/clang-tidy/misc-misplaced-widening-cast-implicit-enabled.cpp
@@ -0,0 +1,101 @@
+// RUN: %check_clang_tidy %s misc-misplaced-widening-cast %t -- -config="{CheckOptions: [{key: misc-misplaced-widening-cast.CheckImplicitCasts, value: 1}]}" --
+
+void func(long arg) {}
+
+void assign(int a, int b) {
+  long l;
+
+  l = a * b;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: either cast from 'int' to 'long' is ineffective, or there is loss of precision before the conversion [misc-misplaced-widening-cast]
+  l = (long)(a * b);
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: either cast from 'int' to 'long'
+  l = (long)a * b;
+
+  l = a << 8;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: either cast from 'int' to 'long'
+  l = (long)(a << 8);
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: either cast from 'int' to 'long'
+  l = (long)b << 8;
+
+  l = static_cast(a * b);
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: either cast from 'int' to 'long'
+}
+
+void compare(int a, int b, long c) {
+  bool l;
+
+  l = a * b == c;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: either cast from 'int' to 'long'
+  l = c == a * b;
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: either cast from 'int' to 'long'
+  l = (long)(a * b) == c;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: either cast from 'int' to 'long'
+  l = c == (long)(a * b);
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: either cast from 'int' to 'long'
+  l = (long)a * b == c;
+  l = c == (long)a * b;
+}
+
+void init(unsigned int n) {
+  long l1 = n << 8;
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: either cast from 'unsigned int' to 'long'
+  long l2 = (long)(n << 8);
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: either cast from 'unsigned int' to 'long'
+  long l3 = (long)n << 8;
+}
+
+void call(unsigned int n) {
+  func(n << 8);
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: either cast from 'unsigned int' to 'long'
+  func((long)(n << 8));
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: either cast from 'unsigned int' to 'long'
+  func((long)n << 8);
+}
+
+long ret(int a) {
+  if (a < 0) {
+return a * 1000;
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: either cast from 'int' to 'long'
+  } else if (a > 0) {
+return (long)(a * 1000);
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: either cast from 'int' to 'long'
+  } else {
+return (long)a * 1000;
+  }
+}
+
+void dontwarn1(unsigned char a, int i, unsigned char *p) {
+  long l;
+  // The result is a 9 bit value, there is no truncation in the implicit cast.
+  l = (long)(a + 15);
+  // The result is a 12 bit value, there is no truncation in the implicit cast.
+  l = (long)(a << 4);
+  // The result is a 3 bit value, there is no truncation in the implicit cast.
+  l = (long)((i % 5) + 1);
+  // The result is a 16 bit value, there is no truncation in the implicit cast.
+  l = (long)(((*p) << 8) + *(p + 1));
+}
+
+template  struct DontWarn2 {
+  void assign(T a, T b) {
+long l;
+l = (long)(a * b);
+  }
+};
+DontWarn2 DW2;
+
+// Cast is not suspicious when casting macro.
+#define A  (X<<2)
+long macro1(int X) {
+  return (long)A;
+}
+
+// Don't warn about cast in macro.
+#define B(X,Y)   (long)(X*Y)
+long macro2(int x, int y) {
+  return B(x,y);
+}
+
+void floatingpoint(float a, float b) {
+  double d = (double)(a * b); // Currently we don't warn for this.
+}
Index: clang-tools-extra/trunk/test/clang-tidy/misc-misplaced-widening-cast.cpp
==

[PATCH] D32176: Add #pragma clang attribute support for the external_source_symbol attribute

[Alex Lorenz via Phabricator via cfe-commits]

arphaman updated this revision to Diff 95754.
arphaman marked 4 inline comments as done.
arphaman added a comment.

Addressed Aaron's comments.


Repository:
  rL LLVM

https://reviews.llvm.org/D32176

Files:
  include/clang/Basic/Attr.td
  lib/Sema/SemaDeclAttr.cpp
  test/Misc/pragma-attribute-supported-attributes-list.test
  test/Sema/alloc-align-attr.c
  utils/TableGen/ClangAttrEmitter.cpp

Index: utils/TableGen/ClangAttrEmitter.cpp
===
--- utils/TableGen/ClangAttrEmitter.cpp
+++ utils/TableGen/ClangAttrEmitter.cpp
@@ -1611,7 +1611,36 @@
 
 struct PragmaClangAttributeSupport {
   std::vector Rules;
-  llvm::DenseMap SubjectsToRules;
+
+  class RuleOrAggregateRuleSet {
+std::vector Rules;
+bool IsRule;
+RuleOrAggregateRuleSet(ArrayRef Rules,
+   bool IsRule)
+: Rules(Rules), IsRule(IsRule) {}
+
+  public:
+bool isRule() const { return IsRule; }
+
+const AttributeSubjectMatchRule &getRule() const {
+  assert(IsRule && "not a rule!");
+  return Rules[0];
+}
+
+ArrayRef getAggregateRuleSet() const {
+  return Rules;
+}
+
+static RuleOrAggregateRuleSet
+getRule(const AttributeSubjectMatchRule &Rule) {
+  return RuleOrAggregateRuleSet(Rule, /*IsRule=*/true);
+}
+static RuleOrAggregateRuleSet
+getAggregateRuleSet(ArrayRef Rules) {
+  return RuleOrAggregateRuleSet(Rules, /*IsRule=*/false);
+}
+  };
+  llvm::DenseMap SubjectsToRules;
 
   PragmaClangAttributeSupport(RecordKeeper &Records);
 
@@ -1626,6 +1655,15 @@
 
 } // end anonymous namespace
 
+static bool doesDeclDeriveFrom(const Record *D, const Record *Base) {
+  const Record *CurrentBase = D->getValueAsDef("Base");
+  if (!CurrentBase)
+return false;
+  if (CurrentBase == Base)
+return true;
+  return doesDeclDeriveFrom(CurrentBase, Base);
+}
+
 PragmaClangAttributeSupport::PragmaClangAttributeSupport(
 RecordKeeper &Records) {
   std::vector MetaSubjects =
@@ -1638,7 +1676,11 @@
 SubjectContainer->getValueAsListOfDefs("Subjects");
 for (const auto *Subject : ApplicableSubjects) {
   bool Inserted =
-  SubjectsToRules.try_emplace(Subject, MetaSubject, Constraint).second;
+  SubjectsToRules
+  .try_emplace(Subject, RuleOrAggregateRuleSet::getRule(
+AttributeSubjectMatchRule(MetaSubject,
+  Constraint)))
+  .second;
   if (!Inserted) {
 PrintFatalError("Attribute subject match rules should not represent"
 "same attribute subjects.");
@@ -1652,6 +1694,37 @@
 for (const auto *Constraint : Constraints)
   MapFromSubjectsToRules(Constraint, MetaSubject, Constraint);
   }
+
+  std::vector Aggregates =
+  Records.getAllDerivedDefinitions("AttrSubjectMatcherAggregateRule");
+  std::vector DeclNodes = Records.getAllDerivedDefinitions("DDecl");
+  for (const auto *Aggregate : Aggregates) {
+Record *SubjectDecl = Aggregate->getValueAsDef("Subject");
+
+// Gather sub-classes of the aggregate subject that act as attribute
+// subject rules.
+std::vector Rules;
+for (const auto *D : DeclNodes) {
+  if (doesDeclDeriveFrom(D, SubjectDecl)) {
+auto It = SubjectsToRules.find(D);
+if (It == SubjectsToRules.end())
+  continue;
+if (!It->second.isRule() || It->second.getRule().isSubRule())
+  continue; // Assume that the rule will be included as well.
+Rules.push_back(It->second.getRule());
+  }
+}
+
+bool Inserted =
+SubjectsToRules
+.try_emplace(SubjectDecl,
+ RuleOrAggregateRuleSet::getAggregateRuleSet(Rules))
+.second;
+if (!Inserted) {
+  PrintFatalError("Attribute subject match rules should not represent"
+  "same attribute subjects.");
+}
+  }
 }
 
 static PragmaClangAttributeSupport &
@@ -1738,24 +1811,25 @@
 auto It = SubjectsToRules.find(Subject);
 assert(It != SubjectsToRules.end() &&
"This attribute is unsupported by #pragma clang attribute");
-AttributeSubjectMatchRule Rule = It->getSecond();
-// The rule might be language specific, so only subtract it from the given
-// rules if the specific language options are specified.
-std::vector LangOpts = Rule.getLangOpts();
-SS << "  MatchRules.push_back(std::make_pair(" << Rule.getEnumValue()
-   << ", /*IsSupported=*/";
-if (!LangOpts.empty()) {
-  for (auto I = LangOpts.begin(), E = LangOpts.end(); I != E; ++I) {
-std::string Part = (*I)->getValueAsString("Name");
-if ((*I)->getValueAsBit("Negated"))
-  SS << "!";
-SS << "LangOpts." + Part;
-if (I + 1 != E)
-  SS << " || ";
-  }
-} else
-  SS << "true";
-SS << "));\n";
+for (const auto &Rule : It

[PATCH] D32176: Add #pragma clang attribute support for the external_source_symbol attribute

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added inline comments.



Comment at: lib/Sema/SemaDeclAttr.cpp:5773
+  // Check whether the attribute appertains to the given subject.
+  if (!Attr.diagnoseAppertainsTo(S, D))
+return true;

I hoisted it after `diagnoseLangOpts` to keep some tests happy.


Repository:
  rL LLVM

https://reviews.llvm.org/D32176



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

r300703 - Remove unnecessary condition as suggested by clang-tidy. NFC

[Gabor Horvath via cfe-commits]

Author: xazax
Date: Wed Apr 19 10:11:10 2017
New Revision: 300703

URL: http://llvm.org/viewvc/llvm-project?rev=300703&view=rev
Log:
Remove unnecessary condition as suggested by clang-tidy. NFC

Patch by: Gergely Angeli!

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

Modified:
cfe/trunk/lib/AST/ASTContext.cpp

Modified: cfe/trunk/lib/AST/ASTContext.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTContext.cpp?rev=300703&r1=300702&r2=300703&view=diff
==
--- cfe/trunk/lib/AST/ASTContext.cpp (original)
+++ cfe/trunk/lib/AST/ASTContext.cpp Wed Apr 19 10:11:10 2017
@@ -9414,10 +9414,8 @@ createDynTypedNode(const NestedNameSpeci
   if (!NodeOrVector.template is()) {
 auto *Vector = new ASTContext::ParentVector(
 1, getSingleDynTypedNodeFromParentMap(NodeOrVector));
-if (auto *Node =
-NodeOrVector
-.template dyn_cast())
-  delete Node;
+delete NodeOrVector
+.template dyn_cast();
 NodeOrVector = Vector;
   }
 


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

[PATCH] D31885: Remove TBAA information from LValues representing union members

[Daniel Berlin via Phabricator via cfe-commits]

dberlin added a comment.

In https://reviews.llvm.org/D31885#730072, @rjmccall wrote:

> Thanks for CC'ing me.  There are two problems here.
>
> The second is that our alias-analysis philosophy says that the fact that two 
> accesses "sufficiently obviously" can alias should always override TBAA.


I'm not sure i agree.
We have made explicit exceptions for parts of the standard that are nonsensical 
(and they've been slowly updated in newer standards to be less nonsensical).  
Outside of that, i don't believe that is our philosophy, nor would i believe it 
should be.
There is no "do what i mean" here we can define that makes sense.

>   I have a hard time seeing what about the test cases I'm seeing makes it 
> insufficiently obvious that the accesses alias.  The  accesses are ultimately 
> derived from the same pointer; either the analysis is capable of counting to 
> 32, in which case it should see that they definitely alias, or it is not, in 
> which can it needs to assume that they might.  Perhaps the existing AA 
> interface is just inadequate?  It seems to me that it can't express that 
> middle-ground of "I have a concrete reason to think that these pointers are 
> related but cannot answer for certain whether they overlap".  Because if it 
> could just answer that rather than having to downgrade all the way to 
> MayAlias, it seems clear to me that (1) most memory transforms would treat it 
> exactly the same as MayAlias but (2) it should be considered strong enough to 
> suppress TBAA.

Just to note: Even assuming this wasn't crazytown, we certainly can't do it in  
the backend :)

The answer in such a case is to disable TBAA on those accesses and not emit it.
That is the direction we are moving, because trying to implement any rule about 
what "sufficiently obvious" means in the backend is a losing game.
It has no idea what it is doing, nor can it.
TBAA in the backend is a misnomer.
It is just walking a tree of hierarchical sets.   It has literally no idea what 
any of those sets mean, nor should it.

> John.




Repository:
  rL LLVM

https://reviews.llvm.org/D31885



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

[PATCH] D32210: [Sema][ObjC] Add support for attribute "noescape"

[Jonathan Roelofs via Phabricator via cfe-commits]

jroelofs added a comment.

This doesn't forbid assigning them to block properties... is that intentional?

  typedef void (^Block)(int);
  
  @interface Foo
  @property Block B;
  @end
  
  void foo(Foo *f, Block __attribute__((noescape)) b) {
f.B = b;
  }




Comment at: include/clang/Basic/AttrDocs.td:123
+* Cannot be captured by a block
+* Cannot be assigned to a variable
+

Is it helpful to forbid taking the address of them, too?


https://reviews.llvm.org/D32210



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

[PATCH] D32176: Add #pragma clang attribute support for the external_source_symbol attribute

[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:
  rL LLVM

https://reviews.llvm.org/D32176



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

[PATCH] D32231: [CMake] Enable ARM target in Fuchsia toolchain

[Petr Hosek via Phabricator via cfe-commits]

phosek created this revision.
Herald added subscribers: mgorny, rengolin, aemerson.

This is still used by some users of Fuchsia toolchain. Also include llc and opt 
which is useful for development and testing.


Repository:
  rL LLVM

https://reviews.llvm.org/D32231

Files:
  cmake/caches/Fuchsia-stage2.cmake


Index: cmake/caches/Fuchsia-stage2.cmake
===
--- cmake/caches/Fuchsia-stage2.cmake
+++ cmake/caches/Fuchsia-stage2.cmake
@@ -1,7 +1,7 @@
 # This file sets up a CMakeCache for the second stage of a Fuchsia toolchain
 # build.
 
-set(LLVM_TARGETS_TO_BUILD X86;AArch64 CACHE STRING "")
+set(LLVM_TARGETS_TO_BUILD X86;ARM;AArch64 CACHE STRING "")
 
 set(PACKAGE_VENDOR Fuchsia CACHE STRING "")
 
@@ -49,6 +49,8 @@
   llvm-readobj
   llvm-size
   llvm-symbolizer
+  llc
+  opt
   CACHE STRING "")
 
 set(LLVM_DISTRIBUTION_COMPONENTS


Index: cmake/caches/Fuchsia-stage2.cmake
===
--- cmake/caches/Fuchsia-stage2.cmake
+++ cmake/caches/Fuchsia-stage2.cmake
@@ -1,7 +1,7 @@
 # This file sets up a CMakeCache for the second stage of a Fuchsia toolchain
 # build.
 
-set(LLVM_TARGETS_TO_BUILD X86;AArch64 CACHE STRING "")
+set(LLVM_TARGETS_TO_BUILD X86;ARM;AArch64 CACHE STRING "")
 
 set(PACKAGE_VENDOR Fuchsia CACHE STRING "")
 
@@ -49,6 +49,8 @@
   llvm-readobj
   llvm-size
   llvm-symbolizer
+  llc
+  opt
   CACHE STRING "")
 
 set(LLVM_DISTRIBUTION_COMPONENTS
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32231: [CMake] Enable ARM target in Fuchsia toolchain

[Petr Hosek via Phabricator via cfe-commits]

phosek updated this revision to Diff 95766.

Repository:
  rL LLVM

https://reviews.llvm.org/D32231

Files:
  cmake/caches/Fuchsia-stage2.cmake


Index: cmake/caches/Fuchsia-stage2.cmake
===
--- cmake/caches/Fuchsia-stage2.cmake
+++ cmake/caches/Fuchsia-stage2.cmake
@@ -1,7 +1,7 @@
 # This file sets up a CMakeCache for the second stage of a Fuchsia toolchain
 # build.
 
-set(LLVM_TARGETS_TO_BUILD X86;AArch64 CACHE STRING "")
+set(LLVM_TARGETS_TO_BUILD X86;ARM;AArch64 CACHE STRING "")
 
 set(PACKAGE_VENDOR Fuchsia CACHE STRING "")
 
@@ -36,6 +36,7 @@
 # Setup toolchain.
 set(LLVM_INSTALL_TOOLCHAIN_ONLY ON CACHE BOOL "")
 set(LLVM_TOOLCHAIN_TOOLS
+  llc
   llvm-ar
   llvm-cov
   llvm-cxxfilt
@@ -49,6 +50,7 @@
   llvm-readobj
   llvm-size
   llvm-symbolizer
+  opt
   CACHE STRING "")
 
 set(LLVM_DISTRIBUTION_COMPONENTS


Index: cmake/caches/Fuchsia-stage2.cmake
===
--- cmake/caches/Fuchsia-stage2.cmake
+++ cmake/caches/Fuchsia-stage2.cmake
@@ -1,7 +1,7 @@
 # This file sets up a CMakeCache for the second stage of a Fuchsia toolchain
 # build.
 
-set(LLVM_TARGETS_TO_BUILD X86;AArch64 CACHE STRING "")
+set(LLVM_TARGETS_TO_BUILD X86;ARM;AArch64 CACHE STRING "")
 
 set(PACKAGE_VENDOR Fuchsia CACHE STRING "")
 
@@ -36,6 +36,7 @@
 # Setup toolchain.
 set(LLVM_INSTALL_TOOLCHAIN_ONLY ON CACHE BOOL "")
 set(LLVM_TOOLCHAIN_TOOLS
+  llc
   llvm-ar
   llvm-cov
   llvm-cxxfilt
@@ -49,6 +50,7 @@
   llvm-readobj
   llvm-size
   llvm-symbolizer
+  opt
   CACHE STRING "")
 
 set(LLVM_DISTRIBUTION_COMPONENTS
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32176: Add #pragma clang attribute support for the external_source_symbol attribute

[Alex Lorenz via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL300712: Add #pragma clang attribute support to the 
external_source_symbol attribute (authored by arphaman).

Changed prior to commit:
  https://reviews.llvm.org/D32176?vs=95754&id=95767#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D32176

Files:
  cfe/trunk/include/clang/Basic/Attr.td
  cfe/trunk/lib/Sema/SemaDeclAttr.cpp
  cfe/trunk/test/Misc/pragma-attribute-supported-attributes-list.test
  cfe/trunk/test/Sema/alloc-align-attr.c
  cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp

Index: cfe/trunk/include/clang/Basic/Attr.td
===
--- cfe/trunk/include/clang/Basic/Attr.td
+++ cfe/trunk/include/clang/Basic/Attr.td
@@ -368,6 +368,16 @@
   let LangOpts = [BlocksSupported];
 }
 
+// Aggregate attribute subject match rules are abstract match rules that can't
+// be used directly in #pragma clang attribute. Instead, users have to use
+// subject match rules that correspond to attribute subjects that derive from
+// the specified subject.
+class AttrSubjectMatcherAggregateRule {
+  AttrSubject Subject = subject;
+}
+
+def SubjectMatcherForNamed : AttrSubjectMatcherAggregateRule;
+
 class Attr {
   // The various ways in which an attribute can be spelled in source
   list Spellings;
@@ -656,7 +666,7 @@
   StringArgument<"definedIn", 1>,
   BoolArgument<"generatedDeclaration", 1>];
   let HasCustomParsing = 1;
-//  let Subjects = SubjectList<[Named]>;
+  let Subjects = SubjectList<[Named]>;
   let Documentation = [ExternalSourceSymbolDocs];
 }
 
Index: cfe/trunk/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- cfe/trunk/test/Misc/pragma-attribute-supported-attributes-list.test
+++ cfe/trunk/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -2,7 +2,7 @@
 
 // The number of supported attributes should never go down!
 
-// CHECK: #pragma clang attribute supports 57 attributes:
+// CHECK: #pragma clang attribute supports 58 attributes:
 // CHECK-NEXT: AMDGPUFlatWorkGroupSize (SubjectMatchRule_function)
 // CHECK-NEXT: AMDGPUNumSGPR (SubjectMatchRule_function)
 // CHECK-NEXT: AMDGPUNumVGPR (SubjectMatchRule_function)
@@ -23,6 +23,7 @@
 // CHECK-NEXT: DisableTailCalls (SubjectMatchRule_function, SubjectMatchRule_objc_method)
 // CHECK-NEXT: EnableIf (SubjectMatchRule_function)
 // CHECK-NEXT: EnumExtensibility (SubjectMatchRule_enum)
+// CHECK-NEXT: ExternalSourceSymbol ((SubjectMatchRule_record, SubjectMatchRule_enum, SubjectMatchRule_enum_constant, SubjectMatchRule_field, SubjectMatchRule_function, SubjectMatchRule_namespace, SubjectMatchRule_objc_category, SubjectMatchRule_objc_interface, SubjectMatchRule_objc_method, SubjectMatchRule_objc_property, SubjectMatchRule_objc_protocol, SubjectMatchRule_record, SubjectMatchRule_type_alias, SubjectMatchRule_variable))
 // CHECK-NEXT: FlagEnum (SubjectMatchRule_enum)
 // CHECK-NEXT: Flatten (SubjectMatchRule_function)
 // CHECK-NEXT: IFunc (SubjectMatchRule_function)
Index: cfe/trunk/test/Sema/alloc-align-attr.c
===
--- cfe/trunk/test/Sema/alloc-align-attr.c
+++ cfe/trunk/test/Sema/alloc-align-attr.c
@@ -13,7 +13,7 @@
 void *test_bad_param_type(void) __attribute((alloc_align(1.1))); // expected-error {{'alloc_align' attribute requires parameter 1 to be an integer constant}}
 
 // argument count
-void *test_no_fn_proto() __attribute__((alloc_align)); // expected-error {{'alloc_align' attribute takes one argument}}
-void *test_no_fn_proto() __attribute__((alloc_align())); // expected-error {{'alloc_align' attribute takes one argument}}
-void *test_no_fn_proto() __attribute__((alloc_align(32, 45, 37))); // expected-error {{'alloc_align' attribute takes one argument}}
+void *test_no_fn_proto(int x, int y) __attribute__((alloc_align)); // expected-error {{'alloc_align' attribute takes one argument}}
+void *test_no_fn_proto(int x, int y) __attribute__((alloc_align())); // expected-error {{'alloc_align' attribute takes one argument}}
+void *test_no_fn_proto(int x, int y) __attribute__((alloc_align(32, 45, 37))); // expected-error {{'alloc_align' attribute takes one argument}}
 
Index: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
===
--- cfe/trunk/lib/Sema/SemaDeclAttr.cpp
+++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp
@@ -2511,12 +2511,6 @@
   assert(checkAttributeAtMostNumArgs(S, Attr, 3) &&
  "Invalid number of arguments in an external_source_symbol attribute");
 
-  if (!isa(D)) {
-S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type)
-<< Attr.getName() << ExpectedNamedDecl;
-return;
-  }
-
   StringRef Language;
   if (const auto *SE = dyn_cast_or_null(Attr.getArgAsExpr(0)))
 Language = SE->getString();
@@ -5765,18 +5759,21 @@
 static bool handleCommonAttributeFe

r300712 - Add #pragma clang attribute support to the external_source_symbol attribute

[Alex Lorenz via cfe-commits]

Author: arphaman
Date: Wed Apr 19 10:52:11 2017
New Revision: 300712

URL: http://llvm.org/viewvc/llvm-project?rev=300712&view=rev
Log:
Add #pragma clang attribute support to the external_source_symbol attribute

Prior to this commit the external_source_symbol attribute wasn't supported by
#pragma clang attribute for the following two reasons:

- The Named attribute subject hasn't been supported by TableGen.
- There was no way to specify a subject match rule for #pragma clang attribute
 that could operate on a set of attribute subjects (e.g. the ones that derive
 from NamedDecl).

This commit fixes the two issues and thus adds external_source_symbol support to
#pragma clang attribute.

rdar://31169028

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

Modified:
cfe/trunk/include/clang/Basic/Attr.td
cfe/trunk/lib/Sema/SemaDeclAttr.cpp
cfe/trunk/test/Misc/pragma-attribute-supported-attributes-list.test
cfe/trunk/test/Sema/alloc-align-attr.c
cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp

Modified: cfe/trunk/include/clang/Basic/Attr.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=300712&r1=300711&r2=300712&view=diff
==
--- cfe/trunk/include/clang/Basic/Attr.td (original)
+++ cfe/trunk/include/clang/Basic/Attr.td Wed Apr 19 10:52:11 2017
@@ -368,6 +368,16 @@ def SubjectMatcherForBlock : AttrSubject
   let LangOpts = [BlocksSupported];
 }
 
+// Aggregate attribute subject match rules are abstract match rules that can't
+// be used directly in #pragma clang attribute. Instead, users have to use
+// subject match rules that correspond to attribute subjects that derive from
+// the specified subject.
+class AttrSubjectMatcherAggregateRule {
+  AttrSubject Subject = subject;
+}
+
+def SubjectMatcherForNamed : AttrSubjectMatcherAggregateRule;
+
 class Attr {
   // The various ways in which an attribute can be spelled in source
   list Spellings;
@@ -656,7 +666,7 @@ def ExternalSourceSymbol : InheritableAt
   StringArgument<"definedIn", 1>,
   BoolArgument<"generatedDeclaration", 1>];
   let HasCustomParsing = 1;
-//  let Subjects = SubjectList<[Named]>;
+  let Subjects = SubjectList<[Named]>;
   let Documentation = [ExternalSourceSymbolDocs];
 }
 

Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=300712&r1=300711&r2=300712&view=diff
==
--- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Wed Apr 19 10:52:11 2017
@@ -2511,12 +2511,6 @@ static void handleExternalSourceSymbolAt
   assert(checkAttributeAtMostNumArgs(S, Attr, 3) &&
  "Invalid number of arguments in an external_source_symbol attribute");
 
-  if (!isa(D)) {
-S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type)
-<< Attr.getName() << ExpectedNamedDecl;
-return;
-  }
-
   StringRef Language;
   if (const auto *SE = dyn_cast_or_null(Attr.getArgAsExpr(0)))
 Language = SE->getString();
@@ -5765,18 +5759,21 @@ static void handleOpenCLNoSVMAttr(Sema &
 static bool handleCommonAttributeFeatures(Sema &S, Scope *scope, Decl *D,
   const AttributeList &Attr) {
   // Several attributes carry different semantics than the parsing requires, so
-  // those are opted out of the common handling.
+  // those are opted out of the common argument checks.
   //
   // We also bail on unknown and ignored attributes because those are handled
   // as part of the target-specific handling logic.
-  if (Attr.hasCustomParsing() ||
-  Attr.getKind() == AttributeList::UnknownAttribute)
+  if (Attr.getKind() == AttributeList::UnknownAttribute)
 return false;
-
   // Check whether the attribute requires specific language extensions to be
   // enabled.
   if (!Attr.diagnoseLangOpts(S))
 return true;
+  // Check whether the attribute appertains to the given subject.
+  if (!Attr.diagnoseAppertainsTo(S, D))
+return true;
+  if (Attr.hasCustomParsing())
+return false;
 
   if (Attr.getMinArgs() == Attr.getMaxArgs()) {
 // If there are no optional arguments, then checking for the argument count
@@ -5793,10 +5790,6 @@ static bool handleCommonAttributeFeature
   return true;
   }
 
-  // Check whether the attribute appertains to the given subject.
-  if (!Attr.diagnoseAppertainsTo(S, D))
-return true;
-
   return false;
 }
 

Modified: cfe/trunk/test/Misc/pragma-attribute-supported-attributes-list.test
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Misc/pragma-attribute-supported-attributes-list.test?rev=300712&r1=300711&r2=300712&view=diff
==
--- cfe/trunk/test/Misc/pragma-attribute-supported-attributes-list.test 
(original)
+++ cfe/trunk/test/Misc/pragm

[PATCH] D32210: [Sema][ObjC] Add support for attribute "noescape"

[Adam Kemp via Phabricator via cfe-commits]

TheRealAdamKemp added inline comments.



Comment at: include/clang/Basic/AttrDocs.td:122
+* Cannot be returned from a function
+* Cannot be captured by a block
+* Cannot be assigned to a variable

This may be too restrictive in some cases. Consider this:

```
typedef void (^BlockTy)();
void nonescapingFunc(__attribute__((noescape)) BlockTy);

void callerFunc(__attribute__((noescape)) BlockTy block) {
  nonescapingFunc(^{ block(); });
}
```

It sounds like the above code would give an error, but the capturing block is 
also nonescaping, which means it should be allowed. This is a useful pattern, 
and disallowing it would make these attributes very cumbersome.

The code could also be written like this:

```
typedef void (^BlockTy)();
void nonescapingFunc(__attribute__((noescape)) BlockTy);

void callerFunc(__attribute__((noescape)) BlockTy block) {
  BlockTy wrapBlock = ^{
block();
  };
  nonescapingFunc(wrapBlock);
}
```

Again, I would expect that to not give an error or at least be able to decorate 
the declaration of `wrapBlock` to indicate that it is also nonescaping (can 
this attribute be applied to locals or only function arguments?).


https://reviews.llvm.org/D32210



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

[PATCH] D32207: Corrrect warn_unused_result attribute

[Erich Keane via Phabricator via cfe-commits]

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



Comment at: lib/AST/Decl.cpp:3010
+(OpCode == OO_PlusPlus || OpCode == OO_MinusMinus) &&
+(this->getNumParams() + (isa(this) ? 1 : 0)) == 2;
+if (Ret && !IsPostfix) {

aaron.ballman wrote:
> CXXMethodDecl represents a static or instance method, so I'm not certain this 
> logic is quite correct.
I actually stole this logic from lib/Sema/SemaDeclCXX.cpp lines 12708 and 
12754.  I believe the logic here is that it operators aren't allowed to be 
static?  Since it is illegal to do so,  I would expect that I'd simply need to 
do an assert to prevent it?

I'm open for ideas if you have one here, I'm not sure what the 'right' thing to 
do is.



Comment at: test/SemaCXX/warn-unused-result.cpp:166
+// are special-cased to not warn for return-type due to ubiquitousness.
+struct[[clang::warn_unused_result]] S {
+  S DoThing() { return {}; };

aaron.ballman wrote:
> Can you also add a test with [[nodiscard]] and verify that the uses of 
> increment/decrement *are* diagnosed?
I can add that, it will obviously cause a slight change in logic since I didn't 
handle that separately.  Are we surewe WANT [[nodiscard]] to warn in this case?


https://reviews.llvm.org/D32207



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

[PATCH] D32207: Corrrect warn_unused_result attribute

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

1 more thing:  I excepted post-increment/decrement from this warning because I 
figured it would give a TON of warnings from people using post-inc/dec in for 
loops.

However, after further discussion with Craig (who brought this up on the 
mailing list), I severely wonder if that is the 'right' thing to do.  Someone 
who is going to mark their struct in this way would likely ALSO mean it in this 
case, right?

Aaron: Should I simply remove the special casing all together, so that it warns 
on post inc/dec?  That would definitely solve all your concerns in the review 
so far.


https://reviews.llvm.org/D32207



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

[PATCH] D32199: [TBAASan] A TBAA Sanitizer (Clang)

[Filipe Cabecinhas via Phabricator via cfe-commits]

filcab added a comment.

Missing a `sanitizer-ld.c` test for freebsd.




Comment at: include/clang/Basic/Attr.td:1849
+   GNU<"no_sanitize_memory">,
+   GNU<"no_sanitize_tbaa">];
   let Subjects = SubjectList<[Function, GlobalVar], ErrorDiag,

Shouldn't you be extending the no_sanitize attribute instead, as it says in the 
comment?



Comment at: lib/CodeGen/CodeGenModule.cpp:130
   if (LangOpts.Sanitize.has(SanitizerKind::Thread) ||
+  LangOpts.Sanitize.has(SanitizerKind::TBAA) ||
   (!CodeGenOpts.RelaxedAliasing && CodeGenOpts.OptimizationLevel > 0))

`hasOneOf(SanitizerKind::Thread | SanitizerKind::TBAA)`?



Comment at: lib/CodeGen/CodeGenTBAA.cpp:96
+  if (!Features.Sanitize.has(SanitizerKind::TBAA) &&
+  (CodeGenOpts.OptimizationLevel == 0 || CodeGenOpts.RelaxedAliasing))
 return nullptr;

Interesting that TSan needs TBAA (per the comment in the chunk above), but is 
not in this condition.


https://reviews.llvm.org/D32199



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

[PATCH] D31885: Remove TBAA information from LValues representing union members

[Krzysztof Parzyszek via Phabricator via cfe-commits]

kparzysz added a comment.

In https://reviews.llvm.org/D31885#730038, @hfinkel wrote:

> I'm somewhat concerned that this patch is quadratic in the AST.


I'd be happy to address this, but I'm not sure how.  Memoizing results could be 
one way, but don't know if that's acceptable.

This location in codegen seems to be the last place where the original C/C++ 
types are available, in particular the information as to whether a type is a 
union or not.  Maybe it would be possible to propagate some bit somewhere, but 
then this patch would become much less localized.


Repository:
  rL LLVM

https://reviews.llvm.org/D31885



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

[PATCH] D32231: [CMake] Enable ARM target in Fuchsia toolchain

[Roland McGrath via Phabricator via cfe-commits]

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

lgtm


Repository:
  rL LLVM

https://reviews.llvm.org/D32231



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

r300718 - Prefer addAttr(Attribute::AttrKind) over the AttributeList overload

[Reid Kleckner via cfe-commits]

Author: rnk
Date: Wed Apr 19 12:28:52 2017
New Revision: 300718

URL: http://llvm.org/viewvc/llvm-project?rev=300718&view=rev
Log:
Prefer addAttr(Attribute::AttrKind) over the AttributeList overload

This should simplify the call sites, which typically want to tweak one
attribute at a time. It should also avoid creating ephemeral
AttributeLists that live forever.

Modified:
cfe/trunk/lib/CodeGen/CGCall.cpp
cfe/trunk/lib/CodeGen/CGObjC.cpp

Modified: cfe/trunk/lib/CodeGen/CGCall.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCall.cpp?rev=300718&r1=300717&r2=300718&view=diff
==
--- cfe/trunk/lib/CodeGen/CGCall.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGCall.cpp Wed Apr 19 12:28:52 2017
@@ -2310,8 +2310,7 @@ void CodeGenFunction::EmitFunctionProlog
 llvm::AttrBuilder Attrs;
 Attrs.addDereferenceableAttr(
   getContext().getTypeSizeInChars(ETy).getQuantity()*ArrSize);
-AI->addAttr(llvm::AttributeList::get(
-getLLVMContext(), AI->getArgNo() + 1, Attrs));
+AI->addAttrs(Attrs);
   } else if (getContext().getTargetAddressSpace(ETy) == 0) {
 AI->addAttr(llvm::Attribute::NonNull);
   }
@@ -2330,19 +2329,14 @@ void CodeGenFunction::EmitFunctionProlog
   if (!AVAttr)
 if (const auto *TOTy = dyn_cast(OTy))
   AVAttr = TOTy->getDecl()->getAttr();
-  if (AVAttr) { 
+  if (AVAttr) {
 llvm::Value *AlignmentValue =
   EmitScalarExpr(AVAttr->getAlignment());
 llvm::ConstantInt *AlignmentCI =
   cast(AlignmentValue);
-unsigned Alignment =
-  std::min((unsigned) AlignmentCI->getZExtValue(),
-   +llvm::Value::MaximumAlignment);
-
-llvm::AttrBuilder Attrs;
-Attrs.addAlignmentAttr(Alignment);
-AI->addAttr(llvm::AttributeList::get(getLLVMContext(),
- AI->getArgNo() + 1, Attrs));
+unsigned Alignment = 
std::min((unsigned)AlignmentCI->getZExtValue(),
+  +llvm::Value::MaximumAlignment);
+AI->addAttrs(llvm::AttrBuilder().addAlignmentAttr(Alignment));
   }
 }
 

Modified: cfe/trunk/lib/CodeGen/CGObjC.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGObjC.cpp?rev=300718&r1=300717&r2=300718&view=diff
==
--- cfe/trunk/lib/CodeGen/CGObjC.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGObjC.cpp Wed Apr 19 12:28:52 2017
@@ -1850,12 +1850,8 @@ static llvm::Constant *createARCRuntimeF
   F->addFnAttr(llvm::Attribute::NonLazyBind);
 }
 
-if (IsForwarding(Name)) {
-  llvm::AttrBuilder B;
-  B.addAttribute(llvm::Attribute::Returned);
-
-  F->arg_begin()->addAttr(llvm::AttributeList::get(F->getContext(), 1, B));
-}
+if (IsForwarding(Name))
+  F->arg_begin()->addAttr(llvm::Attribute::Returned);
   }
 
   return RTF;


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

[PATCH] D32207: Corrrect warn_unused_result attribute

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In https://reviews.llvm.org/D32207#730681, @erichkeane wrote:

> 1 more thing:  I excepted post-increment/decrement from this warning because 
> I figured it would give a TON of warnings from people using post-inc/dec in 
> for loops.
>
> However, after further discussion with Craig (who brought this up on the 
> mailing list), I severely wonder if that is the 'right' thing to do.  Someone 
> who is going to mark their struct in this way would likely ALSO mean it in 
> this case, right?
>
> Aaron: Should I simply remove the special casing all together, so that it 
> warns on post inc/dec?  That would definitely solve all your concerns in the 
> review so far.


I think that would make sense, yes.




Comment at: lib/AST/Decl.cpp:3010
+(OpCode == OO_PlusPlus || OpCode == OO_MinusMinus) &&
+(this->getNumParams() + (isa(this) ? 1 : 0)) == 2;
+if (Ret && !IsPostfix) {

erichkeane wrote:
> aaron.ballman wrote:
> > CXXMethodDecl represents a static or instance method, so I'm not certain 
> > this logic is quite correct.
> I actually stole this logic from lib/Sema/SemaDeclCXX.cpp lines 12708 and 
> 12754.  I believe the logic here is that it operators aren't allowed to be 
> static?  Since it is illegal to do so,  I would expect that I'd simply need 
> to do an assert to prevent it?
> 
> I'm open for ideas if you have one here, I'm not sure what the 'right' thing 
> to do is.
I was wondering more how a friend declaration of the operator might show up. 
e.g.,
```
struct S {
  friend S operator++(S Operand, int);
};
```



Comment at: test/SemaCXX/warn-unused-result.cpp:166
+// are special-cased to not warn for return-type due to ubiquitousness.
+struct[[clang::warn_unused_result]] S {
+  S DoThing() { return {}; };

erichkeane wrote:
> aaron.ballman wrote:
> > Can you also add a test with [[nodiscard]] and verify that the uses of 
> > increment/decrement *are* diagnosed?
> I can add that, it will obviously cause a slight change in logic since I 
> didn't handle that separately.  Are we surewe WANT [[nodiscard]] to warn in 
> this case?
I think so, yes. The C++ standard doesn't provide normative requirements here, 
but its encouragement to implementers doesn't mention this kind of behavior. 
[dcl.attr.nodiscard]p2:
```
A nodiscard call is a function call expression that calls a function previously 
declared nodiscard, or
whose return type is a possibly cv-qualified class or enumeration type marked 
nodiscard. Appearance of a nodiscard call as a potentially-evaluated 
discarded-value expression (Clause 8) is discouraged unless explicitly cast to 
void. Implementations are encouraged to issue a warning in such cases. This is 
typically because discarding the return value of a nodiscard call has 
surprising consequences.
```
Since this is returning a class type that is marked as nodiscard, I would 
expect it to be diagnosed with the standard attribute spelling.


https://reviews.llvm.org/D32207



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

[PATCH] D31885: Remove TBAA information from LValues representing union members

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In https://reviews.llvm.org/D31885#730539, @dberlin wrote:

> In https://reviews.llvm.org/D31885#730072, @rjmccall wrote:
>
> > Thanks for CC'ing me.  There are two problems here.
> >
> > The second is that our alias-analysis philosophy says that the fact that 
> > two accesses "sufficiently obviously" can alias should always override TBAA.
>
>
> I'm not sure i agree.
>  We have made explicit exceptions for parts of the standard that are 
> nonsensical (and they've been slowly updated in newer standards to be less 
> nonsensical).  Outside of that, i don't believe that is our philosophy, nor 
> would i believe it should be.
>  There is no "do what i mean" here we can define that makes sense.


There was a deliberate decision to make TBAA conservative about type punning in 
LLVM because, in practice, the strict form of TBAA you think we should follow 
has proven to be a failed optimization paradigm: it just leads users to 
globally disable TBAA, which is obviously a huge loss.  This was never 
motivated purely by the nonsensicality of the standardization of unions.

(And the nonsensicality of the standard very much continues.  The code that 
we've all agreed is obviously being miscompiled here is still a strict 
violation of the "improved" union rules in C++, and if we decide to treat it as 
ill-formed we're going to break a massive amount of code (and vector code in 
particular heavily uses unions), because trust me, nobody is reliably 
placement-newing union fields when they're non-trivial.  C++'s new rules are 
unworkable in part because they rely on C++-specific features that cannot be 
easily adopted in headers which need to be language-neutral.)

>>   I have a hard time seeing what about the test cases I'm seeing makes it 
>> insufficiently obvious that the accesses alias.  The  accesses are 
>> ultimately derived from the same pointer; either the analysis is capable of 
>> counting to 32, in which case it should see that they definitely alias, or 
>> it is not, in which can it needs to assume that they might.  Perhaps the 
>> existing AA interface is just inadequate?  It seems to me that it can't 
>> express that middle-ground of "I have a concrete reason to think that these 
>> pointers are related but cannot answer for certain whether they overlap".  
>> Because if it could just answer that rather than having to downgrade all the 
>> way to MayAlias, it seems clear to me that (1) most memory transforms would 
>> treat it exactly the same as MayAlias but (2) it should be considered strong 
>> enough to suppress TBAA.
> 
> Just to note: Even assuming this wasn't crazytown, we certainly can't do it 
> in  the backend :)

Well, thanks for calling me crazy, I guess.

John.


Repository:
  rL LLVM

https://reviews.llvm.org/D31885



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

[PATCH] D31885: Remove TBAA information from LValues representing union members

[Krzysztof Parzyszek via Phabricator via cfe-commits]

kparzysz updated this revision to Diff 95780.
kparzysz added a comment.

Memoize known results of checking for union access.


Repository:
  rL LLVM

https://reviews.llvm.org/D31885

Files:
  lib/CodeGen/CGExpr.cpp
  lib/CodeGen/CodeGenFunction.cpp
  lib/CodeGen/CodeGenFunction.h
  test/CodeGen/union-tbaa1.c
  test/CodeGenCXX/union-tbaa2.cpp

Index: test/CodeGenCXX/union-tbaa2.cpp
===
--- /dev/null
+++ test/CodeGenCXX/union-tbaa2.cpp
@@ -0,0 +1,41 @@
+// RUN: %clang_cc1 %s -triple x86_64-unknown-linux-gnu -target-cpu x86-64 -target-feature +sse4.2 -target-feature +avx -emit-llvm -o - | FileCheck %s
+
+// Testcase from llvm.org/PR32056
+
+extern "C" int printf (const char *__restrict __format, ...);
+
+typedef double __m256d __attribute__((__vector_size__(32)));
+
+static __inline __m256d __attribute__((__always_inline__, __nodebug__,
+   __target__("avx")))
+_mm256_setr_pd(double __a, double __b, double __c, double __d) {
+  return (__m256d){ __a, __b, __c, __d };
+}
+
+struct A {
+  A () {
+// Check that there is no TBAA information generated for the stores to the
+// union members:
+// CHECK: store <4 x double>
+// CHECK-NOT: tbaa
+// CHECK: store <4 x double>
+// CHECK-NOT: tbaa
+a = _mm256_setr_pd(0.0, 1.0, 2.0, 3.0);
+b = _mm256_setr_pd(4.0, 5.0, 6.0, 7.0);
+  }
+
+  const double *begin() { return c; }
+  const double *end() { return c+8; }
+
+  union {
+struct { __m256d a, b; };
+double c[8];
+  };
+};
+
+int main(int argc, char *argv[]) {
+  A a;
+  for (double value : a)
+printf("%f ", value);
+  return 0;
+}
Index: test/CodeGen/union-tbaa1.c
===
--- /dev/null
+++ test/CodeGen/union-tbaa1.c
@@ -0,0 +1,42 @@
+// RUN: %clang_cc1 %s -triple hexagon-unknown-elf -O2 -emit-llvm -o - | FileCheck %s
+
+typedef union __attribute__((aligned(4))) {
+  unsigned short uh[2];
+  unsigned uw;
+} vect32;
+
+void bar(vect32 p[][2]);
+
+// CHECK-LABEL: define void @fred
+void fred(unsigned Num, int Vec[2], int *Index, int Arr[4][2]) {
+  vect32 Tmp[4][2];
+// Generate tbaa for the load of Index:
+// CHECK: load i32, i32* %Index{{.*}}tbaa
+// But no tbaa for the two stores:
+// CHECK: %uw[[UW1:[0-9]*]] = getelementptr
+// CHECK: store{{.*}}%uw[[UW1]]
+// CHECK-NOT: tbaa
+// There will be a load after the store, and it will use tbaa. Make sure
+// the check-not above doesn't find it:
+// CHECK: load
+  Tmp[*Index][0].uw = Arr[*Index][0] * Num;
+// CHECK: %uw[[UW2:[0-9]*]] = getelementptr
+// CHECK: store{{.*}}%uw[[UW2]]
+// CHECK-NOT: tbaa
+  Tmp[*Index][1].uw = Arr[*Index][1] * Num;
+// Same here, don't generate tbaa for the loads:
+// CHECK: %uh[[UH1:[0-9]*]] = bitcast %union.vect32
+// CHECK: %arrayidx[[AX1:[0-9]*]] = getelementptr{{.*}}%uh[[UH1]]
+// CHECK: load i16, i16* %arrayidx[[AX1]]
+// CHECK-NOT: tbaa
+// CHECK: store
+  Vec[0] = Tmp[*Index][0].uh[1];
+// CHECK: %uh[[UH2:[0-9]*]] = bitcast %union.vect32
+// CHECK: %arrayidx[[AX2:[0-9]*]] = getelementptr{{.*}}%uh[[UH2]]
+// CHECK: load i16, i16* %arrayidx[[AX2]]
+// CHECK-NOT: tbaa
+// CHECK: store
+  Vec[1] = Tmp[*Index][1].uh[1];
+  bar(Tmp);
+}
+
Index: lib/CodeGen/CodeGenFunction.h
===
--- lib/CodeGen/CodeGenFunction.h
+++ lib/CodeGen/CodeGenFunction.h
@@ -2885,6 +2885,21 @@
   /// that the address will be used to access the object.
   LValue EmitCheckedLValue(const Expr *E, TypeCheckKind TCK);
 
+private:
+  /// The actual implementations of LValue emission. As a workaround for
+  /// a problem in representing union member accesses in TBAA, the public
+  /// functions will remove the TBAA information from any LValue generated
+  /// for such an access.
+  /// When the TBAA issue is fixed, the public wrappers (declared above)
+  /// should be replaced with these functions.
+  LValue EmitLValueImpl(const Expr *E);
+  LValue EmitCheckedLValueImpl(const Expr *E, TypeCheckKind TCK);
+
+  /// Cached results of previous checks for union access. This is reset
+  /// at the beginning of GenerateCode.
+  llvm::DenseMap UnionAccesses;
+
+public:
   RValue convertTempToRValue(Address addr, QualType type,
  SourceLocation Loc);
 
Index: lib/CodeGen/CodeGenFunction.cpp
===
--- lib/CodeGen/CodeGenFunction.cpp
+++ lib/CodeGen/CodeGenFunction.cpp
@@ -1103,6 +1103,9 @@
 
 void CodeGenFunction::GenerateCode(GlobalDecl GD, llvm::Function *Fn,
const CGFunctionInfo &FnInfo) {
+  // Clear the cache of known union accesses.
+  UnionAccesses.clear();
+
   const FunctionDecl *FD = cast(GD.getDecl());
   CurGD = GD;
 
Index: lib/CodeGen/CGExpr.cpp
===
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -995,7 +995,49 @@
   return true;
 }
 
+/

[PATCH] D31745: [OpenCL] Added diagnostic for implicit declaration of function in OpenCL

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.



Comment at: test/SemaOpenCL/clang-builtin-version.cl:32
+  work_group_reserve_write_pipe(tmp, tmp); // expected-error{{implicit 
declaration of function 'work_group_reserve_write_pipe' is invalid in OpenCL}}
+  // expected-note@-1{{did you mean 'work_group_reserve_read_pipe'?}}
+  // expected-note@-2{{'work_group_reserve_write_pipe' declared here}}

echuraev wrote:
> Anastasia wrote:
> > Anastasia wrote:
> > > echuraev wrote:
> > > > Anastasia wrote:
> > > > > Why do we get this note now? I believe work_group_reserve_read_pipe 
> > > > > shouldn't be available either?
> > > > May be I don't understand something but I think that it is the right 
> > > > diagnostic message. We called work_group_reserve_read_pipe in line 29. 
> > > > So for this case we will get the following message: 
> > > > //clang-builtin-version.cl: 31 error: implicit declaration of function 
> > > > 'work_group_reserve_write_pipe' is invalid in OpenCL
> > > > clang-builtin-version.cl: 31 note: did you mean 
> > > > 'work_group_reserve_read_pipe'?
> > > > clang-builtin-version.cl: 29 note: 'work_group_reserve_read_pipe' 
> > > > declared here//
> > > But there is an error now given for the call to 
> > > 'work_group_reserve_read_pipe'. Why is it still added to the 
> > > declarations? I think we should prevent this.
> > Also do you know why we didn't have these notes before? I don't see 
> > anything related in your change.
> I run this test and got a log with errors. The full log you can find in the 
> end of the message.
> I think that the diagnostic messages are right. First error we get for 
> implicit declaration of **//work_group_reserve_read_pipe//**. After that we 
> call function **//work_group_reserve_write_pipe//** and get the same error 
> about implicit declaration of function. But also we get two diagnostic 
> messages. Compiler already know about **//work_group_reserve_read_pipe//** 
> and make a hypothesis: "//note: did you mean 
> 'work_group_reserve_read_pipe'?//" and "//note: 
> 'work_group_reserve_read_pipe' declared here//".
> As well, next we have declaration of **//work_group_commit_read_pipe//**. And 
> we get an error about implicit declaration of this function and also get a 
> note messages: "//note: did you mean 'work_group_reserve_read_pipe'?//" and 
> "//note: 'work_group_reserve_read_pipe' declared here//".
> 
> 
> > Also do you know why we didn't have these notes before? I don't see 
> > anything related in your change.
> 
> Unfortunately, I don't know why we get these notes. They have appeared when I 
> changed the status of diagnostic message for implicit declaration from 
> warning to error. If we had only warning messages then we wouldn't have these 
> notes.
> 
> 
> 
> 
> ```
> clang-builtin-version.cl:35:3: error: implicit declaration of function 
> 'work_group_reserve_read_pipe' is invalid in OpenCL
>   work_group_reserve_read_pipe(tmp, tmp);
>   ^
> 
> clang-builtin-version.cl:37:3: error: implicit declaration of function 
> 'work_group_reserve_write_pipe' is invalid in OpenCL
>   work_group_reserve_write_pipe(tmp, tmp);
>   ^
> clang-builtin-version.cl:37:3: note: did you mean 
> 'work_group_reserve_read_pipe'?
> clang-builtin-version.cl:35:3: note: 'work_group_reserve_read_pipe' declared 
> here
>   work_group_reserve_read_pipe(tmp, tmp);
>   ^
> 
> clang-builtin-version.cl:50:3: error: implicit declaration of function 
> 'work_group_commit_read_pipe' is invalid in OpenCL
>   work_group_commit_read_pipe(tmp, tmp);
>   ^
> clang-builtin-version.cl:50:3: note: did you mean 
> 'work_group_reserve_read_pipe'?
> clang-builtin-version.cl:35:3: note: 'work_group_reserve_read_pipe' declared 
> here
>   work_group_reserve_read_pipe(tmp, tmp);
>   ^
> ```
I find those notes counterintuitive to be honest because if there is an error 
given for the function declaration - let's say `foo()`, it shouldn't have been 
added anywhere to any dictionary because if the next function let's say `foa()` 
is corrected to match the name from the note (i.e. `foo()`) it is already known 
that it won't be compiled too. So to me this note seems wrong because it 
attempts to correct to something which is already known not to be compiled. I 
think it's worth digging into it a bit more...


https://reviews.llvm.org/D31745



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

r300722 - [Sema][ObjC] Disallow jumping into ObjC fast enumeration loops.

[Akira Hatanaka via cfe-commits]

Author: ahatanak
Date: Wed Apr 19 12:54:08 2017
New Revision: 300722

URL: http://llvm.org/viewvc/llvm-project?rev=300722&view=rev
Log:
[Sema][ObjC] Disallow jumping into ObjC fast enumeration loops.

rdar://problem/31635406

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

Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/Sema/JumpDiagnostics.cpp
cfe/trunk/lib/Sema/SemaStmt.cpp
cfe/trunk/test/SemaObjC/foreach.m

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=300722&r1=300721&r2=300722&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Wed Apr 19 12:54:08 
2017
@@ -5000,6 +5000,8 @@ def note_protected_by_if_available : Not
   "jump enters controlled statement of if available">;
 def note_protected_by_vla : Note<
   "jump bypasses initialization of variable length array">;
+def note_protected_by_objc_fast_enumeration : Note<
+  "jump enters Objective-C fast enumeration loop">;
 def note_protected_by_objc_try : Note<
   "jump bypasses initialization of @try block">;
 def note_protected_by_objc_catch : Note<

Modified: cfe/trunk/lib/Sema/JumpDiagnostics.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/JumpDiagnostics.cpp?rev=300722&r1=300721&r2=300722&view=diff
==
--- cfe/trunk/lib/Sema/JumpDiagnostics.cpp (original)
+++ cfe/trunk/lib/Sema/JumpDiagnostics.cpp Wed Apr 19 12:54:08 2017
@@ -287,6 +287,15 @@ void JumpScopeChecker::BuildScopeInforma
 IndirectJumpTargets.push_back(cast(S)->getLabel());
 break;
 
+  case Stmt::ObjCForCollectionStmtClass: {
+auto *CS = cast(S);
+unsigned Diag = diag::note_protected_by_objc_fast_enumeration;
+unsigned NewParentScope = Scopes.size();
+Scopes.push_back(GotoScope(ParentScope, Diag, 0, S->getLocStart()));
+BuildScopeInformation(CS->getBody(), NewParentScope);
+return;
+  }
+
   case Stmt::IndirectGotoStmtClass:
 // "goto *&&lbl;" is a special case which we treat as equivalent
 // to a normal goto.  In addition, we don't calculate scope in the

Modified: cfe/trunk/lib/Sema/SemaStmt.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaStmt.cpp?rev=300722&r1=300721&r2=300722&view=diff
==
--- cfe/trunk/lib/Sema/SemaStmt.cpp (original)
+++ cfe/trunk/lib/Sema/SemaStmt.cpp Wed Apr 19 12:54:08 2017
@@ -1783,6 +1783,7 @@ StmtResult
 Sema::ActOnObjCForCollectionStmt(SourceLocation ForLoc,
  Stmt *First, Expr *collection,
  SourceLocation RParenLoc) {
+  getCurFunction()->setHasBranchProtectedScope();
 
   ExprResult CollectionExprResult =
 CheckObjCForCollectionOperand(ForLoc, collection);

Modified: cfe/trunk/test/SemaObjC/foreach.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/foreach.m?rev=300722&r1=300721&r2=300722&view=diff
==
--- cfe/trunk/test/SemaObjC/foreach.m (original)
+++ cfe/trunk/test/SemaObjC/foreach.m Wed Apr 19 12:54:08 2017
@@ -55,3 +55,27 @@ void test2(NSObject *
   for (obj.prop in collection) { /* expected-error {{selector element is not a 
valid lvalue}} */
   }
 }
+
+int cond();
+
+void test3(NSObject *a0, NSObject *a1) {
+  for (id i in a0) { /* expected-note 2 {{jump enters Objective-C fast 
enumeration loop}} */
+for (id j in a1) { /* expected-note 2 {{jump enters Objective-C fast 
enumeration loop}} */
+  (void)i, (void)j;
+label0:
+  if (cond())
+goto label1;
+}
+label1:
+if (cond())
+  goto label0; /* expected-error {{cannot jump from this goto statement to 
its label}} */
+if (cond())
+  goto label2;
+  }
+
+label2:
+  if (cond())
+goto label0; /* expected-error {{cannot jump from this goto statement to 
its label}} */
+  if (cond())
+goto label1; /* expected-error{{cannot jump from this goto statement to 
its label}} */
+}


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

[PATCH] D32187: [CodeGen][ObjC]

[Akira Hatanaka via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL300722: [Sema][ObjC] Disallow jumping into ObjC fast 
enumeration loops. (authored by ahatanak).

Changed prior to commit:
  https://reviews.llvm.org/D32187?vs=95667&id=95781#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D32187

Files:
  cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
  cfe/trunk/lib/Sema/JumpDiagnostics.cpp
  cfe/trunk/lib/Sema/SemaStmt.cpp
  cfe/trunk/test/SemaObjC/foreach.m


Index: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
===
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
@@ -5000,6 +5000,8 @@
   "jump enters controlled statement of if available">;
 def note_protected_by_vla : Note<
   "jump bypasses initialization of variable length array">;
+def note_protected_by_objc_fast_enumeration : Note<
+  "jump enters Objective-C fast enumeration loop">;
 def note_protected_by_objc_try : Note<
   "jump bypasses initialization of @try block">;
 def note_protected_by_objc_catch : Note<
Index: cfe/trunk/test/SemaObjC/foreach.m
===
--- cfe/trunk/test/SemaObjC/foreach.m
+++ cfe/trunk/test/SemaObjC/foreach.m
@@ -55,3 +55,27 @@
   for (obj.prop in collection) { /* expected-error {{selector element is not a 
valid lvalue}} */
   }
 }
+
+int cond();
+
+void test3(NSObject *a0, NSObject *a1) {
+  for (id i in a0) { /* expected-note 2 {{jump enters Objective-C fast 
enumeration loop}} */
+for (id j in a1) { /* expected-note 2 {{jump enters Objective-C fast 
enumeration loop}} */
+  (void)i, (void)j;
+label0:
+  if (cond())
+goto label1;
+}
+label1:
+if (cond())
+  goto label0; /* expected-error {{cannot jump from this goto statement to 
its label}} */
+if (cond())
+  goto label2;
+  }
+
+label2:
+  if (cond())
+goto label0; /* expected-error {{cannot jump from this goto statement to 
its label}} */
+  if (cond())
+goto label1; /* expected-error{{cannot jump from this goto statement to 
its label}} */
+}
Index: cfe/trunk/lib/Sema/SemaStmt.cpp
===
--- cfe/trunk/lib/Sema/SemaStmt.cpp
+++ cfe/trunk/lib/Sema/SemaStmt.cpp
@@ -1783,6 +1783,7 @@
 Sema::ActOnObjCForCollectionStmt(SourceLocation ForLoc,
  Stmt *First, Expr *collection,
  SourceLocation RParenLoc) {
+  getCurFunction()->setHasBranchProtectedScope();
 
   ExprResult CollectionExprResult =
 CheckObjCForCollectionOperand(ForLoc, collection);
Index: cfe/trunk/lib/Sema/JumpDiagnostics.cpp
===
--- cfe/trunk/lib/Sema/JumpDiagnostics.cpp
+++ cfe/trunk/lib/Sema/JumpDiagnostics.cpp
@@ -287,6 +287,15 @@
 IndirectJumpTargets.push_back(cast(S)->getLabel());
 break;
 
+  case Stmt::ObjCForCollectionStmtClass: {
+auto *CS = cast(S);
+unsigned Diag = diag::note_protected_by_objc_fast_enumeration;
+unsigned NewParentScope = Scopes.size();
+Scopes.push_back(GotoScope(ParentScope, Diag, 0, S->getLocStart()));
+BuildScopeInformation(CS->getBody(), NewParentScope);
+return;
+  }
+
   case Stmt::IndirectGotoStmtClass:
 // "goto *&&lbl;" is a special case which we treat as equivalent
 // to a normal goto.  In addition, we don't calculate scope in the


Index: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
===
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
@@ -5000,6 +5000,8 @@
   "jump enters controlled statement of if available">;
 def note_protected_by_vla : Note<
   "jump bypasses initialization of variable length array">;
+def note_protected_by_objc_fast_enumeration : Note<
+  "jump enters Objective-C fast enumeration loop">;
 def note_protected_by_objc_try : Note<
   "jump bypasses initialization of @try block">;
 def note_protected_by_objc_catch : Note<
Index: cfe/trunk/test/SemaObjC/foreach.m
===
--- cfe/trunk/test/SemaObjC/foreach.m
+++ cfe/trunk/test/SemaObjC/foreach.m
@@ -55,3 +55,27 @@
   for (obj.prop in collection) { /* expected-error {{selector element is not a valid lvalue}} */
   }
 }
+
+int cond();
+
+void test3(NSObject *a0, NSObject *a1) {
+  for (id i in a0) { /* expected-note 2 {{jump enters Objective-C fast enumeration loop}} */
+for (id j in a1) { /* expected-note 2 {{jump enters Objective-C fast enumeration loop}} */
+  (void)i, (void)j;
+label0:
+  if (cond())
+goto label1;
+}
+label1:
+if (cond())
+  goto label0; /* expected-error {{cannot jump from this goto statement to its label}} */
+if (cond())
+  goto label2;
+  }
+
+l

[PATCH] D32210: [Sema][ObjC] Add support for attribute "noescape"

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

The rule we actually want is that no reference to the block derived from the 
parameter value will survive after the function returns.  You can include 
examples of things that would cause potential escapes, but trying to actually 
lock down the list seems ill-advised.

Are you sure you want to try to enforce this in the frontend?  In Swift, we 
discovered that we needed some way of escaping the static checking in order to 
make this practical, because there are a lot of patterns that locally look like 
escapes but really aren't (e.g. passing the block to dispatch_async and then 
waiting for that to complete).  Seems more like a static analysis than a 
frontend warning.


https://reviews.llvm.org/D32210



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

r300723 - [Coverage] Don't emit mappings for functions in dependent contexts (fixes PR32679)

[Vedant Kumar via cfe-commits]

Author: vedantk
Date: Wed Apr 19 12:58:30 2017
New Revision: 300723

URL: http://llvm.org/viewvc/llvm-project?rev=300723&view=rev
Log:
[Coverage] Don't emit mappings for functions in dependent contexts (fixes 
PR32679)

The coverage implementation marks functions which won't be emitted as
'deferred', so that it can emit empty coverage regions for them later
(once their linkages are known).

Functions in dependent contexts are an exception: if there isn't a full
instantiation of a function, it shouldn't be marked 'deferred'. We've
been breaking that rule without much consequence because we just ended
up with useless, extra, empty coverage mappings. With PR32679, this
behavior finally caused a crash, because clang marked a partial template
specialization as 'deferred', causing the MS mangler to choke in its
delayed-template-parsing mode:

  error: cannot mangle this template type parameter type yet
  (http://bugs.llvm.org/show_bug.cgi?id=32679)

Fix this by checking if a decl's context is a dependent context before
marking it 'deferred'.

Based on a patch by Adam Folwarczny!

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

Added:
cfe/trunk/test/CoverageMapping/pr32679.cpp
Modified:
cfe/trunk/lib/CodeGen/ModuleBuilder.cpp

Modified: cfe/trunk/lib/CodeGen/ModuleBuilder.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/ModuleBuilder.cpp?rev=300723&r1=300722&r2=300723&view=diff
==
--- cfe/trunk/lib/CodeGen/ModuleBuilder.cpp (original)
+++ cfe/trunk/lib/CodeGen/ModuleBuilder.cpp Wed Apr 19 12:58:30 2017
@@ -197,7 +197,7 @@ namespace {
   // Provide some coverage mapping even for methods that aren't emitted.
   // Don't do this for templated classes though, as they may not be
   // instantiable.
-  if (!MD->getParent()->getDescribedClassTemplate())
+  if (!MD->getParent()->isDependentContext())
 Builder->AddDeferredUnusedCoverageMapping(MD);
 }
 

Added: cfe/trunk/test/CoverageMapping/pr32679.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CoverageMapping/pr32679.cpp?rev=300723&view=auto
==
--- cfe/trunk/test/CoverageMapping/pr32679.cpp (added)
+++ cfe/trunk/test/CoverageMapping/pr32679.cpp Wed Apr 19 12:58:30 2017
@@ -0,0 +1,32 @@
+// RUN: %clang_cc1 -cc1 -triple i686-pc-windows-msvc19.0.0 -emit-obj 
-fprofile-instrument=clang -std=c++14 -fdelayed-template-parsing 
-fcoverage-mapping -dump-coverage-mapping -emit-llvm-only -main-file-name 
pr32679.cpp -o - %s | FileCheck %s -check-prefix=MSABI -implicit-check-not=f2
+// RUN: %clang_cc1 -cc1 -triple %itanium_abi_triple -emit-obj 
-fprofile-instrument=clang -std=c++14 -fcoverage-mapping -dump-coverage-mapping 
-emit-llvm-only -main-file-name pr32679.cpp -o - %s | FileCheck %s 
-check-prefix=ITANIUM -implicit-check-not=f2
+
+template 
+struct CreateSpecialization;
+
+template 
+struct CreateSpecialization {
+  static constexpr T f1() { return 0; }
+  static constexpr T f2() { return 0; }
+};
+
+int main() {
+  CreateSpecialization::f1();
+
+  // Don't emit coverage mapping info for functions in dependent contexts.
+  //
+  // E.g we never fully instantiate CreateSpecialization::f2(), so there
+  // should be no mapping for it.
+
+  return 0;
+}
+
+// MSABI: main:
+// MSABI-NEXT:   File 0, [[@LINE-12]]:12 -> [[@LINE-3]]:2 = #0
+// MSABI-NEXT: ?f1@?$CreateSpecialization@H$0A@@@SAHXZ:
+// MSABI-NEXT:   File 0, [[@LINE-18]]:27 -> [[@LINE-18]]:40 = #0
+
+// ITANIUM: main:
+// ITANIUM-NEXT:   File 0, [[@LINE-17]]:12 -> [[@LINE-8]]:2 = #0
+// ITANIUM-NEXT: _ZN20CreateSpecializationIiLi0EE2f1Ev:
+// ITANIUM-NEXT:   File 0, [[@LINE-23]]:27 -> [[@LINE-23]]:40 = #0


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

[PATCH] D32144: [Coverage] Don't emit mappings for functions in dependent contexts (fixes PR32679)

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL300723: [Coverage] Don't emit mappings for functions in 
dependent contexts (fixes… (authored by vedantk).

Changed prior to commit:
  https://reviews.llvm.org/D32144?vs=95513&id=95783#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D32144

Files:
  cfe/trunk/lib/CodeGen/ModuleBuilder.cpp
  cfe/trunk/test/CoverageMapping/pr32679.cpp


Index: cfe/trunk/test/CoverageMapping/pr32679.cpp
===
--- cfe/trunk/test/CoverageMapping/pr32679.cpp
+++ cfe/trunk/test/CoverageMapping/pr32679.cpp
@@ -0,0 +1,32 @@
+// RUN: %clang_cc1 -cc1 -triple i686-pc-windows-msvc19.0.0 -emit-obj 
-fprofile-instrument=clang -std=c++14 -fdelayed-template-parsing 
-fcoverage-mapping -dump-coverage-mapping -emit-llvm-only -main-file-name 
pr32679.cpp -o - %s | FileCheck %s -check-prefix=MSABI -implicit-check-not=f2
+// RUN: %clang_cc1 -cc1 -triple %itanium_abi_triple -emit-obj 
-fprofile-instrument=clang -std=c++14 -fcoverage-mapping -dump-coverage-mapping 
-emit-llvm-only -main-file-name pr32679.cpp -o - %s | FileCheck %s 
-check-prefix=ITANIUM -implicit-check-not=f2
+
+template 
+struct CreateSpecialization;
+
+template 
+struct CreateSpecialization {
+  static constexpr T f1() { return 0; }
+  static constexpr T f2() { return 0; }
+};
+
+int main() {
+  CreateSpecialization::f1();
+
+  // Don't emit coverage mapping info for functions in dependent contexts.
+  //
+  // E.g we never fully instantiate CreateSpecialization::f2(), so there
+  // should be no mapping for it.
+
+  return 0;
+}
+
+// MSABI: main:
+// MSABI-NEXT:   File 0, [[@LINE-12]]:12 -> [[@LINE-3]]:2 = #0
+// MSABI-NEXT: ?f1@?$CreateSpecialization@H$0A@@@SAHXZ:
+// MSABI-NEXT:   File 0, [[@LINE-18]]:27 -> [[@LINE-18]]:40 = #0
+
+// ITANIUM: main:
+// ITANIUM-NEXT:   File 0, [[@LINE-17]]:12 -> [[@LINE-8]]:2 = #0
+// ITANIUM-NEXT: _ZN20CreateSpecializationIiLi0EE2f1Ev:
+// ITANIUM-NEXT:   File 0, [[@LINE-23]]:27 -> [[@LINE-23]]:40 = #0
Index: cfe/trunk/lib/CodeGen/ModuleBuilder.cpp
===
--- cfe/trunk/lib/CodeGen/ModuleBuilder.cpp
+++ cfe/trunk/lib/CodeGen/ModuleBuilder.cpp
@@ -197,7 +197,7 @@
   // Provide some coverage mapping even for methods that aren't emitted.
   // Don't do this for templated classes though, as they may not be
   // instantiable.
-  if (!MD->getParent()->getDescribedClassTemplate())
+  if (!MD->getParent()->isDependentContext())
 Builder->AddDeferredUnusedCoverageMapping(MD);
 }
 


Index: cfe/trunk/test/CoverageMapping/pr32679.cpp
===
--- cfe/trunk/test/CoverageMapping/pr32679.cpp
+++ cfe/trunk/test/CoverageMapping/pr32679.cpp
@@ -0,0 +1,32 @@
+// RUN: %clang_cc1 -cc1 -triple i686-pc-windows-msvc19.0.0 -emit-obj -fprofile-instrument=clang -std=c++14 -fdelayed-template-parsing -fcoverage-mapping -dump-coverage-mapping -emit-llvm-only -main-file-name pr32679.cpp -o - %s | FileCheck %s -check-prefix=MSABI -implicit-check-not=f2
+// RUN: %clang_cc1 -cc1 -triple %itanium_abi_triple -emit-obj -fprofile-instrument=clang -std=c++14 -fcoverage-mapping -dump-coverage-mapping -emit-llvm-only -main-file-name pr32679.cpp -o - %s | FileCheck %s -check-prefix=ITANIUM -implicit-check-not=f2
+
+template 
+struct CreateSpecialization;
+
+template 
+struct CreateSpecialization {
+  static constexpr T f1() { return 0; }
+  static constexpr T f2() { return 0; }
+};
+
+int main() {
+  CreateSpecialization::f1();
+
+  // Don't emit coverage mapping info for functions in dependent contexts.
+  //
+  // E.g we never fully instantiate CreateSpecialization::f2(), so there
+  // should be no mapping for it.
+
+  return 0;
+}
+
+// MSABI: main:
+// MSABI-NEXT:   File 0, [[@LINE-12]]:12 -> [[@LINE-3]]:2 = #0
+// MSABI-NEXT: ?f1@?$CreateSpecialization@H$0A@@@SAHXZ:
+// MSABI-NEXT:   File 0, [[@LINE-18]]:27 -> [[@LINE-18]]:40 = #0
+
+// ITANIUM: main:
+// ITANIUM-NEXT:   File 0, [[@LINE-17]]:12 -> [[@LINE-8]]:2 = #0
+// ITANIUM-NEXT: _ZN20CreateSpecializationIiLi0EE2f1Ev:
+// ITANIUM-NEXT:   File 0, [[@LINE-23]]:27 -> [[@LINE-23]]:40 = #0
Index: cfe/trunk/lib/CodeGen/ModuleBuilder.cpp
===
--- cfe/trunk/lib/CodeGen/ModuleBuilder.cpp
+++ cfe/trunk/lib/CodeGen/ModuleBuilder.cpp
@@ -197,7 +197,7 @@
   // Provide some coverage mapping even for methods that aren't emitted.
   // Don't do this for templated classes though, as they may not be
   // instantiable.
-  if (!MD->getParent()->getDescribedClassTemplate())
+  if (!MD->getParent()->isDependentContext())
 Builder->AddDeferredUnusedCoverageMapping(MD);
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D31885: Remove TBAA information from LValues representing union members

[Daniel Berlin via Phabricator via cfe-commits]

dberlin added a comment.

In https://reviews.llvm.org/D31885#730766, @rjmccall wrote:

> In https://reviews.llvm.org/D31885#730539, @dberlin wrote:
>
> > In https://reviews.llvm.org/D31885#730072, @rjmccall wrote:
> >
> > > Thanks for CC'ing me.  There are two problems here.
> > >
> > > The second is that our alias-analysis philosophy says that the fact that 
> > > two accesses "sufficiently obviously" can alias should always override 
> > > TBAA.
> >
> >
> > I'm not sure i agree.
> >  We have made explicit exceptions for parts of the standard that are 
> > nonsensical (and they've been slowly updated in newer standards to be less 
> > nonsensical).  Outside of that, i don't believe that is our philosophy, nor 
> > would i believe it should be.
> >  There is no "do what i mean" here we can define that makes sense.
>
>
> There was a deliberate decision to make TBAA conservative about type punning 
> in LLVM because, in practice, the strict form of TBAA you think we should 
> follow has proven to be a failed optimization paradigm: it just leads users 
> to globally disable TBAA, which is obviously a huge loss.  This was never 
> motivated purely by the nonsensicality of the standardization of unions.


I think you have completely missed my point.  i have not said we should be 
super strict, i'm saying "there is no DWIM" and "we can't do it in the 
backend". work.

> (And the nonsensicality of the standard very much continues.  The code that 
> we've all agreed is obviously being miscompiled here is still a strict 
> violation of the "improved" union rules in C++, and if we decide to treat it 
> as ill-formed we're going to break a massive amount of code (and vector code 
> in particular heavily uses unions), because trust me, nobody is reliably 
> placement-newing union fields when they're non-trivial.  C++'s new rules are 
> unworkable in part because they rely on C++-specific features that cannot be 
> easily adopted in headers which need to be language-neutral.)
> 
>>>   I have a hard time seeing what about the test cases I'm seeing makes it 
>>> insufficiently obvious that the accesses alias.  The  accesses are 
>>> ultimately derived from the same pointer; either the analysis is capable of 
>>> counting to 32, in which case it should see that they definitely alias, or 
>>> it is not, in which can it needs to assume that they might.  Perhaps the 
>>> existing AA interface is just inadequate?  It seems to me that it can't 
>>> express that middle-ground of "I have a concrete reason to think that these 
>>> pointers are related but cannot answer for certain whether they overlap".  
>>> Because if it could just answer that rather than having to downgrade all 
>>> the way to MayAlias, it seems clear to me that (1) most memory transforms 
>>> would treat it exactly the same as MayAlias but (2) it should be considered 
>>> strong enough to suppress TBAA.
>> 
>> Just to note: Even assuming this wasn't crazytown, we certainly can't do it 
>> in  the backend :)
> 
> Well, thanks for calling me crazy, I guess.

???
I didn't call you crazy, and apologies if you took it that way.
I meant the approach of trying to do it in the backend is crazy and doesn't 
work.

> John.




Repository:
  rL LLVM

https://reviews.llvm.org/D31885



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

[PATCH] D31542: [clang-tidy] Extend readability-container-size-empty to add comparisons to newly-constructed objects

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tidy/utils/ASTUtils.cpp:28
+bool IsBinaryOrTernary(const Expr *expr) {
+  if (clang::isa(expr->IgnoreImpCasts()) ||
+  clang::isa(expr->IgnoreImpCasts())) {

Rather than call `IgnoreImpCasts()` three times, you should hoist it out of the 
if statements.



Comment at: clang-tidy/utils/ASTUtils.cpp:33
+
+  if (const auto* binary = clang::dyn_cast(
+  expr->IgnoreImpCasts())) {

`binary` doesn't match the usual naming conventions, and the asterisk should 
bind right rather than left.



Comment at: clang-tidy/utils/ASTUtils.h:22
+// Determine whether Expr is a Binary or Ternary expression.
+bool IsBinaryOrTernary(const Expr *expr);
 } // namespace utils

The parameter name doesn't match our usual naming conventions. I'd just go with 
`E`.


Repository:
  rL LLVM

https://reviews.llvm.org/D31542



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

[PATCH] D31885: Remove TBAA information from LValues representing union members

[Hal Finkel via Phabricator via cfe-commits]

hfinkel added a comment.

> There was a deliberate decision to make TBAA conservative about type punning 
> in LLVM because, in practice, the strict form of TBAA you think we should 
> follow has proven to be a failed optimization paradigm: it just leads users 
> to globally disable TBAA, which is obviously a huge loss. This was never 
> motivated purely by the nonsensicality of the standardization of unions.

Okay, so the problem right now is that we're **not** conservative regarding 
union type punning, and while BasicAA sometimes fixes this for us, it doesn't 
always, and can't always, and so now we're miscompiling code.  Removing the 
TBAA from union accesses would make us conservative. In the long term, I think 
we agree that we need a better way of actually representing unions in our TBAA 
representation. The question is what to do in the short term. Maybe there's 
some less drastic short-term solution that addresses these test cases?

> (And the nonsensicality of the standard very much continues. The code that 
> we've all agreed is obviously being miscompiled here is still a strict 
> violation of the "improved" union rules in C++, and if we decide to treat it 
> as ill-formed we're going to break a massive amount of code (and vector code 
> in particular heavily uses unions), because trust me, nobody is reliably 
> placement-newing union fields when they're non-trivial. C++'s new rules are 
> unworkable in part because they rely on C++-specific features that cannot be 
> easily adopted in headers which need to be language-neutral.)

I think we all agree that the vector types need to have their scalar (element) 
types as parents in the current TBAA representation. That's a separate change 
that we should just make. That, unfortunately, only fixes a subset of the 
union-related miscompiles.

@rjmccall, if you agree that not adding TBAA to union accesses is reasonable at 
this stage, what do you think of this implementation of that goal?


Repository:
  rL LLVM

https://reviews.llvm.org/D31885



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

[PATCH] D31885: Remove TBAA information from LValues representing union members

[Hal Finkel via Phabricator via cfe-commits]

hfinkel added a comment.

In https://reviews.llvm.org/D31885#730728, @kparzysz wrote:

> In https://reviews.llvm.org/D31885#730038, @hfinkel wrote:
>
> > I'm somewhat concerned that this patch is quadratic in the AST.
>
>
> I'd be happy to address this, but I'm not sure how.  Memoizing results could 
> be one way, but don't know if that's acceptable.
>
> This location in codegen seems to be the last place where the original C/C++ 
> types are available, in particular the information as to whether a type is a 
> union or not.  Maybe it would be possible to propagate some bit somewhere, 
> but then this patch would become much less localized.


I'm not worried particularly about localization because we want to extend TBAA 
to support unions, etc. anyway. I suppose I don't understand how the 
propagation that is necessary here differs from what is necessary to propagate 
the base-type information for structs.


Repository:
  rL LLVM

https://reviews.llvm.org/D31885



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

[PATCH] D32237: [OpenMP] Prepare sema to support combined constructs with omp distribute and omp for

[Alexey Bataev via Phabricator via cfe-commits]

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

LG


Repository:
  rL LLVM

https://reviews.llvm.org/D32237



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

[PATCH] D32207: Corrrect warn_unused_result attribute

[Erich Keane via Phabricator via cfe-commits]

erichkeane updated this revision to Diff 95797.
erichkeane added a comment.

As discussed, removed the exception for postfix operators.  It seems like the 
right thing to do.I didn't add the [[nodiscard]] test, since it is the same 
intended behavior now.


https://reviews.llvm.org/D32207

Files:
  include/clang/AST/Decl.h
  lib/AST/Decl.cpp
  test/SemaCXX/warn-unused-result.cpp


Index: test/SemaCXX/warn-unused-result.cpp
===
--- test/SemaCXX/warn-unused-result.cpp
+++ test/SemaCXX/warn-unused-result.cpp
@@ -160,3 +160,42 @@
   (void)noexcept(f(), false); // Should not warn.
 }
 }
+
+namespace {
+// C++ Methods should warn even in their own class.
+struct[[clang::warn_unused_result]] S {
+  S DoThing() { return {}; };
+  S operator++(int) { return {}; };
+  S operator--(int) { return {}; };
+  // Improperly written prefix.
+  S operator++() { return {}; };
+  S operator--() { return {}; };
+};
+
+struct[[clang::warn_unused_result]] P {
+  P DoThing() { return {}; };
+};
+
+P operator++(const P &, int) { return {}; };
+P operator--(const P &, int) { return {}; };
+// Improperly written prefix.
+P operator++(const P &) { return {}; };
+P operator--(const P &) { return {}; };
+
+void f() {
+  S s;
+  P p;
+  s.DoThing(); // expected-warning {{ignoring return value}}
+  p.DoThing(); // expected-warning {{ignoring return value}}
+  // Only postfix is expected to warn when written correctly.
+  s++; // expected-warning {{ignoring return value}}
+  s--; // expected-warning {{ignoring return value}}
+  p++; // expected-warning {{ignoring return value}}
+  p--; // expected-warning {{ignoring return value}}
+  // Improperly written prefix operators should still warn.
+  ++s; // expected-warning {{ignoring return value}}
+  --s; // expected-warning {{ignoring return value}}
+  ++p; // expected-warning {{ignoring return value}}
+  --p; // expected-warning {{ignoring return value}}
+}
+} // namespace
Index: lib/AST/Decl.cpp
===
--- lib/AST/Decl.cpp
+++ lib/AST/Decl.cpp
@@ -3003,9 +3003,7 @@
 const Attr *FunctionDecl::getUnusedResultAttr() const {
   QualType RetType = getReturnType();
   if (RetType->isRecordType()) {
-const CXXRecordDecl *Ret = RetType->getAsCXXRecordDecl();
-const auto *MD = dyn_cast(this);
-if (Ret && !(MD && MD->getCorrespondingMethodInClass(Ret, true))) {
+if (const CXXRecordDecl *Ret = RetType->getAsCXXRecordDecl()) {
   if (const auto *R = Ret->getAttr())
 return R;
 }
Index: include/clang/AST/Decl.h
===
--- include/clang/AST/Decl.h
+++ include/clang/AST/Decl.h
@@ -2082,10 +2082,7 @@
   const Attr *getUnusedResultAttr() const;
 
   /// \brief Returns true if this function or its return type has the
-  /// warn_unused_result attribute. If the return type has the attribute and
-  /// this function is a method of the return type's class, then false will be
-  /// returned to avoid spurious warnings on member methods such as assignment
-  /// operators.
+  /// warn_unused_result attribute.
   bool hasUnusedResultAttr() const { return getUnusedResultAttr() != nullptr; }
 
   /// \brief Returns the storage class as written in the source. For the


Index: test/SemaCXX/warn-unused-result.cpp
===
--- test/SemaCXX/warn-unused-result.cpp
+++ test/SemaCXX/warn-unused-result.cpp
@@ -160,3 +160,42 @@
   (void)noexcept(f(), false); // Should not warn.
 }
 }
+
+namespace {
+// C++ Methods should warn even in their own class.
+struct[[clang::warn_unused_result]] S {
+  S DoThing() { return {}; };
+  S operator++(int) { return {}; };
+  S operator--(int) { return {}; };
+  // Improperly written prefix.
+  S operator++() { return {}; };
+  S operator--() { return {}; };
+};
+
+struct[[clang::warn_unused_result]] P {
+  P DoThing() { return {}; };
+};
+
+P operator++(const P &, int) { return {}; };
+P operator--(const P &, int) { return {}; };
+// Improperly written prefix.
+P operator++(const P &) { return {}; };
+P operator--(const P &) { return {}; };
+
+void f() {
+  S s;
+  P p;
+  s.DoThing(); // expected-warning {{ignoring return value}}
+  p.DoThing(); // expected-warning {{ignoring return value}}
+  // Only postfix is expected to warn when written correctly.
+  s++; // expected-warning {{ignoring return value}}
+  s--; // expected-warning {{ignoring return value}}
+  p++; // expected-warning {{ignoring return value}}
+  p--; // expected-warning {{ignoring return value}}
+  // Improperly written prefix operators should still warn.
+  ++s; // expected-warning {{ignoring return value}}
+  --s; // expected-warning {{ignoring return value}}
+  ++p; // expected-warning {{ignoring return value}}
+  --p; // expected-warning {{ignoring return value}}
+}
+} // namespace
Index: lib/AST/Decl.cpp
===

[PATCH] D31885: Remove TBAA information from LValues representing union members

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In https://reviews.llvm.org/D31885#730799, @dberlin wrote:

> In https://reviews.llvm.org/D31885#730766, @rjmccall wrote:
>
> > In https://reviews.llvm.org/D31885#730539, @dberlin wrote:
> >
> > > In https://reviews.llvm.org/D31885#730072, @rjmccall wrote:
> > >
> > > > Thanks for CC'ing me.  There are two problems here.
> > > >
> > > > The second is that our alias-analysis philosophy says that the fact 
> > > > that two accesses "sufficiently obviously" can alias should always 
> > > > override TBAA.
> > >
> > >
> > > I'm not sure i agree.
> > >  We have made explicit exceptions for parts of the standard that are 
> > > nonsensical (and they've been slowly updated in newer standards to be 
> > > less nonsensical).  Outside of that, i don't believe that is our 
> > > philosophy, nor would i believe it should be.
> > >  There is no "do what i mean" here we can define that makes sense.
> >
> >
> > There was a deliberate decision to make TBAA conservative about type 
> > punning in LLVM because, in practice, the strict form of TBAA you think we 
> > should follow has proven to be a failed optimization paradigm: it just 
> > leads users to globally disable TBAA, which is obviously a huge loss.   
> > This was never motivated purely by the nonsensicality of the 
> > standardization of unions.
>
>
> I think you have completely missed my point.  i have not said we should be 
> super strict,. We already aren't around unions. I"m fine with that. I'm 
> saying "there is no DWIM like you are advocating we can reasonably define 
> overall" and " whatever we do come up with, we can't do it in the backend".   
> I don't believe a philosophy of "we come up with a reasonable interpretation 
> and write it down" is a "failed paradigm" (it's also definitely not going to 
> be DWIM).Compilers already are pretty nice about TBAA relative to what 
> the standard says, as they must be.  People who want to abide turn it on, 
> those who don't, don't.  Most actually *do* turn it on, contrary to your 
> argument that it is a failed paradigm.


We have it on by default.  That has been acceptable to users precisely because 
we are conservative about punning.  In contrast, GCC adopted very strict rules 
about punning that led basically everyone to turn off strict aliasing.  That is 
what I am describing as a failed paradigm.

> You seem to be advocating for "if it's obvious they should alias, they should 
> alias", but that is not amenable to any real implementation either, and we 
> don't get whatever we thought was the right answer to this question right now 
> anyway!

That's our current implementation.  That's why TBAA delegates to BasicAA and 
allows it to override its judgement.  That is not an accident.

> So let me be concrete: How, precisely, do you propose we fix these bugs, such 
> that they work 100% of the time, and do not require the backend to understand 
> language rules?

I'm not saying the backend should magically understand language rules.  Clearly 
language rules should be abstractly represented in IR in a way that can be 
interpreted.  Case in point, we need to be representing the aliasability of 
vector types and their elements correctly in TBAA metadata.

I'm saying that it's sometimes okay for frontends to rely on an operational 
understanding of how the backend uses that information, along with major 
exceptions and overrides to its analysis.  For example, readonly/readnone have 
a basically behavioral semantics that doesn't actually constrain the operations 
carried out by the callee.  This kind of analysis is appropriate for situations 
like this where we want to be generally aggressive as long as any resulting 
miscompiles are reasonably defensible, which in practice means not miscompiling 
sufficiently obvious instances of type-punning.

Your proposal to we simply drop TBAA from all accesses related to unions is 
extremely conservative.  It means that an access through a union has to be 
treated as potentially aliasing every other visible access, at least in terms 
of their types.  That level of conservatism is not necessary in this case if we 
hew to the original understanding of LLVM's TBAA that punning is only allowed 
when it is sufficiently obvious.  If you are interested in making a strict 
definition of "sufficiently obvious", and then arguing that it cannot possibly 
work, we can have that conversation; but right now I'm not seeing anything that 
says that our approach is basically broken, other you repeatedly asserting it 
because you think TBAA should be capable of completely standing alone.

My proposal is that (1) we should correctly encode the aliasability of vector 
types in TBAA metadata and, separately, (2) we should also ensure that TBAA is 
correctly overridden by BasicAA in this case, which may require introducing an 
intermediate level of response between MayAlias and MustAlias.

John.


Repository:
  rL LLVM

https://reviews.llvm.org/D31885



___

[PATCH] D31975: [Analyzer] Iterator Checkers

[Anna Zaks via Phabricator via cfe-commits]

zaks.anna added a comment.

@baloghadamsoftware  Thanks for working on this!

However, this patch is getting too big. Could you, please, split it into 
incremental patches so that it would be easier to review? More motivation of 
why this is very important is here 
http://llvm.org/docs/DeveloperPolicy.html#incremental-development


https://reviews.llvm.org/D31975



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

[PATCH] D31885: Remove TBAA information from LValues representing union members

[Daniel Berlin via Phabricator via cfe-commits]

dberlin added a comment.



> Your proposal to we simply drop TBAA from all accesses related to unions is 
> extremely conservative.  It means that an access through a union has to be 
> treated as potentially aliasing every other visible access, at least in terms 
> of their types.  That level of conservatism is not necessary in this case if 
> we hew to the original understanding of LLVM's TBAA that punning is only 
> allowed when it is sufficiently obvious.  If you are interested in making a 
> strict definition of "sufficiently obvious", and then arguing that it cannot 
> possibly work, we can have that conversation; but right now I'm not seeing 
> anything that says that our approach is basically broken, other you 
> repeatedly asserting it because you think TBAA should be capable of 
> completely standing alone.

Did you look at the large number of bugs that prompted this patch and 
discussions on the mailing list?

> My proposal is that (1) we should correctly encode the aliasability of vector 
> types in TBAA metadata and, separately, (2) we should also ensure that TBAA 
> is correctly overridden by BasicAA in this case, which may require 
> introducing an intermediate level of response between MayAlias and MustAlias.

How do you see #2 working?
RIght now, it is overridden if TBAA says no-alias and basicaa says must-alias.

It's quite literally not possible for basicaa to detect all cases it should 
override.  If you want paper references on the strict breakdown of what can be 
decided statically, i'm happy to provide them.
Otherwise, there are plenty of bugs filed with real-world examples already.
So it doesn't now, it can't in the future.  This is the source of a number of 
those bugs.
If BasicAA could prove no-alias, it wouldn't' need to override, so you can't 
just override any may-alias cases.  It can't detect all must-aliasing, or even 
anything approaching them.  
It doesn't work in probabilities, and llvm's type system is unrelated to the C 
level one, so it can't even flag those things that might be "bad", so there is 
nothing it can answer in between.
This opposed to doing it in the front end, where it knows *all* of this, and 
could simply stop telling the backend two things don't alias when you want it 
to do that.

So i'd like to understand how you see it "correctly overriding in BasicAA". 
There is *nothing*, in any of these accesses or examples, at the LLVM IR level, 
that tells us we should override it.


Repository:
  rL LLVM

https://reviews.llvm.org/D31885



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

[PATCH] D31885: Remove TBAA information from LValues representing union members

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In https://reviews.llvm.org/D31885#730853, @hfinkel wrote:

> > There was a deliberate decision to make TBAA conservative about type 
> > punning in LLVM because, in practice, the strict form of TBAA you think we 
> > should follow has proven to be a failed optimization paradigm: it just 
> > leads users to globally disable TBAA, which is obviously a huge loss. This 
> > was never motivated purely by the nonsensicality of the standardization of 
> > unions.
>
> Okay, so the problem right now is that we're **not** conservative regarding 
> union type punning, and while BasicAA sometimes fixes this for us, it doesn't 
> always, and can't always, and so now we're miscompiling code.


I agree that BasicAA "can't always", but I'm not sure it "can't often enough".  
Again, it seems to me that the low-level problem is just that the AA interface 
isn't designed for what we're trying to do with TBAA.  "MayAlias" is the 
default answer for everything that can't be proven one way or the other, and 
"MustAlias" demands that the memory be actually known to overlap.  If there 
were an intermediate "LikelyAlias" for pointers that are known to be related 
but BasicAA just doesn't know for certain whether the accesses overlap, then 
TBAA would turn itself off in those cases as long as at least a basic 
value-propagation pass has been run.  That would put us on much firmer ground 
to say "it's reasonable for the compiler to assume that these things don't 
alias, and if you're going to use type-punning like this, you just need to 
rewrite your code to make it more obvious that the pointers are related".  When 
you're given a language rule as weak as this, especially one that's so 
frequently violated, that's the only kind of argument which is going to have 
any traction with users.

John.


Repository:
  rL LLVM

https://reviews.llvm.org/D31885



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

[PATCH] D31885: Remove TBAA information from LValues representing union members

[Daniel Berlin via Phabricator via cfe-commits]

dberlin added a comment.

In https://reviews.llvm.org/D31885#730909, @rjmccall wrote:

> In https://reviews.llvm.org/D31885#730853, @hfinkel wrote:
>
> > > There was a deliberate decision to make TBAA conservative about type 
> > > punning in LLVM because, in practice, the strict form of TBAA you think 
> > > we should follow has proven to be a failed optimization paradigm: it just 
> > > leads users to globally disable TBAA, which is obviously a huge loss. 
> > > This was never motivated purely by the nonsensicality of the 
> > > standardization of unions.
> >
> > Okay, so the problem right now is that we're **not** conservative regarding 
> > union type punning, and while BasicAA sometimes fixes this for us, it 
> > doesn't always, and can't always, and so now we're miscompiling code.
>
>
> I agree that BasicAA "can't always", but I'm not sure it "can't often 
> enough".  Again, it seems to me that the low-level problem is just that the 
> AA interface isn't designed for what we're trying to do with TBAA.  
> "MayAlias" is the default answer for everything that can't be proven one way 
> or the other, and "MustAlias" demands that the memory be actually known to 
> overlap.  If there were an intermediate "LikelyAlias" for pointers that are 
> known to be related but BasicAA just doesn't know for certain whether the 
> accesses overlap, then TBAA would turn itself off in those cases as long as 
> at least a basic value-propagation pass has been run.  That would put us on 
> much firmer ground to say "it's reasonable for the compiler to assume that 
> these things don't alias, and if you're going to use type-punning like this, 
> you just need to rewrite your code to make it more obvious that the pointers 
> are related".  When you're given a language rule as weak as this, especially 
> one that's so frequently violated, that's the only kind of argument which is 
> going to have any traction with users.
>
> John.




In https://reviews.llvm.org/D31885#730909, @rjmccall wrote:

> In https://reviews.llvm.org/D31885#730853, @hfinkel wrote:
>
> > > There was a deliberate decision to make TBAA conservative about type 
> > > punning in LLVM because, in practice, the strict form of TBAA you think 
> > > we should follow has proven to be a failed optimization paradigm: it just 
> > > leads users to globally disable TBAA, which is obviously a huge loss. 
> > > This was never motivated purely by the nonsensicality of the 
> > > standardization of unions.
> >
> > Okay, so the problem right now is that we're **not** conservative regarding 
> > union type punning, and while BasicAA sometimes fixes this for us, it 
> > doesn't always, and can't always, and so now we're miscompiling code.
>
>
> I agree that BasicAA "can't always", but I'm not sure it "can't often 
> enough".  Again, it seems to me that the low-level problem is just that the 
> AA interface isn't designed for what we're trying to do with TBAA.  
> "MayAlias" is the default answer for everything that can't be proven one way 
> or the other, and "MustAlias" demands that the memory be actually known to 
> overlap.  If there were an intermediate "LikelyAlias" for pointers that are 
> known to be related but BasicAA just doesn't know for certain whether the 
> accesses overlap, then TBAA would turn itself off in those cases as long as 
> at least a basic value-propagation pass has been run.  That would put us on 
> much firmer ground to say "it's reasonable for the compiler to assume that 
> these things don't alias, and if you're going to use type-punning like this, 
> you just need to rewrite your code to make it more obvious that the pointers 
> are related".  When you're given a language rule as weak as this, especially 
> one that's so frequently violated, that's the only kind of argument which is 
> going to have any traction with users.
>
> John.


Just so i understand: Ignoring everything else (we can't actually make 
likelyalias work, i think the code in the bugs makes that very clear), you also 
believe we should effectively pessimize every other language that generates 
correct TBAA info for LLVM  and will now no longer get optimized well because 
we've decided not to have clang emit TBAA metadata only in the cases where  it 
actually wants TBAA applied?


Repository:
  rL LLVM

https://reviews.llvm.org/D31885



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

[PATCH] D32207: Corrrect warn_unused_result attribute

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

I'm surprised this change didn't cause any other tests to need to be updated. A 
few small formatting nits and a request for another test, but otherwise looking 
good.




Comment at: test/SemaCXX/warn-unused-result.cpp:166
+// C++ Methods should warn even in their own class.
+struct[[clang::warn_unused_result]] S {
+  S DoThing() { return {}; };

Add a space between `struct` and the attribute introducer.



Comment at: test/SemaCXX/warn-unused-result.cpp:175
+
+struct[[clang::warn_unused_result]] P {
+  P DoThing() { return {}; };

Same here.



Comment at: test/SemaCXX/warn-unused-result.cpp:200
+  --p; // expected-warning {{ignoring return value}}
+}
+} // namespace

Can you add some tests showing that casting to void still silence the 
diagnostic even for these operator cases? It doesn't need to happen for all of 
them, but it's good to ensure that still behaves as expected.


https://reviews.llvm.org/D32207



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

[PATCH] D31885: Remove TBAA information from LValues representing union members

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In https://reviews.llvm.org/D31885#730920, @dberlin wrote:

> Just so i understand: Ignoring everything else (we can't actually make 
> likelyalias work, i think the code in the bugs makes that very clear),


None of the code in the bugs I've seen makes that very clear, but perhaps I 
just missed the compelling example.

> you also believe we should effectively pessimize every other language that 
> generates correct TBAA info for LLVM  and will now no longer get optimized 
> well because we've decided not to have clang emit TBAA metadata only in the 
> cases where  it actually wants TBAA applied?

Are you under the impression that I'm proposing something new and that TBAA 
does not currently defer to BasicAA?

Clang really does want TBAA applied to unions.  I want LLVM to know that an 
access to a float through a union doesn't alias an arbitrary int* argument, 
even if the union includes an int.

Anyway, TBAA tends to be less important in other languages, which tend to make 
much stronger non-type-based aliasing assumptions: functional languages assert 
that values are immutable, Fortran knows that arrays don't alias, Rust knows 
when references are unique, etc.

John.


Repository:
  rL LLVM

https://reviews.llvm.org/D31885



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

[PATCH] D32238: [Clangd] Failed to decode params using 1.x-compatible request message

[Benjamin Kramer via Phabricator via cfe-commits]

bkramer added a comment.

A test case would be nice.


Repository:
  rL LLVM

https://reviews.llvm.org/D32238



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

[PATCH] D32234: [Clangd] Support Authority-less URIs

[Benjamin Kramer via Phabricator via cfe-commits]

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

This is fine. Test case would be nice though.


Repository:
  rL LLVM

https://reviews.llvm.org/D32234



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

[PATCH] D31885: Remove TBAA information from LValues representing union members

[Daniel Berlin via Phabricator via cfe-commits]

dberlin added a comment.

In https://reviews.llvm.org/D31885#730936, @rjmccall wrote:

> In https://reviews.llvm.org/D31885#730920, @dberlin wrote:
>
> > Just so i understand: Ignoring everything else (we can't actually make 
> > likelyalias work, i think the code in the bugs makes that very clear),
>
>
> None of the code in the bugs I've seen makes that very clear, but perhaps I 
> just missed the compelling example.
>
> > you also believe we should effectively pessimize every other language that 
> > generates correct TBAA info for LLVM  and will now no longer get optimized 
> > well because we've decided not to have clang emit TBAA metadata only in the 
> > cases where  it actually wants TBAA applied?
>
> Are you under the impression that I'm proposing something new and that TBAA 
> does not currently defer to BasicAA?


Well, actually, chandler changed how it works, so it doesn't actually work 
quite the way it used to but it has the same effect ;)
(It used to be explicit deferral, it is not explicit, neither depends on the 
other any more.  It just happens to be how the default AA pipeline is ordered 
right now)
I'm quite aware of the machinations of AA in LLVM :)

I'm saying "this approach does not work well now" (ie see bugs) , and i don't 
think continuing to try to make the contract even *more* incestuous is better 
than trying to make it *less*.

I see literally no advantage to doing so, and plenty of disadvantage 
(pessimization of other languages, continuation of a pipeline ordering that 
requires overrides, etc)

IE this is a thing we should be fixing, over time, to be a cleaner and better 
design, that does not make clang special here, and fixes these bugs.
Others generate tbaa metadata where they want it, and avoid it where they want 
basicaa to make a decision.

I have trouble seeing, why, if folks are willing to do that work, one would be 
opposed to it.
You seem to claim better optimization opportunity.
I ran numbers.
Across all things in llvm's benchmark suite, disabling unions entirely for tbaa 
causes no regressions on x86 (I could try more platforms if you like).

I also  ran it across 50 third party packages and spec2006 that use strict 
aliasing (These are just those things i could quickly test that had perf 
benchmarks)

If it really caused any real perf loss, we could always define metadata to say 
these accesses are noalias if we can't prove they are mustalias.
We could even use that metadata to explicitly try to have basicaa try much 
harder than it does now to prove must-aliasing (for example, ignore depth 
limits), since it would likely be too expensive to try it for everything.

as an aside, I do not believe you could reasonably apply this to unions  
because it's not possible to tell people what to do differently when it's *llvm 
ir* that has the issue.

IE i can't reasonably tell users "please stop doing things that don't generate 
explicit enough IR".
Especially when those answers change with inlining decisions, etc.

If the frontend did it, i would likely have something reasonably to tell them.


Repository:
  rL LLVM

https://reviews.llvm.org/D31885



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

r300738 - [sanitizer-coverage] deprecate some of the stale coverage variants

[Kostya Serebryany via cfe-commits]

Author: kcc
Date: Wed Apr 19 14:57:16 2017
New Revision: 300738

URL: http://llvm.org/viewvc/llvm-project?rev=300738&view=rev
Log:
[sanitizer-coverage] deprecate some of the stale coverage variants

Modified:
cfe/trunk/docs/SanitizerCoverage.rst
cfe/trunk/lib/Driver/SanitizerArgs.cpp
cfe/trunk/test/Driver/fsanitize-coverage.c

Modified: cfe/trunk/docs/SanitizerCoverage.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/SanitizerCoverage.rst?rev=300738&r1=300737&r2=300738&view=diff
==
--- cfe/trunk/docs/SanitizerCoverage.rst (original)
+++ cfe/trunk/docs/SanitizerCoverage.rst Wed Apr 19 14:57:16 2017
@@ -202,27 +202,7 @@ edges by introducing new dummy blocks an
 Bitset
 ==
 
-When ``coverage_bitset=1`` run-time flag is given, the coverage will also be
-dumped as a bitset (text file with 1 for blocks that have been executed and 0
-for blocks that were not).
-
-.. code-block:: console
-
-% clang++ -fsanitize=address -fsanitize-coverage=edge cov.cc
-% ASAN_OPTIONS="coverage=1:coverage_bitset=1" ./a.out
-main
-% ASAN_OPTIONS="coverage=1:coverage_bitset=1" ./a.out 1
-foo
-main
-% head *bitset*
-==> a.out.38214.bitset-sancov <==
-01101
-==> a.out.6128.bitset-sancov <==
-11011%
-
-For a given executable the length of the bitset is always the same (well,
-unless dlopen/dlclose come into play), so the bitset coverage can be
-easily used for bitset-based corpus distillation.
+**coverage_bitset=1 is deprecated, don't use**
 
 Caller-callee coverage
 ==
@@ -326,8 +306,6 @@ Basic block tracing is currently support
 Tracing PCs
 ===
 
-**Deprecated, don't use**
-
 *Experimental* feature similar to tracing basic blocks, but with a different 
API.
 With ``-fsanitize-coverage=trace-pc`` the compiler will insert
 ``__sanitizer_cov_trace_pc()`` on every edge.

Modified: cfe/trunk/lib/Driver/SanitizerArgs.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/SanitizerArgs.cpp?rev=300738&r1=300737&r2=300738&view=diff
==
--- cfe/trunk/lib/Driver/SanitizerArgs.cpp (original)
+++ cfe/trunk/lib/Driver/SanitizerArgs.cpp Wed Apr 19 14:57:16 2017
@@ -469,34 +469,12 @@ SanitizerArgs::SanitizerArgs(const ToolC
   int LegacySanitizeCoverage;
   if (Arg->getNumValues() == 1 &&
   !StringRef(Arg->getValue(0))
-   .getAsInteger(0, LegacySanitizeCoverage) &&
-  LegacySanitizeCoverage >= 0 && LegacySanitizeCoverage <= 4) {
-switch (LegacySanitizeCoverage) {
-case 0:
-  CoverageFeatures = 0;
-  Arg->claim();
-  break;
-case 1:
-  D.Diag(diag::warn_drv_deprecated_arg) << Arg->getAsString(Args)
-<< "-fsanitize-coverage=func";
-  CoverageFeatures = CoverageFunc;
-  break;
-case 2:
-  D.Diag(diag::warn_drv_deprecated_arg) << Arg->getAsString(Args)
-<< "-fsanitize-coverage=bb";
-  CoverageFeatures = CoverageBB;
-  break;
-case 3:
-  D.Diag(diag::warn_drv_deprecated_arg) << Arg->getAsString(Args)
-<< "-fsanitize-coverage=edge";
-  CoverageFeatures = CoverageEdge;
-  break;
-case 4:
+   .getAsInteger(0, LegacySanitizeCoverage)) {
+CoverageFeatures = 0;
+Arg->claim();
+if (LegacySanitizeCoverage != 0) {
   D.Diag(diag::warn_drv_deprecated_arg)
-  << Arg->getAsString(Args)
-  << "-fsanitize-coverage=edge,indirect-calls";
-  CoverageFeatures = CoverageEdge | CoverageIndirCall;
-  break;
+  << Arg->getAsString(Args) << 
"-fsanitize-coverage=trace-pc-guard";
 }
 continue;
   }

Modified: cfe/trunk/test/Driver/fsanitize-coverage.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/fsanitize-coverage.c?rev=300738&r1=300737&r2=300738&view=diff
==
--- cfe/trunk/test/Driver/fsanitize-coverage.c (original)
+++ cfe/trunk/test/Driver/fsanitize-coverage.c Wed Apr 19 14:57:16 2017
@@ -23,14 +23,7 @@
 // CHECK-SANITIZE-COVERAGE-FUNC_INDIR: fsanitize-coverage-indirect-calls
 
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=address 
-fsanitize-coverage=1 %s -### 2>&1 | FileCheck %s 
--check-prefix=CHECK-SANITIZE-COVERAGE-1
-// CHECK-SANITIZE-COVERAGE-1: warning: argument '-fsanitize-coverage=1' is 
deprecated, use '-fsanitize-coverage=func' instead
-// RUN: %clang -target x86_64-linux-gnu -fsanitize=address 
-fsanitize-coverage=2 %s -### 2>&1 | FileCheck %s 
--check-prefix=CHECK-SANITIZE-COVERAGE-2
-// CHECK-SANITIZE-COVERAGE-2: warning: argument '-fsanitize-coverage=2' is 
deprecated, u

[PATCH] D32207: Corrrect warn_unused_result attribute

[Erich Keane via Phabricator via cfe-commits]

erichkeane updated this revision to Diff 95807.
erichkeane marked 3 inline comments as done.
erichkeane added a comment.

Added the tests, plus the two formatting changes.


https://reviews.llvm.org/D32207

Files:
  include/clang/AST/Decl.h
  lib/AST/Decl.cpp
  test/SemaCXX/warn-unused-result.cpp


Index: test/SemaCXX/warn-unused-result.cpp
===
--- test/SemaCXX/warn-unused-result.cpp
+++ test/SemaCXX/warn-unused-result.cpp
@@ -160,3 +160,49 @@
   (void)noexcept(f(), false); // Should not warn.
 }
 }
+
+namespace {
+// C++ Methods should warn even in their own class.
+struct [[clang::warn_unused_result]] S {
+  S DoThing() { return {}; };
+  S operator++(int) { return {}; };
+  S operator--(int) { return {}; };
+  // Improperly written prefix.
+  S operator++() { return {}; };
+  S operator--() { return {}; };
+};
+
+struct [[clang::warn_unused_result]] P {
+  P DoThing() { return {}; };
+};
+
+P operator++(const P &, int) { return {}; };
+P operator--(const P &, int) { return {}; };
+// Improperly written prefix.
+P operator++(const P &) { return {}; };
+P operator--(const P &) { return {}; };
+
+void f() {
+  S s;
+  P p;
+  s.DoThing(); // expected-warning {{ignoring return value}}
+  p.DoThing(); // expected-warning {{ignoring return value}}
+  // Only postfix is expected to warn when written correctly.
+  s++; // expected-warning {{ignoring return value}}
+  s--; // expected-warning {{ignoring return value}}
+  p++; // expected-warning {{ignoring return value}}
+  p--; // expected-warning {{ignoring return value}}
+  // Improperly written prefix operators should still warn.
+  ++s; // expected-warning {{ignoring return value}}
+  --s; // expected-warning {{ignoring return value}}
+  ++p; // expected-warning {{ignoring return value}}
+  --p; // expected-warning {{ignoring return value}}
+
+  // Silencing the warning by cast to void still works.
+  (void)s.DoThing();
+  (void)s++;
+  (void)p++;
+  (void)++s;
+  (void)++p;
+}
+} // namespace
Index: lib/AST/Decl.cpp
===
--- lib/AST/Decl.cpp
+++ lib/AST/Decl.cpp
@@ -3003,9 +3003,7 @@
 const Attr *FunctionDecl::getUnusedResultAttr() const {
   QualType RetType = getReturnType();
   if (RetType->isRecordType()) {
-const CXXRecordDecl *Ret = RetType->getAsCXXRecordDecl();
-const auto *MD = dyn_cast(this);
-if (Ret && !(MD && MD->getCorrespondingMethodInClass(Ret, true))) {
+if (const CXXRecordDecl *Ret = RetType->getAsCXXRecordDecl()) {
   if (const auto *R = Ret->getAttr())
 return R;
 }
Index: include/clang/AST/Decl.h
===
--- include/clang/AST/Decl.h
+++ include/clang/AST/Decl.h
@@ -2082,10 +2082,7 @@
   const Attr *getUnusedResultAttr() const;
 
   /// \brief Returns true if this function or its return type has the
-  /// warn_unused_result attribute. If the return type has the attribute and
-  /// this function is a method of the return type's class, then false will be
-  /// returned to avoid spurious warnings on member methods such as assignment
-  /// operators.
+  /// warn_unused_result attribute.
   bool hasUnusedResultAttr() const { return getUnusedResultAttr() != nullptr; }
 
   /// \brief Returns the storage class as written in the source. For the


Index: test/SemaCXX/warn-unused-result.cpp
===
--- test/SemaCXX/warn-unused-result.cpp
+++ test/SemaCXX/warn-unused-result.cpp
@@ -160,3 +160,49 @@
   (void)noexcept(f(), false); // Should not warn.
 }
 }
+
+namespace {
+// C++ Methods should warn even in their own class.
+struct [[clang::warn_unused_result]] S {
+  S DoThing() { return {}; };
+  S operator++(int) { return {}; };
+  S operator--(int) { return {}; };
+  // Improperly written prefix.
+  S operator++() { return {}; };
+  S operator--() { return {}; };
+};
+
+struct [[clang::warn_unused_result]] P {
+  P DoThing() { return {}; };
+};
+
+P operator++(const P &, int) { return {}; };
+P operator--(const P &, int) { return {}; };
+// Improperly written prefix.
+P operator++(const P &) { return {}; };
+P operator--(const P &) { return {}; };
+
+void f() {
+  S s;
+  P p;
+  s.DoThing(); // expected-warning {{ignoring return value}}
+  p.DoThing(); // expected-warning {{ignoring return value}}
+  // Only postfix is expected to warn when written correctly.
+  s++; // expected-warning {{ignoring return value}}
+  s--; // expected-warning {{ignoring return value}}
+  p++; // expected-warning {{ignoring return value}}
+  p--; // expected-warning {{ignoring return value}}
+  // Improperly written prefix operators should still warn.
+  ++s; // expected-warning {{ignoring return value}}
+  --s; // expected-warning {{ignoring return value}}
+  ++p; // expected-warning {{ignoring return value}}
+  --p; // expected-warning {{ignoring return value}}
+
+  // Silencing the war

r300741 - Parse backend options during thinlto backend compile actions

[David Blaikie via cfe-commits]

Author: dblaikie
Date: Wed Apr 19 15:08:21 2017
New Revision: 300741

URL: http://llvm.org/viewvc/llvm-project?rev=300741&view=rev
Log:
Parse backend options during thinlto backend compile actions

Added:
cfe/trunk/test/CodeGen/thinlto-backend-option.ll
Modified:
cfe/trunk/lib/CodeGen/BackendUtil.cpp

Modified: cfe/trunk/lib/CodeGen/BackendUtil.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/BackendUtil.cpp?rev=300741&r1=300740&r2=300741&view=diff
==
--- cfe/trunk/lib/CodeGen/BackendUtil.cpp (original)
+++ cfe/trunk/lib/CodeGen/BackendUtil.cpp Wed Apr 19 15:08:21 2017
@@ -83,9 +83,6 @@ class EmitAssemblyHelper {
 return TargetIRAnalysis();
   }
 
-  /// Set LLVM command line options passed through -backend-option.
-  void setCommandLineOpts();
-
   void CreatePasses(legacy::PassManager &MPM, legacy::FunctionPassManager 
&FPM);
 
   /// Generates the TargetMachine.
@@ -604,7 +601,7 @@ void EmitAssemblyHelper::CreatePasses(le
   PMBuilder.populateModulePassManager(MPM);
 }
 
-void EmitAssemblyHelper::setCommandLineOpts() {
+static void setCommandLineOpts(const CodeGenOptions &CodeGenOpts) {
   SmallVector BackendArgs;
   BackendArgs.push_back("clang"); // Fake program name.
   if (!CodeGenOpts.DebugPass.empty()) {
@@ -677,7 +674,7 @@ void EmitAssemblyHelper::EmitAssembly(Ba
   std::unique_ptr OS) {
   TimeRegion Region(llvm::TimePassesIsEnabled ? &CodeGenerationTime : nullptr);
 
-  setCommandLineOpts();
+  setCommandLineOpts(CodeGenOpts);
 
   bool UsesCodeGen = (Action != Backend_EmitNothing &&
   Action != Backend_EmitBC &&
@@ -806,7 +803,7 @@ static PassBuilder::OptimizationLevel ma
 void EmitAssemblyHelper::EmitAssemblyWithNewPassManager(
 BackendAction Action, std::unique_ptr OS) {
   TimeRegion Region(llvm::TimePassesIsEnabled ? &CodeGenerationTime : nullptr);
-  setCommandLineOpts();
+  setCommandLineOpts(CodeGenOpts);
 
   // The new pass manager always makes a target machine available to passes
   // during construction.
@@ -944,6 +941,8 @@ static void runThinLTOBackend(ModuleSumm
   ModuleToDefinedGVSummaries;
   
CombinedIndex->collectDefinedGVSummariesPerModule(ModuleToDefinedGVSummaries);
 
+  setCommandLineOpts(CGOpts);
+
   // We can simply import the values mentioned in the combined index, since
   // we should only invoke this using the individual indexes written out
   // via a WriteIndexesThinBackend.

Added: cfe/trunk/test/CodeGen/thinlto-backend-option.ll
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/thinlto-backend-option.ll?rev=300741&view=auto
==
--- cfe/trunk/test/CodeGen/thinlto-backend-option.ll (added)
+++ cfe/trunk/test/CodeGen/thinlto-backend-option.ll Wed Apr 19 15:08:21 2017
@@ -0,0 +1,13 @@
+; Test to ensure -backend-options work when invoking the ThinLTO backend path.
+
+; This test uses a non-existent backend option to test that backend options are
+; being parsed. While it's more important that the existing options are parsed
+; than that this error is produced, this provides a reliable way to test this
+; scenario independent of any particular backend options that may exist now or
+; in the future.
+
+; RUN: %clang -flto=thin -c -o %t.o %s
+; RUN: llvm-lto -thinlto -o %t %t.o
+; RUN: not %clang_cc1 -x ir %t.o -fthinlto-index=%t.thinlto.bc -backend-option 
-nonexistent -emit-obj -o /dev/null 2>&1 | FileCheck %s
+
+; CHECK: clang: Unknown command line argument '-nonexistent'


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

[PATCH] D32207: Corrrect warn_unused_result attribute

[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!


https://reviews.llvm.org/D32207



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

[libcxx] r300743 - [libc++] Use _LIBCPP_ABI_MICROSOFT instead of _MSC_VER

[Shoaib Meenai via cfe-commits]

Author: smeenai
Date: Wed Apr 19 15:11:04 2017
New Revision: 300743

URL: http://llvm.org/viewvc/llvm-project?rev=300743&view=rev
Log:
[libc++] Use _LIBCPP_ABI_MICROSOFT instead of _MSC_VER

_LIBCPP_ABI_MICROSOFT is more appropriate to use here, since the
conditionals are controlling Microsoft mangling. It wasn't used
originally since it didn't exist at the time.

Modified:
libcxx/trunk/src/iostream.cpp

Modified: libcxx/trunk/src/iostream.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/src/iostream.cpp?rev=300743&r1=300742&r2=300743&view=diff
==
--- libcxx/trunk/src/iostream.cpp (original)
+++ libcxx/trunk/src/iostream.cpp Wed Apr 19 15:11:04 2017
@@ -15,14 +15,14 @@ _LIBCPP_BEGIN_NAMESPACE_STD
 
 #ifndef _LIBCPP_HAS_NO_STDIN
 _ALIGNAS_TYPE (istream) _LIBCPP_FUNC_VIS char cin[sizeof(istream)]
-#if defined(_MSC_VER) && defined(__clang__)
+#if defined(_LIBCPP_ABI_MICROSOFT) && defined(__clang__)
 __asm__("?cin@__1@std@@3V?$basic_istream@DU?$char_traits@D@__1@std@@@12@A")
 #endif
 ;
 _ALIGNAS_TYPE (__stdinbuf ) static char __cin[sizeof(__stdinbuf )];
 static mbstate_t mb_cin;
 _ALIGNAS_TYPE (wistream) _LIBCPP_FUNC_VIS char wcin[sizeof(wistream)]
-#if defined(_MSC_VER) && defined(__clang__)
+#if defined(_LIBCPP_ABI_MICROSOFT) && defined(__clang__)
 __asm__("?wcin@__1@std@@3V?$basic_istream@_WU?$char_traits@_W@__1@std@@@12@A")
 #endif
 ;
@@ -32,14 +32,14 @@ static mbstate_t mb_wcin;
 
 #ifndef _LIBCPP_HAS_NO_STDOUT
 _ALIGNAS_TYPE (ostream) _LIBCPP_FUNC_VIS char cout[sizeof(ostream)]
-#if defined(_MSC_VER) && defined(__clang__)
+#if defined(_LIBCPP_ABI_MICROSOFT) && defined(__clang__)
 __asm__("?cout@__1@std@@3V?$basic_ostream@DU?$char_traits@D@__1@std@@@12@A")
 #endif
 ;
 _ALIGNAS_TYPE (__stdoutbuf) static char 
__cout[sizeof(__stdoutbuf)];
 static mbstate_t mb_cout;
 _ALIGNAS_TYPE (wostream) _LIBCPP_FUNC_VIS char wcout[sizeof(wostream)]
-#if defined(_MSC_VER) && defined(__clang__)
+#if defined(_LIBCPP_ABI_MICROSOFT) && defined(__clang__)
 __asm__("?wcout@__1@std@@3V?$basic_ostream@_WU?$char_traits@_W@__1@std@@@12@A")
 #endif
 ;
@@ -48,14 +48,14 @@ static mbstate_t mb_wcout;
 #endif
 
 _ALIGNAS_TYPE (ostream) _LIBCPP_FUNC_VIS char cerr[sizeof(ostream)]
-#if defined(_MSC_VER) && defined(__clang__)
+#if defined(_LIBCPP_ABI_MICROSOFT) && defined(__clang__)
 __asm__("?cerr@__1@std@@3V?$basic_ostream@DU?$char_traits@D@__1@std@@@12@A")
 #endif
 ;
 _ALIGNAS_TYPE (__stdoutbuf) static char 
__cerr[sizeof(__stdoutbuf)];
 static mbstate_t mb_cerr;
 _ALIGNAS_TYPE (wostream) _LIBCPP_FUNC_VIS char wcerr[sizeof(wostream)]
-#if defined(_MSC_VER) && defined(__clang__)
+#if defined(_LIBCPP_ABI_MICROSOFT) && defined(__clang__)
 __asm__("?wcerr@__1@std@@3V?$basic_ostream@_WU?$char_traits@_W@__1@std@@@12@A")
 #endif
 ;
@@ -63,12 +63,12 @@ _ALIGNAS_TYPE (__stdoutbuf) sta
 static mbstate_t mb_wcerr;
 
 _ALIGNAS_TYPE (ostream) _LIBCPP_FUNC_VIS char clog[sizeof(ostream)]
-#if defined(_MSC_VER) && defined(__clang__)
+#if defined(_LIBCPP_ABI_MICROSOFT) && defined(__clang__)
 __asm__("?clog@__1@std@@3V?$basic_ostream@DU?$char_traits@D@__1@std@@@12@A")
 #endif
 ;
 _ALIGNAS_TYPE (wostream) _LIBCPP_FUNC_VIS char wclog[sizeof(wostream)]
-#if defined(_MSC_VER) && defined(__clang__)
+#if defined(_LIBCPP_ABI_MICROSOFT) && defined(__clang__)
 __asm__("?wclog@__1@std@@3V?$basic_ostream@_WU?$char_traits@_W@__1@std@@@12@A")
 #endif
 ;


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

r300744 - [sanitizer-coverage] deprecate -fsanitize-coverage=8bit-counters

[Kostya Serebryany via cfe-commits]

Author: kcc
Date: Wed Apr 19 15:15:58 2017
New Revision: 300744

URL: http://llvm.org/viewvc/llvm-project?rev=300744&view=rev
Log:
[sanitizer-coverage] deprecate -fsanitize-coverage=8bit-counters

Modified:
cfe/trunk/lib/Driver/SanitizerArgs.cpp
cfe/trunk/test/Driver/fsanitize-coverage.c

Modified: cfe/trunk/lib/Driver/SanitizerArgs.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/SanitizerArgs.cpp?rev=300744&r1=300743&r2=300744&view=diff
==
--- cfe/trunk/lib/Driver/SanitizerArgs.cpp (original)
+++ cfe/trunk/lib/Driver/SanitizerArgs.cpp Wed Apr 19 15:15:58 2017
@@ -513,11 +513,10 @@ SanitizerArgs::SanitizerArgs(const ToolC
 D.Diag(clang::diag::err_drv_argument_only_allowed_with)
 << "-fsanitize-coverage=trace-bb"
 << "-fsanitize-coverage=(func|bb|edge)";
-  if ((CoverageFeatures & Coverage8bitCounters) &&
-  !(CoverageFeatures & CoverageTypes))
-D.Diag(clang::diag::err_drv_argument_only_allowed_with)
+  if ((CoverageFeatures & Coverage8bitCounters))
+D.Diag(clang::diag::warn_drv_deprecated_arg)
 << "-fsanitize-coverage=8bit-counters"
-<< "-fsanitize-coverage=(func|bb|edge)";
+<< "-fsanitize-coverage=trace-pc-guard";
   // trace-pc w/o func/bb/edge implies edge.
   if ((CoverageFeatures & (CoverageTracePC | CoverageTracePCGuard)) &&
   !(CoverageFeatures & CoverageTypes))

Modified: cfe/trunk/test/Driver/fsanitize-coverage.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/fsanitize-coverage.c?rev=300744&r1=300743&r2=300744&view=diff
==
--- cfe/trunk/test/Driver/fsanitize-coverage.c (original)
+++ cfe/trunk/test/Driver/fsanitize-coverage.c Wed Apr 19 15:15:58 2017
@@ -33,14 +33,13 @@
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=address 
-fsanitize-coverage=func -fno-sanitize=address %s -### 2>&1 | FileCheck %s 
--check-prefix=CHECK-SANITIZE-COVERAGE-SAN-DISABLED
 // CHECK-SANITIZE-COVERAGE-SAN-DISABLED-NOT: argument unused
 
-// RUN: %clang -target x86_64-linux-gnu -fsanitize=address 
-fsanitize-coverage=edge,indirect-calls,trace-bb,trace-pc,trace-cmp,8bit-counters,trace-div,trace-gep
 %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SANITIZE-COVERAGE-FEATURES
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=address 
-fsanitize-coverage=edge,indirect-calls,trace-bb,trace-pc,trace-cmp,trace-div,trace-gep
 %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SANITIZE-COVERAGE-FEATURES
 // CHECK-SANITIZE-COVERAGE-FEATURES: -fsanitize-coverage-type=3
 // CHECK-SANITIZE-COVERAGE-FEATURES: -fsanitize-coverage-indirect-calls
 // CHECK-SANITIZE-COVERAGE-FEATURES: -fsanitize-coverage-trace-bb
 // CHECK-SANITIZE-COVERAGE-FEATURES: -fsanitize-coverage-trace-cmp
 // CHECK-SANITIZE-COVERAGE-FEATURES: -fsanitize-coverage-trace-div
 // CHECK-SANITIZE-COVERAGE-FEATURES: -fsanitize-coverage-trace-gep
-// CHECK-SANITIZE-COVERAGE-FEATURES: -fsanitize-coverage-8bit-counters
 // CHECK-SANITIZE-COVERAGE-FEATURES: -fsanitize-coverage-trace-pc
 
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=address 
-fsanitize-coverage=func,edge,indirect-calls,trace-bb,trace-cmp 
-fno-sanitize-coverage=edge,indirect-calls,trace-bb %s -### 2>&1 | FileCheck %s 
--check-prefix=CHECK-MASK
@@ -54,8 +53,8 @@
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=address 
-fsanitize-coverage=func -fsanitize-coverage=edge %s -### 2>&1 | FileCheck %s 
--check-prefix=CHECK-INCOMPATIBLE
 // CHECK-INCOMPATIBLE: error: invalid argument '-fsanitize-coverage=func' not 
allowed with '-fsanitize-coverage=edge'
 
-// RUN: %clang -target x86_64-linux-gnu -fsanitize=address 
-fsanitize-coverage=8bit-counters %s -### 2>&1 | FileCheck %s 
--check-prefix=CHECK-MISSING-TYPE
-// CHECK-MISSING-TYPE: error: invalid argument 
'-fsanitize-coverage=8bit-counters' only allowed with 
'-fsanitize-coverage=(func|bb|edge)'
+// RUN: %clang -target x86_64-linux-gnu -fsanitize-coverage=8bit-counters %s 
-### 2>&1 | FileCheck %s --check-prefix=CHECK-8BIT
+// CHECK-8BIT: warning: argument '-fsanitize-coverage=8bit-counters' is 
deprecated, use '-fsanitize-coverage=trace-pc-guard' instead
 
 // RUN: %clang -target x86_64-linux-gnu -fsanitize-coverage=trace-pc %s -### 
2>&1 | FileCheck %s --check-prefix=CHECK-TRACE_PC_EDGE
 // RUN: %clang -target x86_64-linux-gnu -fsanitize-coverage=edge,trace-pc %s 
-### 2>&1 | FileCheck %s --check-prefix=CHECK-TRACE_PC_EDGE


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

[PATCH] D32234: [Clangd] Support Authority-less URIs

[Marc-Andre Laperle via Phabricator via cfe-commits]

malaperle-ericsson updated this revision to Diff 95809.
malaperle-ericsson added a comment.

Add test


https://reviews.llvm.org/D32234

Files:
  clangd/Protocol.cpp
  test/clangd/completion.test


Index: test/clangd/completion.test
===
--- test/clangd/completion.test
+++ test/clangd/completion.test
@@ -20,6 +20,16 @@
 # CHECK-DAG: {"label":"bb","kind":5}
 # CHECK-DAG: {"label":"ccc","kind":5}
 # CHECK: ]}
+Content-Length: 146
+
+{"jsonrpc":"2.0","id":1,"method":"textDocument/completion","params":{"textDocument":{"uri":"file:/main.cpp"},"position":{"line":3,"character":5}}}
+# Test authority-less URI
+#
+# CHECK: {"jsonrpc":"2.0","id":1,"result":[
+# CHECK-DAG: {"label":"a","kind":5}
+# CHECK-DAG: {"label":"bb","kind":5}
+# CHECK-DAG: {"label":"ccc","kind":5}
+# CHECK: ]}
 Content-Length: 44
 
 {"jsonrpc":"2.0","id":3,"method":"shutdown"}
Index: clangd/Protocol.cpp
===
--- clangd/Protocol.cpp
+++ clangd/Protocol.cpp
@@ -25,6 +25,8 @@
   URI Result;
   Result.uri = uri;
   uri.consume_front("file://");
+  // Also trim authority-less URIs
+  uri.consume_front("file:");
   // For Windows paths e.g. /X:
   if (uri.size() > 2 && uri[0] == '/' && uri[2] == ':')
 uri.consume_front("/");


Index: test/clangd/completion.test
===
--- test/clangd/completion.test
+++ test/clangd/completion.test
@@ -20,6 +20,16 @@
 # CHECK-DAG: {"label":"bb","kind":5}
 # CHECK-DAG: {"label":"ccc","kind":5}
 # CHECK: ]}
+Content-Length: 146
+
+{"jsonrpc":"2.0","id":1,"method":"textDocument/completion","params":{"textDocument":{"uri":"file:/main.cpp"},"position":{"line":3,"character":5}}}
+# Test authority-less URI
+#
+# CHECK: {"jsonrpc":"2.0","id":1,"result":[
+# CHECK-DAG: {"label":"a","kind":5}
+# CHECK-DAG: {"label":"bb","kind":5}
+# CHECK-DAG: {"label":"ccc","kind":5}
+# CHECK: ]}
 Content-Length: 44
 
 {"jsonrpc":"2.0","id":3,"method":"shutdown"}
Index: clangd/Protocol.cpp
===
--- clangd/Protocol.cpp
+++ clangd/Protocol.cpp
@@ -25,6 +25,8 @@
   URI Result;
   Result.uri = uri;
   uri.consume_front("file://");
+  // Also trim authority-less URIs
+  uri.consume_front("file:");
   // For Windows paths e.g. /X:
   if (uri.size() > 2 && uri[0] == '/' && uri[2] == ':')
 uri.consume_front("/");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32238: [Clangd] Failed to decode params using 1.x-compatible request message

[Marc-Andre Laperle via Phabricator via cfe-commits]

malaperle-ericsson updated this revision to Diff 95811.
malaperle-ericsson added a comment.

Add test


https://reviews.llvm.org/D32238

Files:
  clangd/Protocol.cpp
  test/clangd/completion.test


Index: test/clangd/completion.test
===
--- test/clangd/completion.test
+++ test/clangd/completion.test
@@ -20,6 +20,17 @@
 # CHECK-DAG: {"label":"bb","kind":5}
 # CHECK-DAG: {"label":"ccc","kind":5}
 # CHECK: ]}
+
+Content-Length: 172
+
+{"jsonrpc":"2.0","id":1,"method":"textDocument/completion","params":{"textDocument":{"uri":"file:///main.cpp"},"uri":"file:///main.cpp","position":{"line":3,"character":5}}}
+# Test params parsing in the presence of a 1.x-compatible client (inlined 
"uri")
+#
+# CHECK: {"jsonrpc":"2.0","id":1,"result":[
+# CHECK-DAG: {"label":"a","kind":5}
+# CHECK-DAG: {"label":"bb","kind":5}
+# CHECK-DAG: {"label":"ccc","kind":5}
+# CHECK: ]}
 Content-Length: 44
 
 {"jsonrpc":"2.0","id":3,"method":"shutdown"}
Index: clangd/Protocol.cpp
===
--- clangd/Protocol.cpp
+++ clangd/Protocol.cpp
@@ -646,7 +646,7 @@
 auto *Value =
 dyn_cast_or_null(NextKeyValue.getValue());
 if (!Value)
-  return llvm::None;
+  continue;
 
 llvm::SmallString<10> Storage;
 if (KeyValue == "textDocument") {


Index: test/clangd/completion.test
===
--- test/clangd/completion.test
+++ test/clangd/completion.test
@@ -20,6 +20,17 @@
 # CHECK-DAG: {"label":"bb","kind":5}
 # CHECK-DAG: {"label":"ccc","kind":5}
 # CHECK: ]}
+
+Content-Length: 172
+
+{"jsonrpc":"2.0","id":1,"method":"textDocument/completion","params":{"textDocument":{"uri":"file:///main.cpp"},"uri":"file:///main.cpp","position":{"line":3,"character":5}}}
+# Test params parsing in the presence of a 1.x-compatible client (inlined "uri")
+#
+# CHECK: {"jsonrpc":"2.0","id":1,"result":[
+# CHECK-DAG: {"label":"a","kind":5}
+# CHECK-DAG: {"label":"bb","kind":5}
+# CHECK-DAG: {"label":"ccc","kind":5}
+# CHECK: ]}
 Content-Length: 44
 
 {"jsonrpc":"2.0","id":3,"method":"shutdown"}
Index: clangd/Protocol.cpp
===
--- clangd/Protocol.cpp
+++ clangd/Protocol.cpp
@@ -646,7 +646,7 @@
 auto *Value =
 dyn_cast_or_null(NextKeyValue.getValue());
 if (!Value)
-  return llvm::None;
+  continue;
 
 llvm::SmallString<10> Storage;
 if (KeyValue == "textDocument") {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32243: Fix a leak in tools/driver/cc1as_main.cpp

[Kostya Serebryany via Phabricator via cfe-commits]

kcc created this revision.

For some reason, the asan bot has recently started reporting this leak even 
though it existed for ages.


https://reviews.llvm.org/D32243

Files:
  tools/driver/cc1as_main.cpp


Index: tools/driver/cc1as_main.cpp
===
--- tools/driver/cc1as_main.cpp
+++ tools/driver/cc1as_main.cpp
@@ -506,12 +506,12 @@
   // FIXME: Remove this, one day.
   if (!Asm.LLVMArgs.empty()) {
 unsigned NumArgs = Asm.LLVMArgs.size();
-const char **Args = new const char*[NumArgs + 2];
+auto Args = llvm::make_unique(NumArgs + 2);
 Args[0] = "clang (LLVM option parsing)";
 for (unsigned i = 0; i != NumArgs; ++i)
   Args[i + 1] = Asm.LLVMArgs[i].c_str();
 Args[NumArgs + 1] = nullptr;
-llvm::cl::ParseCommandLineOptions(NumArgs + 1, Args);
+llvm::cl::ParseCommandLineOptions(NumArgs + 1, Args.get());
   }
 
   // Execute the invocation, unless there were parsing errors.


Index: tools/driver/cc1as_main.cpp
===
--- tools/driver/cc1as_main.cpp
+++ tools/driver/cc1as_main.cpp
@@ -506,12 +506,12 @@
   // FIXME: Remove this, one day.
   if (!Asm.LLVMArgs.empty()) {
 unsigned NumArgs = Asm.LLVMArgs.size();
-const char **Args = new const char*[NumArgs + 2];
+auto Args = llvm::make_unique(NumArgs + 2);
 Args[0] = "clang (LLVM option parsing)";
 for (unsigned i = 0; i != NumArgs; ++i)
   Args[i + 1] = Asm.LLVMArgs[i].c_str();
 Args[NumArgs + 1] = nullptr;
-llvm::cl::ParseCommandLineOptions(NumArgs + 1, Args);
+llvm::cl::ParseCommandLineOptions(NumArgs + 1, Args.get());
   }
 
   // Execute the invocation, unless there were parsing errors.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32243: Fix a leak in tools/driver/cc1as_main.cpp

[Peter Collingbourne via Phabricator via cfe-commits]

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

LGTM


https://reviews.llvm.org/D32243



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

[PATCH] D31885: Remove TBAA information from LValues representing union members

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In https://reviews.llvm.org/D31885#730958, @dberlin wrote:

> In https://reviews.llvm.org/D31885#730936, @rjmccall wrote:
>
> > In https://reviews.llvm.org/D31885#730920, @dberlin wrote:
> >
> > > Just so i understand: Ignoring everything else (we can't actually make 
> > > likelyalias work, i think the code in the bugs makes that very clear),
> >
> >
> > Are you under the impression that I'm proposing something new and that TBAA 
> > does not currently defer to BasicAA?
>
>
> Well, actually, chandler changed how it works, so it doesn't actually work 
> quite the way it used to but it has the same effect ;)
>  (It used to be explicit deferral, it is not explicit, neither depends on the 
> other any more.  It just happens to be how the default AA pipeline is ordered 
> right now)
>  I'm quite aware of the machinations of AA in LLVM :)
>
> I'm saying "this approach does not work well now" (ie see bugs) , and i don't 
> think continuing to try to make the contract even *more* incestuous is better 
> than trying to make it *less*.


What I'm trying to get across is that you're not going to be able to completely 
eliminate the "incest" without making TBAA substantially more aggressive, in 
ways that aren't really acceptable and definitely not consistent with how we've 
always described LLVM's TBAA to users.  I certainly would not feel comfortable 
ever switching Clang by default to a GCC-like model which will literally 
miscompile something as simple as:

void foo(float *f) -> int {

  *f = /*something*/;
  return *(int*) f;

}

If you acknowledge that, then we can legitimately talk about the right way to 
get the behavior that we want.  I would be fine with making that conservatism 
opt-in somehow, for example, especially if (as you suggest) we could use that 
to signal to BasicAA that it should not impose artificial limits on 
differentiating LikelyAlias from MayAlias.  (I still think that that would be 
useful to allow this kind of LikelyAlias result, at least for TBAA's use.  For 
example, the double* in the printf loop in the union-tbaa2.cpp test doesn't 
definitively overlap either store, but it's clearly still related enough that 
conservative-TBAA should be suppressed.)

> You seem to claim better optimization opportunity.
>  I ran numbers.
>  Across all things in llvm's benchmark suite, disabling unions entirely for 
> tbaa causes no regressions on x86 (I could try more platforms if you like).
> 
> I also  ran it across 50 third party packages and spec2006 that use strict 
> aliasing (These are just those things i could quickly test that had perf 
> benchmarks)

This is interesting information, thank you.  One obvious concern is that it 
almost certainly under-represents unions, which tend to be uncommon in 
benchmarks; benchmark swifts have also often be explicitly massaged to remove 
anything that could be described undefined behavior, which again is not 
consistent with the real world.  But it does suggest that we could try it.

> If it really caused any real perf loss, we could always define metadata to 
> say these accesses are noalias if we can't prove they are mustalias.
>  We could even use that metadata to explicitly try to have basicaa try much 
> harder than it does now to prove must-aliasing (for example, ignore depth 
> limits), since it would likely be too expensive to try it for everything.

Yes, this seems like a direction we should be taking.

> as an aside, I do not believe you could reasonably apply this to unions  
> because it's not possible to tell people what to do differently when it's 
> *llvm ir* that has the issue.
> 
> IE i can't reasonably tell users "please stop doing things that don't 
> generate explicit enough IR".
>  Especially when those answers change with inlining decisions, etc.

Specific simple patterns in the source language will reliably turn into 
specific patterns in the IR that we can guarantee are explicit enough.  If you 
get a pointer value from some opaque source, assign it to a local variable, and 
then use it multiple times, the optimizer will reliably see that the pointer is 
the same at each use.  Similarly if the pointer is just a local variable 
itself.  That's basically all I would guarantee as "sufficiently obvious": the 
pointers are obviously derived from the same source.

In these test cases, the accesses happen from different functions; but in order 
to be miscompiled they have to get inlined into one (unless we added some sort 
of cross-function AA, I suppose), which still admits the same kind of trivial 
pointer analysis.

John.


Repository:
  rL LLVM

https://reviews.llvm.org/D31885



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

r300755 - Fix a leak in tools/driver/cc1as_main.cpp

[Kostya Serebryany via cfe-commits]

Author: kcc
Date: Wed Apr 19 15:57:13 2017
New Revision: 300755

URL: http://llvm.org/viewvc/llvm-project?rev=300755&view=rev
Log:
Fix a leak in tools/driver/cc1as_main.cpp

Summary: For some reason, the asan bot has recently started reporting this leak 
even though it existed for ages.

Reviewers: pcc

Reviewed By: pcc

Subscribers: cfe-commits

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

Modified:
cfe/trunk/tools/driver/cc1as_main.cpp

Modified: cfe/trunk/tools/driver/cc1as_main.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/driver/cc1as_main.cpp?rev=300755&r1=300754&r2=300755&view=diff
==
--- cfe/trunk/tools/driver/cc1as_main.cpp (original)
+++ cfe/trunk/tools/driver/cc1as_main.cpp Wed Apr 19 15:57:13 2017
@@ -506,12 +506,12 @@ int cc1as_main(ArrayRef Ar
   // FIXME: Remove this, one day.
   if (!Asm.LLVMArgs.empty()) {
 unsigned NumArgs = Asm.LLVMArgs.size();
-const char **Args = new const char*[NumArgs + 2];
+auto Args = llvm::make_unique(NumArgs + 2);
 Args[0] = "clang (LLVM option parsing)";
 for (unsigned i = 0; i != NumArgs; ++i)
   Args[i + 1] = Asm.LLVMArgs[i].c_str();
 Args[NumArgs + 1] = nullptr;
-llvm::cl::ParseCommandLineOptions(NumArgs + 1, Args);
+llvm::cl::ParseCommandLineOptions(NumArgs + 1, Args.get());
   }
 
   // Execute the invocation, unless there were parsing errors.


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

r300756 - [CodeGen] Use preincrement version of APInt::operator++ instead of postincrement to avoid creating and immediately discarding a temporary APInt.

[Craig Topper via cfe-commits]

Author: ctopper
Date: Wed Apr 19 16:02:45 2017
New Revision: 300756

URL: http://llvm.org/viewvc/llvm-project?rev=300756&view=rev
Log:
[CodeGen] Use preincrement version of APInt::operator++ instead of 
postincrement to avoid creating and immediately discarding a temporary APInt.

This is preparation for a clang change to improve the [[nodiscard]] warning to 
not be ignored on methods that return a class marked [[nodiscard]] that are 
defined in the class itself. See D32207.


Modified:
cfe/trunk/lib/CodeGen/CGStmt.cpp

Modified: cfe/trunk/lib/CodeGen/CGStmt.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGStmt.cpp?rev=300756&r1=300755&r2=300756&view=diff
==
--- cfe/trunk/lib/CodeGen/CGStmt.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGStmt.cpp Wed Apr 19 16:02:45 2017
@@ -1166,7 +1166,7 @@ void CodeGenFunction::EmitCaseStmtRange(
   if (Rem)
 Rem--;
   SwitchInsn->addCase(Builder.getInt(LHS), CaseDest);
-  LHS++;
+  ++LHS;
 }
 return;
   }


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

r300762 - Fix assertion failure in codegen on non-template deduction guide.

[Richard Smith via cfe-commits]

Author: rsmith
Date: Wed Apr 19 16:15:45 2017
New Revision: 300762

URL: http://llvm.org/viewvc/llvm-project?rev=300762&view=rev
Log:
Fix assertion failure in codegen on non-template deduction guide.

Added:
cfe/trunk/test/CodeGenCXX/cxx1z-class-deduction.cpp
Modified:
cfe/trunk/lib/CodeGen/CodeGenModule.cpp

Modified: cfe/trunk/lib/CodeGen/CodeGenModule.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.cpp?rev=300762&r1=300761&r2=300762&view=diff
==
--- cfe/trunk/lib/CodeGen/CodeGenModule.cpp (original)
+++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp Wed Apr 19 16:15:45 2017
@@ -3794,6 +3794,10 @@ void CodeGenModule::EmitTopLevelDecl(Dec
 AddDeferredUnusedCoverageMapping(D);
 break;
 
+  case Decl::CXXDeductionGuide:
+// Function-like, but does not result in code emission.
+break;
+
   case Decl::Var:
   case Decl::Decomposition:
 // Skip variable templates

Added: cfe/trunk/test/CodeGenCXX/cxx1z-class-deduction.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/cxx1z-class-deduction.cpp?rev=300762&view=auto
==
--- cfe/trunk/test/CodeGenCXX/cxx1z-class-deduction.cpp (added)
+++ cfe/trunk/test/CodeGenCXX/cxx1z-class-deduction.cpp Wed Apr 19 16:15:45 2017
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 -std=c++1z %s -emit-llvm -o - | FileCheck %s
+
+template struct A {
+  A(T = 0);
+  A(void*);
+};
+
+template A(T*) -> A;
+A() -> A;
+
+// CHECK-LABEL: @_Z1fPi(
+void f(int *p) {
+  // CHECK: @_ZN1AIiEC
+  A a{};
+
+  // CHECK: @_ZN1AIlEC
+  A b = p;
+
+  // CHECK: @_ZN1AIxEC
+  A c = 123LL;
+}


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

r300764 - Corrrect warn_unused_result attribute

[Erich Keane via cfe-commits]

Author: erichkeane
Date: Wed Apr 19 16:24:55 2017
New Revision: 300764

URL: http://llvm.org/viewvc/llvm-project?rev=300764&view=rev
Log:
Corrrect warn_unused_result attribute 

The original idea was that if the attribute on an operator, 
that the return-value unused-ness wouldn't matter. However, 
all of the operators except postfix inc/dec return 
references! References don't result in this warning 
anyway, so those are already excluded.

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

Modified:
cfe/trunk/include/clang/AST/Decl.h
cfe/trunk/lib/AST/Decl.cpp
cfe/trunk/test/SemaCXX/warn-unused-result.cpp

Modified: cfe/trunk/include/clang/AST/Decl.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Decl.h?rev=300764&r1=300763&r2=300764&view=diff
==
--- cfe/trunk/include/clang/AST/Decl.h (original)
+++ cfe/trunk/include/clang/AST/Decl.h Wed Apr 19 16:24:55 2017
@@ -2082,10 +2082,7 @@ public:
   const Attr *getUnusedResultAttr() const;
 
   /// \brief Returns true if this function or its return type has the
-  /// warn_unused_result attribute. If the return type has the attribute and
-  /// this function is a method of the return type's class, then false will be
-  /// returned to avoid spurious warnings on member methods such as assignment
-  /// operators.
+  /// warn_unused_result attribute.
   bool hasUnusedResultAttr() const { return getUnusedResultAttr() != nullptr; }
 
   /// \brief Returns the storage class as written in the source. For the

Modified: cfe/trunk/lib/AST/Decl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Decl.cpp?rev=300764&r1=300763&r2=300764&view=diff
==
--- cfe/trunk/lib/AST/Decl.cpp (original)
+++ cfe/trunk/lib/AST/Decl.cpp Wed Apr 19 16:24:55 2017
@@ -3003,9 +3003,7 @@ SourceRange FunctionDecl::getExceptionSp
 const Attr *FunctionDecl::getUnusedResultAttr() const {
   QualType RetType = getReturnType();
   if (RetType->isRecordType()) {
-const CXXRecordDecl *Ret = RetType->getAsCXXRecordDecl();
-const auto *MD = dyn_cast(this);
-if (Ret && !(MD && MD->getCorrespondingMethodInClass(Ret, true))) {
+if (const CXXRecordDecl *Ret = RetType->getAsCXXRecordDecl()) {
   if (const auto *R = Ret->getAttr())
 return R;
 }

Modified: cfe/trunk/test/SemaCXX/warn-unused-result.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-unused-result.cpp?rev=300764&r1=300763&r2=300764&view=diff
==
--- cfe/trunk/test/SemaCXX/warn-unused-result.cpp (original)
+++ cfe/trunk/test/SemaCXX/warn-unused-result.cpp Wed Apr 19 16:24:55 2017
@@ -160,3 +160,49 @@ void g() {
   (void)noexcept(f(), false); // Should not warn.
 }
 }
+
+namespace {
+// C++ Methods should warn even in their own class.
+struct [[clang::warn_unused_result]] S {
+  S DoThing() { return {}; };
+  S operator++(int) { return {}; };
+  S operator--(int) { return {}; };
+  // Improperly written prefix.
+  S operator++() { return {}; };
+  S operator--() { return {}; };
+};
+
+struct [[clang::warn_unused_result]] P {
+  P DoThing() { return {}; };
+};
+
+P operator++(const P &, int) { return {}; };
+P operator--(const P &, int) { return {}; };
+// Improperly written prefix.
+P operator++(const P &) { return {}; };
+P operator--(const P &) { return {}; };
+
+void f() {
+  S s;
+  P p;
+  s.DoThing(); // expected-warning {{ignoring return value}}
+  p.DoThing(); // expected-warning {{ignoring return value}}
+  // Only postfix is expected to warn when written correctly.
+  s++; // expected-warning {{ignoring return value}}
+  s--; // expected-warning {{ignoring return value}}
+  p++; // expected-warning {{ignoring return value}}
+  p--; // expected-warning {{ignoring return value}}
+  // Improperly written prefix operators should still warn.
+  ++s; // expected-warning {{ignoring return value}}
+  --s; // expected-warning {{ignoring return value}}
+  ++p; // expected-warning {{ignoring return value}}
+  --p; // expected-warning {{ignoring return value}}
+
+  // Silencing the warning by cast to void still works.
+  (void)s.DoThing();
+  (void)s++;
+  (void)p++;
+  (void)++s;
+  (void)++p;
+}
+} // namespace


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

[PATCH] D32207: Corrrect warn_unused_result attribute

[Erich Keane via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL300764: Corrrect warn_unused_result attribute  (authored by 
erichkeane).

Changed prior to commit:
  https://reviews.llvm.org/D32207?vs=95807&id=95823#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D32207

Files:
  cfe/trunk/include/clang/AST/Decl.h
  cfe/trunk/lib/AST/Decl.cpp
  cfe/trunk/test/SemaCXX/warn-unused-result.cpp


Index: cfe/trunk/lib/AST/Decl.cpp
===
--- cfe/trunk/lib/AST/Decl.cpp
+++ cfe/trunk/lib/AST/Decl.cpp
@@ -3003,9 +3003,7 @@
 const Attr *FunctionDecl::getUnusedResultAttr() const {
   QualType RetType = getReturnType();
   if (RetType->isRecordType()) {
-const CXXRecordDecl *Ret = RetType->getAsCXXRecordDecl();
-const auto *MD = dyn_cast(this);
-if (Ret && !(MD && MD->getCorrespondingMethodInClass(Ret, true))) {
+if (const CXXRecordDecl *Ret = RetType->getAsCXXRecordDecl()) {
   if (const auto *R = Ret->getAttr())
 return R;
 }
Index: cfe/trunk/include/clang/AST/Decl.h
===
--- cfe/trunk/include/clang/AST/Decl.h
+++ cfe/trunk/include/clang/AST/Decl.h
@@ -2082,10 +2082,7 @@
   const Attr *getUnusedResultAttr() const;
 
   /// \brief Returns true if this function or its return type has the
-  /// warn_unused_result attribute. If the return type has the attribute and
-  /// this function is a method of the return type's class, then false will be
-  /// returned to avoid spurious warnings on member methods such as assignment
-  /// operators.
+  /// warn_unused_result attribute.
   bool hasUnusedResultAttr() const { return getUnusedResultAttr() != nullptr; }
 
   /// \brief Returns the storage class as written in the source. For the
Index: cfe/trunk/test/SemaCXX/warn-unused-result.cpp
===
--- cfe/trunk/test/SemaCXX/warn-unused-result.cpp
+++ cfe/trunk/test/SemaCXX/warn-unused-result.cpp
@@ -160,3 +160,49 @@
   (void)noexcept(f(), false); // Should not warn.
 }
 }
+
+namespace {
+// C++ Methods should warn even in their own class.
+struct [[clang::warn_unused_result]] S {
+  S DoThing() { return {}; };
+  S operator++(int) { return {}; };
+  S operator--(int) { return {}; };
+  // Improperly written prefix.
+  S operator++() { return {}; };
+  S operator--() { return {}; };
+};
+
+struct [[clang::warn_unused_result]] P {
+  P DoThing() { return {}; };
+};
+
+P operator++(const P &, int) { return {}; };
+P operator--(const P &, int) { return {}; };
+// Improperly written prefix.
+P operator++(const P &) { return {}; };
+P operator--(const P &) { return {}; };
+
+void f() {
+  S s;
+  P p;
+  s.DoThing(); // expected-warning {{ignoring return value}}
+  p.DoThing(); // expected-warning {{ignoring return value}}
+  // Only postfix is expected to warn when written correctly.
+  s++; // expected-warning {{ignoring return value}}
+  s--; // expected-warning {{ignoring return value}}
+  p++; // expected-warning {{ignoring return value}}
+  p--; // expected-warning {{ignoring return value}}
+  // Improperly written prefix operators should still warn.
+  ++s; // expected-warning {{ignoring return value}}
+  --s; // expected-warning {{ignoring return value}}
+  ++p; // expected-warning {{ignoring return value}}
+  --p; // expected-warning {{ignoring return value}}
+
+  // Silencing the warning by cast to void still works.
+  (void)s.DoThing();
+  (void)s++;
+  (void)p++;
+  (void)++s;
+  (void)++p;
+}
+} // namespace


Index: cfe/trunk/lib/AST/Decl.cpp
===
--- cfe/trunk/lib/AST/Decl.cpp
+++ cfe/trunk/lib/AST/Decl.cpp
@@ -3003,9 +3003,7 @@
 const Attr *FunctionDecl::getUnusedResultAttr() const {
   QualType RetType = getReturnType();
   if (RetType->isRecordType()) {
-const CXXRecordDecl *Ret = RetType->getAsCXXRecordDecl();
-const auto *MD = dyn_cast(this);
-if (Ret && !(MD && MD->getCorrespondingMethodInClass(Ret, true))) {
+if (const CXXRecordDecl *Ret = RetType->getAsCXXRecordDecl()) {
   if (const auto *R = Ret->getAttr())
 return R;
 }
Index: cfe/trunk/include/clang/AST/Decl.h
===
--- cfe/trunk/include/clang/AST/Decl.h
+++ cfe/trunk/include/clang/AST/Decl.h
@@ -2082,10 +2082,7 @@
   const Attr *getUnusedResultAttr() const;
 
   /// \brief Returns true if this function or its return type has the
-  /// warn_unused_result attribute. If the return type has the attribute and
-  /// this function is a method of the return type's class, then false will be
-  /// returned to avoid spurious warnings on member methods such as assignment
-  /// operators.
+  /// warn_unused_result attribute.
   bool hasUnusedResultAttr() const { return getUnusedResultAttr() != nullptr; }
 
   /// \brief Returns the storage class as written in the s

r300767 - [sanitizer-coverage] deprecate -fsanitize-coverage=trace-bb

[Kostya Serebryany via cfe-commits]

Author: kcc
Date: Wed Apr 19 16:31:11 2017
New Revision: 300767

URL: http://llvm.org/viewvc/llvm-project?rev=300767&view=rev
Log:
[sanitizer-coverage] deprecate -fsanitize-coverage=trace-bb

Modified:
cfe/trunk/lib/Driver/SanitizerArgs.cpp
cfe/trunk/test/Driver/fsanitize-coverage.c

Modified: cfe/trunk/lib/Driver/SanitizerArgs.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/SanitizerArgs.cpp?rev=300767&r1=300766&r2=300767&view=diff
==
--- cfe/trunk/lib/Driver/SanitizerArgs.cpp (original)
+++ cfe/trunk/lib/Driver/SanitizerArgs.cpp Wed Apr 19 16:31:11 2017
@@ -508,12 +508,11 @@ SanitizerArgs::SanitizerArgs(const ToolC
   // Basic block tracing and 8-bit counters require some type of coverage
   // enabled.
   int CoverageTypes = CoverageFunc | CoverageBB | CoverageEdge;
-  if ((CoverageFeatures & CoverageTraceBB) &&
-  !(CoverageFeatures & CoverageTypes))
-D.Diag(clang::diag::err_drv_argument_only_allowed_with)
+  if (CoverageFeatures & CoverageTraceBB)
+D.Diag(clang::diag::warn_drv_deprecated_arg)
 << "-fsanitize-coverage=trace-bb"
-<< "-fsanitize-coverage=(func|bb|edge)";
-  if ((CoverageFeatures & Coverage8bitCounters))
+<< "-fsanitize-coverage=trace-pc-guard";
+  if (CoverageFeatures & Coverage8bitCounters)
 D.Diag(clang::diag::warn_drv_deprecated_arg)
 << "-fsanitize-coverage=8bit-counters"
 << "-fsanitize-coverage=trace-pc-guard";

Modified: cfe/trunk/test/Driver/fsanitize-coverage.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/fsanitize-coverage.c?rev=300767&r1=300766&r2=300767&view=diff
==
--- cfe/trunk/test/Driver/fsanitize-coverage.c (original)
+++ cfe/trunk/test/Driver/fsanitize-coverage.c Wed Apr 19 16:31:11 2017
@@ -33,16 +33,15 @@
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=address 
-fsanitize-coverage=func -fno-sanitize=address %s -### 2>&1 | FileCheck %s 
--check-prefix=CHECK-SANITIZE-COVERAGE-SAN-DISABLED
 // CHECK-SANITIZE-COVERAGE-SAN-DISABLED-NOT: argument unused
 
-// RUN: %clang -target x86_64-linux-gnu -fsanitize=address 
-fsanitize-coverage=edge,indirect-calls,trace-bb,trace-pc,trace-cmp,trace-div,trace-gep
 %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SANITIZE-COVERAGE-FEATURES
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=address 
-fsanitize-coverage=edge,indirect-calls,trace-pc,trace-cmp,trace-div,trace-gep 
%s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SANITIZE-COVERAGE-FEATURES
 // CHECK-SANITIZE-COVERAGE-FEATURES: -fsanitize-coverage-type=3
 // CHECK-SANITIZE-COVERAGE-FEATURES: -fsanitize-coverage-indirect-calls
-// CHECK-SANITIZE-COVERAGE-FEATURES: -fsanitize-coverage-trace-bb
 // CHECK-SANITIZE-COVERAGE-FEATURES: -fsanitize-coverage-trace-cmp
 // CHECK-SANITIZE-COVERAGE-FEATURES: -fsanitize-coverage-trace-div
 // CHECK-SANITIZE-COVERAGE-FEATURES: -fsanitize-coverage-trace-gep
 // CHECK-SANITIZE-COVERAGE-FEATURES: -fsanitize-coverage-trace-pc
 
-// RUN: %clang -target x86_64-linux-gnu -fsanitize=address 
-fsanitize-coverage=func,edge,indirect-calls,trace-bb,trace-cmp 
-fno-sanitize-coverage=edge,indirect-calls,trace-bb %s -### 2>&1 | FileCheck %s 
--check-prefix=CHECK-MASK
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=address 
-fsanitize-coverage=func,edge,indirect-calls,trace-cmp 
-fno-sanitize-coverage=edge,indirect-calls %s -### 2>&1 | FileCheck %s 
--check-prefix=CHECK-MASK
 // CHECK-MASK: -fsanitize-coverage-type=1
 // CHECK-MASK: -fsanitize-coverage-trace-cmp
 // CHECK-MASK-NOT: -fsanitize-coverage-
@@ -55,6 +54,8 @@
 
 // RUN: %clang -target x86_64-linux-gnu -fsanitize-coverage=8bit-counters %s 
-### 2>&1 | FileCheck %s --check-prefix=CHECK-8BIT
 // CHECK-8BIT: warning: argument '-fsanitize-coverage=8bit-counters' is 
deprecated, use '-fsanitize-coverage=trace-pc-guard' instead
+// RUN: %clang -target x86_64-linux-gnu -fsanitize-coverage=trace-bb %s -### 
2>&1 | FileCheck %s --check-prefix=CHECK-TRACE-BB
+// CHECK-TRACE-BB: warning: argument '-fsanitize-coverage=trace-bb' is 
deprecated, use '-fsanitize-coverage=trace-pc-guard' instead
 
 // RUN: %clang -target x86_64-linux-gnu -fsanitize-coverage=trace-pc %s -### 
2>&1 | FileCheck %s --check-prefix=CHECK-TRACE_PC_EDGE
 // RUN: %clang -target x86_64-linux-gnu -fsanitize-coverage=edge,trace-pc %s 
-### 2>&1 | FileCheck %s --check-prefix=CHECK-TRACE_PC_EDGE


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

[PATCH] D32199: [TBAASan] A TBAA Sanitizer (Clang)

[Hal Finkel via Phabricator via cfe-commits]

hfinkel added a comment.

In https://reviews.llvm.org/D32199#730716, @filcab wrote:

> Missing a `sanitizer-ld.c` test for freebsd.


Thanks for pointing this out. I'm going to remove the freebsd change for now. I 
suspect it works, but I've not tested it yet.




Comment at: include/clang/Basic/Attr.td:1849
+   GNU<"no_sanitize_memory">,
+   GNU<"no_sanitize_tbaa">];
   let Subjects = SubjectList<[Function, GlobalVar], ErrorDiag,

filcab wrote:
> Shouldn't you be extending the no_sanitize attribute instead, as it says in 
> the comment?
Indeed. I think that happened also. I'll make sure the tests reflect that.



Comment at: lib/CodeGen/CodeGenTBAA.cpp:96
+  if (!Features.Sanitize.has(SanitizerKind::TBAA) &&
+  (CodeGenOpts.OptimizationLevel == 0 || CodeGenOpts.RelaxedAliasing))
 return nullptr;

filcab wrote:
> Interesting that TSan needs TBAA (per the comment in the chunk above), but is 
> not in this condition.
Yea, I didn't understand the TSan-needs-TBAA bit either (considering that it 
does not need it at -O0?).


https://reviews.llvm.org/D32199



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

[PATCH] D32064: [asan] Disable ASan global-GC depending on the target and compiler flags

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a subscriber: chandlerc.
rnk added a comment.

In https://reviews.llvm.org/D32064#728683, @pcc wrote:

> In https://reviews.llvm.org/D32064#728629, @rnk wrote:
>
> > In https://reviews.llvm.org/D32064#726861, @pcc wrote:
> >
> > > I think it would be better to move this logic to the driver and have it 
> > > pass in an `-mllvm` flag. The sanitizer passes should really be taking no 
> > > arguments in the constructor like the other passes, so I don't want us to 
> > > add another argument here.
> >
> >
> > That seems like the opposite of the direction we've been moving, though. 
> > cl::opt flags can't appear twice, and this means one process can't do two 
> > asan compilations in two LLVMContexts in parallel with different settings.
>
>
> Yes, but adding an argument is also the wrong direction. This information 
> should really be passed either via the module (e.g. module flags or 
> attributes) or the TargetMachine. If we aren't going to do that, we might as 
> well pass it via `-mllvm`, as it is simpler.


I'm really just echoing what I thought was conventional wisdom. I think 
avoiding new `cl::opt`s is something that @chandlerc cares more about. It's 
been said on llvm-dev that `cl::opt` should mostly be for debugging. We should 
be able to ship a production compiler that effectively compiles every cl::opt 
to its default value.

I don't see why constructor parameters are the wrong direction. It's already 
how ASan instrumentation takes every other global pass option. Only tsan uses 
-mllvm flags created by clang. If we aren't going to make ASan consistent, we 
should standardize on what we already have.


Repository:
  rL LLVM

https://reviews.llvm.org/D32064



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

[PATCH] D32199: [TBAASan] A TBAA Sanitizer (Clang)

[Hal Finkel via Phabricator via cfe-commits]

hfinkel updated this revision to Diff 95829.
hfinkel added a comment.

Updated per review comments (use only no_sanitize("tbaa") instead of adding 
no_sanitize_tbaa and don't touch freebsd for now).


https://reviews.llvm.org/D32199

Files:
  include/clang/Basic/Sanitizers.def
  include/clang/Driver/SanitizerArgs.h
  lib/CodeGen/BackendUtil.cpp
  lib/CodeGen/CGDeclCXX.cpp
  lib/CodeGen/CodeGenFunction.cpp
  lib/CodeGen/CodeGenModule.cpp
  lib/CodeGen/CodeGenTBAA.cpp
  lib/Driver/SanitizerArgs.cpp
  lib/Driver/ToolChains/CommonArgs.cpp
  lib/Driver/ToolChains/Linux.cpp
  lib/Frontend/CompilerInvocation.cpp
  lib/Lex/PPMacroExpansion.cpp
  lib/Sema/SemaDeclAttr.cpp
  test/CodeGen/sanitize-tbaa-attr.cpp
  test/Driver/sanitizer-ld.c

Index: test/Driver/sanitizer-ld.c
===
--- test/Driver/sanitizer-ld.c
+++ test/Driver/sanitizer-ld.c
@@ -181,6 +181,18 @@
 
 // RUN: %clangxx -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: -target x86_64-unknown-linux -fuse-ld=ld -stdlib=platform -lstdc++ \
+// RUN: -fsanitize=tbaa \
+// RUN: -resource-dir=%S/Inputs/resource_dir \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-TBAASAN-LINUX-CXX %s
+//
+// CHECK-TBAASAN-LINUX-CXX: "{{(.*[^-.0-9A-Z_a-z])?}}ld{{(.exe)?}}"
+// CHECK-TBAASAN-LINUX-CXX-NOT: stdc++
+// CHECK-TBAASAN-LINUX-CXX: "-whole-archive" "{{.*}}libclang_rt.tbaasan-x86_64.a" "-no-whole-archive"
+// CHECK-TBAASAN-LINUX-CXX: stdc++
+
+// RUN: %clangxx -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: -target x86_64-unknown-linux -fuse-ld=ld -stdlib=platform -lstdc++ \
 // RUN: -fsanitize=memory \
 // RUN: -resource-dir=%S/Inputs/resource_dir \
 // RUN: --sysroot=%S/Inputs/basic_linux_tree \
Index: test/CodeGen/sanitize-tbaa-attr.cpp
===
--- /dev/null
+++ test/CodeGen/sanitize-tbaa-attr.cpp
@@ -0,0 +1,64 @@
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -o - %s | FileCheck -check-prefix=WITHOUT %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -o - %s -fsanitize=tbaa | FileCheck -check-prefix=TBAASAN %s
+// RUN: echo "src:%s" | sed -e 's/\\//g' > %t
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -o - %s -fsanitize=tbaa -fsanitize-blacklist=%t | FileCheck -check-prefix=BL %s
+
+// The sanitize_tbaa attribute should be attached to functions
+// when TBAASanitizer is enabled, unless no_sanitize("tbaa") attribute
+// is present.
+
+// WITHOUT:  NoTBAASAN1{{.*}}) [[NOATTR:#[0-9]+]]
+// BL:  NoTBAASAN1{{.*}}) [[NOATTR:#[0-9]+]]
+// TBAASAN:  NoTBAASAN1{{.*}}) [[NOATTR:#[0-9]+]]
+__attribute__((no_sanitize("tbaa")))
+int NoTBAASAN1(int *a) { return *a; }
+
+// WITHOUT:  NoTBAASAN2{{.*}}) [[NOATTR]]
+// BL:  NoTBAASAN2{{.*}}) [[NOATTR]]
+// TBAASAN:  NoTBAASAN2{{.*}}) [[NOATTR]]
+__attribute__((no_sanitize("tbaa")))
+int NoTBAASAN2(int *a);
+int NoTBAASAN2(int *a) { return *a; }
+
+// WITHOUT:  NoTBAASAN3{{.*}}) [[NOATTR:#[0-9]+]]
+// BL:  NoTBAASAN3{{.*}}) [[NOATTR:#[0-9]+]]
+// TBAASAN:  NoTBAASAN3{{.*}}) [[NOATTR:#[0-9]+]]
+__attribute__((no_sanitize("tbaa")))
+int NoTBAASAN3(int *a) { return *a; }
+
+// WITHOUT:  TBAASANOk{{.*}}) [[NOATTR]]
+// BL:  TBAASANOk{{.*}}) [[NOATTR]]
+// TBAASAN: TBAASANOk{{.*}}) [[WITH:#[0-9]+]]
+int TBAASANOk(int *a) { return *a; }
+
+// WITHOUT:  TemplateTBAASANOk{{.*}}) [[NOATTR]]
+// BL:  TemplateTBAASANOk{{.*}}) [[NOATTR]]
+// TBAASAN: TemplateTBAASANOk{{.*}}) [[WITH]]
+template
+int TemplateTBAASANOk() { return i; }
+
+// WITHOUT:  TemplateNoTBAASAN{{.*}}) [[NOATTR]]
+// BL:  TemplateNoTBAASAN{{.*}}) [[NOATTR]]
+// TBAASAN: TemplateNoTBAASAN{{.*}}) [[NOATTR]]
+template
+__attribute__((no_sanitize("tbaa")))
+int TemplateNoTBAASAN() { return i; }
+
+int force_instance = TemplateTBAASANOk<42>()
+   + TemplateNoTBAASAN<42>();
+
+// Check that __cxx_global_var_init* get the sanitize_tbaa attribute.
+int global1 = 0;
+int global2 = *(int*)((char*)&global1+1);
+// WITHOUT: @__cxx_global_var_init{{.*}}[[NOATTR:#[0-9]+]]
+// BL: @__cxx_global_var_init{{.*}}[[NOATTR:#[0-9]+]]
+// TBAASAN: @__cxx_global_var_init{{.*}}[[WITH:#[0-9]+]]
+
+// WITHOUT: attributes [[NOATTR]] = { noinline nounwind{{.*}} }
+
+// BL: attributes [[NOATTR]] = { noinline nounwind{{.*}} }
+
+// TBAASAN: attributes [[NOATTR]] = { noinline nounwind{{.*}} }
+// TBAASAN: attributes [[WITH]] = { noinline nounwind sanitize_tbaa{{.*}} }
+
+// TBAASAN: Simple C++ TBAA
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -5733,7 +5733,8 @@
 .Case("no_address_safety_analysis", "address")
 .Case("no_sanitize_address", "address")
 .Case("no_sanitize_thread", "thread")
-.Case("no_sanitize_memory", 

[PATCH] D32248: CodeGen: Cast alloca to expected address space

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl created this revision.

Alloca always returns a pointer in alloca address space, which may
be different from the type defined by the language. For example,
in C++ the auto variables are in the default address space. Therefore
cast alloca to the expected address space when necessary.


https://reviews.llvm.org/D32248

Files:
  lib/CodeGen/CGDecl.cpp
  lib/CodeGen/CodeGenTypes.cpp
  lib/CodeGen/CodeGenTypes.h
  test/CodeGen/address-space.c
  test/CodeGenCXX/amdgcn-automatic-variable.cpp

Index: test/CodeGenCXX/amdgcn-automatic-variable.cpp
===
--- /dev/null
+++ test/CodeGenCXX/amdgcn-automatic-variable.cpp
@@ -0,0 +1,52 @@
+// RUN: %clang_cc1 -O0 -triple amdgcn---amdgiz -emit-llvm %s -o - | FileCheck %s
+
+// CHECK-LABEL: define void @_Z5func1Pi(i32* %x)
+void func1(int *x) {
+  // CHECK: %[[x_addr:.*]] = alloca i32*{{.*}}addrspace(5)
+  // CHECK: store i32* %x, i32* addrspace(5)* %[[x_addr]]
+  // CHECK: %[[r0:.*]] = load i32*, i32* addrspace(5)* %[[x_addr]]
+  // CHECK: store i32 1, i32* %[[r0]]
+  *x = 1;
+}
+
+// CHECK-LABEL: define void @_Z5func2v()
+void func2(void) {
+  // CHECK: %lv1 = alloca i32, align 4, addrspace(5)
+  // CHECK: %lv2 = alloca i32, align 4, addrspace(5)
+  // CHECK: %la = alloca [100 x i32], align 4, addrspace(5)
+  // CHECK: %lp1 = alloca i32*, align 4, addrspace(5)
+  // CHECK: %lp2 = alloca i32*, align 4, addrspace(5)
+  // CHECK: %lvc = alloca i32, align 4, addrspace(5)
+
+  // CHECK: %[[r0:.*]] = addrspacecast i32 addrspace(5)* %lv1 to i32*
+  // CHECK: store i32 1, i32* %[[r0]]
+  // CHECK: %[[r1:.*]] = addrspacecast i32 addrspace(5)* %lv2 to i32*
+  // CHECK: store i32 2, i32* %[[r1]]
+  int lv1;
+  lv1 = 1;
+  int lv2 = 2;
+
+  // CHECK: %[[r2:.*]] = addrspacecast [100 x i32] addrspace(5)* %la to [100 x i32]*
+  // CHECK: %[[arrayidx:.*]] = getelementptr inbounds [100 x i32], [100 x i32]* %[[r2]], i64 0, i64 0
+  // CHECK: store i32 3, i32* %[[arrayidx]], align 4
+  int la[100];
+  la[0] = 3;
+
+  // CHECK: %[[r3:.*]] = addrspacecast i32* addrspace(5)* %lp1 to i32**
+  // CHECK: store i32* %[[r0]], i32** %[[r3]], align 4
+  int *lp1 = &lv1;
+
+  // CHECK: %[[r4:.*]] = addrspacecast i32* addrspace(5)* %lp2 to i32**
+  // CHECK: %[[arraydecay:.*]] = getelementptr inbounds [100 x i32], [100 x i32]* %[[r2]], i32 0, i32 0
+  // CHECK: store i32* %[[arraydecay]], i32** %[[r4]], align 4
+  int *lp2 = la;
+
+  // CHECK: call void @_Z5func1Pi(i32* %[[r0]])
+  func1(&lv1);
+
+  // CHECK: %[[r5:.*]] = addrspacecast i32 addrspace(5)* %lvc to i32*
+  // CHECK: store i32 4, i32* %[[r5]]
+  // CHECK: store i32 4, i32* %[[r0]]
+  const int lvc = 4;
+  lv1 = lvc;
+}
Index: test/CodeGen/address-space.c
===
--- test/CodeGen/address-space.c
+++ test/CodeGen/address-space.c
@@ -40,8 +40,10 @@
 } MyStruct;
 
 // CHECK-LABEL: define void @test4(
-// CHECK: call void @llvm.memcpy.p0i8.p2i8
-// CHECK: call void @llvm.memcpy.p2i8.p0i8
+// GIZ: call void @llvm.memcpy.p0i8.p2i8
+// GIZ: call void @llvm.memcpy.p2i8.p0i8
+// PIZ: call void @llvm.memcpy.p4i8.p2i8
+// PIZ: call void @llvm.memcpy.p2i8.p4i8
 void test4(MyStruct __attribute__((address_space(2))) *pPtr) {
   MyStruct s = pPtr[0];
   pPtr[0] = s;
Index: lib/CodeGen/CodeGenTypes.h
===
--- lib/CodeGen/CodeGenTypes.h
+++ lib/CodeGen/CodeGenTypes.h
@@ -196,6 +196,9 @@
   /// memory representation is usually i8 or i32, depending on the target.
   llvm::Type *ConvertTypeForMem(QualType T);
 
+  /// Get the LLVM pointer type of a variable.
+  llvm::PointerType *getVariableType(VarDecl D);
+
   /// GetFunctionType - Get the LLVM function type for \arg Info.
   llvm::FunctionType *GetFunctionType(const CGFunctionInfo &Info);
 
Index: lib/CodeGen/CodeGenTypes.cpp
===
--- lib/CodeGen/CodeGenTypes.cpp
+++ lib/CodeGen/CodeGenTypes.cpp
@@ -92,6 +92,11 @@
 (unsigned)Context.getTypeSize(T));
 }
 
+llvm::PointerType *CodeGenTypes::getVariableType(VarDecl D) {
+  auto Ty = D.getType();
+  return ConvertTypeForMem(Ty)->getPointerTo(
+  getContext().getTargetAddressSpace(Ty));
+}
 
 /// isRecordLayoutComplete - Return true if the specified type is already
 /// completely laid out.
Index: lib/CodeGen/CGDecl.cpp
===
--- lib/CodeGen/CGDecl.cpp
+++ lib/CodeGen/CGDecl.cpp
@@ -1102,6 +1102,22 @@
 address = Address(vla, alignment);
   }
 
+  // Alloca always returns a pointer in alloca address space, which may
+  // be different from the type defined by the language. For example,
+  // in C++ the auto variables are in the default address space. Therefore
+  // cast alloca to the expected address space when necessary.
+  auto Addr = address.getPointer();
+  auto AddrTy = cast(Addr->getType());
+  auto ExpectedA

[PATCH] D32199: [TBAASan] A TBAA Sanitizer (Clang)

[Richard Smith via Phabricator via cfe-commits]

rsmith added a comment.

I don't like calling this a "TBAA sanitizer". What we're sanitizing is the 
object model and effective type rules; it seems irrelevant which specific 
compiler analysis passes would result in your program misbehaving if you break 
the rules. I would also expect that we will extend this in future to assign 
types to storage even in cases where there is no store (for instance, we should 
be able to catch `float f() { int n; return *(float*)&n; }` despite there being 
no TBAA violation in the naive IR).

How about renaming this to something more like `-fsanitize=type`?


https://reviews.llvm.org/D32199



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

[libcxx] r300770 - Fix typo in Windows test configuration code

[Eric Fiselier via cfe-commits]

Author: ericwf
Date: Wed Apr 19 16:52:08 2017
New Revision: 300770

URL: http://llvm.org/viewvc/llvm-project?rev=300770&view=rev
Log:
Fix typo in Windows test configuration code

Modified:
libcxx/trunk/utils/libcxx/test/config.py

Modified: libcxx/trunk/utils/libcxx/test/config.py
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/utils/libcxx/test/config.py?rev=300770&r1=300769&r2=300770&view=diff
==
--- libcxx/trunk/utils/libcxx/test/config.py (original)
+++ libcxx/trunk/utils/libcxx/test/config.py Wed Apr 19 16:52:08 2017
@@ -241,7 +241,7 @@ class Configuration(object):
 flags = []
 compile_flags = _prefixed_env_list('INCLUDE', '-isystem')
 link_flags = _prefixed_env_list('LIB', '-L')
-for path in _list_env_var('LIB'):
+for path in _split_env_var('LIB'):
 self.add_path(self.exec_env, path)
 return CXXCompiler(clang_path, flags=flags,
compile_flags=compile_flags,


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

[PATCH] D31542: [clang-tidy] Extend readability-container-size-empty to add comparisons to newly-constructed objects

[Josh Zimmerman via Phabricator via cfe-commits]

joshz updated this revision to Diff 95833.
joshz marked 3 inline comments as done.
joshz added a comment.

Clean up IsBinaryOrTernary


Repository:
  rL LLVM

https://reviews.llvm.org/D31542

Files:
  clang-tidy/readability/ContainerSizeEmptyCheck.cpp
  clang-tidy/utils/ASTUtils.cpp
  clang-tidy/utils/ASTUtils.h
  test/clang-tidy/readability-container-size-empty.cpp

Index: test/clang-tidy/readability-container-size-empty.cpp
===
--- test/clang-tidy/readability-container-size-empty.cpp
+++ test/clang-tidy/readability-container-size-empty.cpp
@@ -3,12 +3,19 @@
 namespace std {
 template  struct vector {
   vector();
+  bool operator==(const vector& other) const;
+  bool operator!=(const vector& other) const;
   unsigned long size() const;
   bool empty() const;
 };
 
 template  struct basic_string {
   basic_string();
+  bool operator==(const basic_string& other) const;
+  bool operator!=(const basic_string& other) const;
+  bool operator==(const char *) const;
+  bool operator!=(const char *) const;
+  basic_string operator+(const basic_string& other) const;
   unsigned long size() const;
   bool empty() const;
 };
@@ -19,6 +26,8 @@
 inline namespace __v2 {
 template  struct set {
   set();
+  bool operator==(const set& other) const;
+  bool operator!=(const set& other) const;
   unsigned long size() const;
   bool empty() const;
 };
@@ -29,13 +38,17 @@
 template 
 class TemplatedContainer {
 public:
+  bool operator==(const TemplatedContainer& other) const;
+  bool operator!=(const TemplatedContainer& other) const;
   int size() const;
   bool empty() const;
 };
 
 template 
 class PrivateEmpty {
 public:
+  bool operator==(const PrivateEmpty& other) const;
+  bool operator!=(const PrivateEmpty& other) const;
   int size() const;
 private:
   bool empty() const;
@@ -54,6 +67,7 @@
 
 class Container {
 public:
+  bool operator==(const Container& other) const;
   int size() const;
   bool empty() const;
 };
@@ -75,9 +89,21 @@
 
 bool Container3::empty() const { return this->size() == 0; }
 
+class Container4 {
+public:
+  bool operator==(const Container4& rhs) const;
+  int size() const;
+  bool empty() const { return *this == Container4(); }
+};
+
+std::string s_func() {
+  return std::string();
+}
+
 int main() {
   std::set intSet;
   std::string str;
+  std::string str2;
   std::wstring wstr;
   str.size() + 0;
   str.size() - 0;
@@ -87,32 +113,73 @@
 ;
   // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used to check for emptiness instead of 'size' [readability-container-size-empty]
   // CHECK-FIXES: {{^  }}if (intSet.empty()){{$}}
-  // CHECK-MESSAGES: :23:8: note: method 'set'::empty() defined here
+  // CHECK-MESSAGES: :32:8: note: method 'set'::empty() defined here
+  if (intSet == std::set())
+;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used to check for emptiness
+  // CHECK-FIXES: {{^  }}if (intSet.empty()){{$}}
+  // CHECK-MESSAGES: :32:8: note: method 'set'::empty() defined here
+  if (s_func() == "")
+;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
+  // CHECK-FIXES: {{^  }}if (s_func().empty()){{$}}
   if (str.size() == 0)
 ;
   // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
   // CHECK-FIXES: {{^  }}if (str.empty()){{$}}
+  if ((str + str2).size() == 0)
+;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
+  // CHECK-FIXES: {{^  }}if ((str + str2).empty()){{$}}
+  if (str == "")
+;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
+  // CHECK-FIXES: {{^  }}if (str.empty()){{$}}
+  if (str + str2 == "")
+;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
+  // CHECK-FIXES: {{^  }}if ((str + str2).empty()){{$}}
   if (wstr.size() == 0)
 ;
   // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
   // CHECK-FIXES: {{^  }}if (wstr.empty()){{$}}
+  if (wstr == "")
+;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
+  // CHECK-FIXES: {{^  }}if (wstr.empty()){{$}}
   std::vector vect;
   if (vect.size() == 0)
 ;
   // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
   // CHECK-FIXES: {{^  }}if (vect.empty()){{$}}
+  if (vect == std::vector())
+;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
+  // CHECK-FIXES: {{^  }}if (vect.empty()){{$}}
   if (vect.size() != 0)
 ;
   // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
   // CHECK-FIXES: {{^  }}if (!vect.empty()){{$}}
+  if (vect != std::vector())
+;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
+  // CHECK-FIXES: {{^  }}if (!vect.empty()){{$}}
   if (0 == vect.size())
 ;
   // CHECK-MESSAGES: :[[@LINE-2]]:12: warning: the 'empty' method

[PATCH] D32210: [Sema][ObjC] Add support for attribute "noescape"

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added a comment.

In https://reviews.llvm.org/D32210#730553, @jroelofs wrote:

> This doesn't forbid assigning them to block properties... is that intentional?
>
>   typedef void (^Block)(int);
>  
>   @interface Foo
>   @property Block B;
>   @end
>  
>   void foo(Foo *f, Block __attribute__((noescape)) b) {
> f.B = b;
>   }
>


No, it's not intentional. The code to check assignments of noescape blocks 
probably had to go to ActOnBinOp or BuildBinOp to forbid this too.




Comment at: include/clang/Basic/AttrDocs.td:122
+* Cannot be returned from a function
+* Cannot be captured by a block
+* Cannot be assigned to a variable

TheRealAdamKemp wrote:
> This may be too restrictive in some cases. Consider this:
> 
> ```
> typedef void (^BlockTy)();
> void nonescapingFunc(__attribute__((noescape)) BlockTy);
> 
> void callerFunc(__attribute__((noescape)) BlockTy block) {
>   nonescapingFunc(^{ block(); });
> }
> ```
> 
> It sounds like the above code would give an error, but the capturing block is 
> also nonescaping, which means it should be allowed. This is a useful pattern, 
> and disallowing it would make these attributes very cumbersome.
> 
> The code could also be written like this:
> 
> ```
> typedef void (^BlockTy)();
> void nonescapingFunc(__attribute__((noescape)) BlockTy);
> 
> void callerFunc(__attribute__((noescape)) BlockTy block) {
>   BlockTy wrapBlock = ^{
> block();
>   };
>   nonescapingFunc(wrapBlock);
> }
> ```
> 
> Again, I would expect that to not give an error or at least be able to 
> decorate the declaration of `wrapBlock` to indicate that it is also 
> nonescaping (can this attribute be applied to locals or only function 
> arguments?).
I didn't think forbidding capturing noescape blocks would be too restrictive in 
practice, but I see your point. If it's a common pattern, we probably shouldn't 
forbid that.



Comment at: lib/Sema/SemaChecking.cpp:2567
+else
+  assert(FDecl->isVariadic() &&
+ "Called function expected to be variadic");

aaron.ballman wrote:
> What about functions that have no prototype (they're not variadic, but they 
> accept any number of arguments)? Should include a test for this.
You are right, this assertion should check for functions without prototypes too.


https://reviews.llvm.org/D32210



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

[PATCH] D32210: [Sema][ObjC] Add support for attribute "noescape"

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added a comment.

In https://reviews.llvm.org/D32210#730777, @rjmccall wrote:

> The rule we actually want is that no reference to the block derived from the 
> parameter value will survive after the function returns.  You can include 
> examples of things that would cause potential escapes, but trying to actually 
> lock down the list seems ill-advised.
>
> Are you sure you want to try to enforce this in the frontend?  In Swift, we 
> discovered that we needed some way of escaping the static checking in order 
> to make this practical, because there are a lot of patterns that locally look 
> like escapes but really aren't (e.g. passing the block to dispatch_async and 
> then waiting for that to complete).  Seems more like a static analysis than a 
> frontend warning.


I'm actually not so sure we want to enforce this in the front-end. I was 
following what I thought Swift was doing, which is to reject code that can 
potentially cause noescape closures to escape. But that might be too 
restrictive and can reject legitimate code in some cases as Adam pointed out. I 
agree that static analysis would probably be better at detecting usages of 
noescape blocks that are likely to cause them to escape.

Do you think we should remove all the restrictions I listed and instead count 
on the users to ensure noescape blocks do not escape?


https://reviews.llvm.org/D32210



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

r300776 - [sanitizer-coverage] trim down the docs

[Kostya Serebryany via cfe-commits]

Author: kcc
Date: Wed Apr 19 17:25:30 2017
New Revision: 300776

URL: http://llvm.org/viewvc/llvm-project?rev=300776&view=rev
Log:
[sanitizer-coverage] trim down the docs

Modified:
cfe/trunk/docs/SanitizerCoverage.rst

Modified: cfe/trunk/docs/SanitizerCoverage.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/SanitizerCoverage.rst?rev=300776&r1=300775&r2=300776&view=diff
==
--- cfe/trunk/docs/SanitizerCoverage.rst (original)
+++ cfe/trunk/docs/SanitizerCoverage.rst Wed Apr 19 17:25:30 2017
@@ -25,17 +25,10 @@ following compile-time flags:
   **extra** slowdown).
 * ``-fsanitize-coverage=edge`` for edge-level coverage (up to 40% slowdown).
 
-You may also specify ``-fsanitize-coverage=indirect-calls`` for
-additional `caller-callee coverage`_.
-
 At run time, pass ``coverage=1`` in ``ASAN_OPTIONS``,
 ``LSAN_OPTIONS``, ``MSAN_OPTIONS`` or ``UBSAN_OPTIONS``, as
 appropriate. For the standalone coverage mode, use ``UBSAN_OPTIONS``.
 
-To get `Coverage counters`_, add ``-fsanitize-coverage=8bit-counters``
-to one of the above compile-time flags. At runtime, use
-``*SAN_OPTIONS=coverage=1:coverage_counters=1``.
-
 Example:
 
 .. code-block:: console
@@ -199,110 +192,6 @@ edges by introducing new dummy blocks an
 |/
 C
 
-Bitset
-==
-
-**coverage_bitset=1 is deprecated, don't use**
-
-Caller-callee coverage
-==
-
-**Deprecated, don't use**
-
-Every indirect function call is instrumented with a run-time function call that
-captures caller and callee.  At the shutdown time the process dumps a separate
-file called ``caller-callee.PID.sancov`` which contains caller/callee pairs as
-pairs of lines (odd lines are callers, even lines are callees)
-
-.. code-block:: console
-
-a.out 0x4a2e0c
-a.out 0x4a6510
-a.out 0x4a2e0c
-a.out 0x4a87f0
-
-Current limitations:
-
-* Only the first 14 callees for every caller are recorded, the rest are 
silently
-  ignored.
-* The output format is not very compact since caller and callee may reside in
-  different modules and we need to spell out the module names.
-* The routine that dumps the output is not optimized for speed
-* Only Linux x86_64 is tested so far.
-* Sandboxes are not supported.
-
-Coverage counters
-=
-
-**Deprecated, don't use**
-
-This experimental feature is inspired by
-`AFL `__'s coverage
-instrumentation. With additional compile-time and run-time flags you can get
-more sensitive coverage information.  In addition to boolean values assigned to
-every basic block (edge) the instrumentation will collect imprecise counters.
-On exit, every counter will be mapped to a 8-bit bitset representing counter
-ranges: ``1, 2, 3, 4-7, 8-15, 16-31, 32-127, 128+`` and those 8-bit bitsets 
will
-be dumped to disk.
-
-.. code-block:: console
-
-% clang++ -g cov.cc -fsanitize=address 
-fsanitize-coverage=edge,8bit-counters
-% ASAN_OPTIONS="coverage=1:coverage_counters=1" ./a.out
-% ls -l *counters-sancov
-... a.out.17110.counters-sancov
-% xxd *counters-sancov
-000: 0001 0100 01
-
-These counters may also be used for in-process coverage-guided fuzzers. See
-``include/sanitizer/coverage_interface.h``:
-
-.. code-block:: c++
-
-// The coverage instrumentation may optionally provide imprecise counters.
-// Rather than exposing the counter values to the user we instead map
-// the counters to a bitset.
-// Every counter is associated with 8 bits in the bitset.
-// We define 8 value ranges: 1, 2, 3, 4-7, 8-15, 16-31, 32-127, 128+
-// The i-th bit is set to 1 if the counter value is in the i-th range.
-// This counter-based coverage implementation is *not* thread-safe.
-
-// Returns the number of registered coverage counters.
-uintptr_t __sanitizer_get_number_of_counters();
-// Updates the counter 'bitset', clears the counters and returns the 
number of
-// new bits in 'bitset'.
-// If 'bitset' is nullptr, only clears the counters.
-// Otherwise 'bitset' should be at least
-// __sanitizer_get_number_of_counters bytes long and 8-aligned.
-uintptr_t
-__sanitizer_update_counter_bitset_and_clear_counters(uint8_t *bitset);
-
-Tracing basic blocks
-
-
-**Deprecated, don't use**
-
-Experimental support for basic block (or edge) tracing.
-With ``-fsanitize-coverage=trace-bb`` the compiler will insert
-``__sanitizer_cov_trace_basic_block(s32 *id)`` before every function, basic 
block, or edge
-(depending on the value of ``-fsanitize-coverage=[func,bb,edge]``).
-Example:
-
-.. code-block:: console
-
-% clang -g -fsanitize=address -fsanitize-coverage=edge,trace-bb foo.cc
-% ASAN_OPTIONS=coverage=1 ./a.out
-
-This will produce two files after the process exit:
-`trace-points.PID.sancov` and `trace-events.PID.sancov`.
-The first file will contain a textual description of al

[PATCH] D32064: [asan] Disable ASan global-GC depending on the target and compiler flags

[Evgeniy Stepanov via Phabricator via cfe-commits]

eugenis added a comment.

The ultimate solution would be a function attribute - if we want this thing to 
work correctly with LTO. But it sounds a bit like overkill, plus none of the 
settings it depends on (integrated-as, data-sections) work with LTO anway: the 
former is ignored and the second does not make sense.


Repository:
  rL LLVM

https://reviews.llvm.org/D32064



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

[PATCH] D32064: [asan] Disable ASan global-GC depending on the target and compiler flags

[Peter Collingbourne via Phabricator via cfe-commits]

pcc added a comment.

I won't stand in the way here if others feel strongly that the flag should be 
passed via a constructor. It will just mean more work to be done if/when this 
flag is ever changed to be passed via some other mechanism, but that's a 
relatively minor detail.


Repository:
  rL LLVM

https://reviews.llvm.org/D32064



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

[PATCH] D32199: [TBAASan] A TBAA Sanitizer (Clang)

[Hal Finkel via Phabricator via cfe-commits]

hfinkel added a comment.

In https://reviews.llvm.org/D32199#731144, @rsmith wrote:

>


...

> I would also expect that we will extend this in future to assign types to 
> storage even in cases where there is no store (for instance, we should be 
> able to catch `float f() { int n; return *(float*)&n; }` despite there being 
> no TBAA violation in the naive IR).

Yes. My thought was that we'd make Clang generate tbaa metadata on allocas and 
lifetime.start intrinsics (and globals) so that we can mark the memory types 
upon creation. Would that catch everything?

> How about renaming this to something more like `-fsanitize=type`?

I'm fine with that. Do you like TypeSanitizer or TypeAccessSantizer or 
TypeAliasingSanitizer best?

One potential concern with calling it the type sanitizer is that we have an 
abbreviation overlap with the thread sanitizer.


https://reviews.llvm.org/D32199



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

[PATCH] D32248: CodeGen: Cast alloca to expected address space

[Tony Tye via Phabricator via cfe-commits]

t-tye added inline comments.



Comment at: lib/CodeGen/CGDecl.cpp:1105-1119
+  // Alloca always returns a pointer in alloca address space, which may
+  // be different from the type defined by the language. For example,
+  // in C++ the auto variables are in the default address space. Therefore
+  // cast alloca to the expected address space when necessary.
+  auto Addr = address.getPointer();
+  auto AddrTy = cast(Addr->getType());
+  auto ExpectedAddrSpace = 
CGM.getTypes().getVariableType(D)->getAddressSpace();

Is any assert done to ensure that it is legal to address space cast from 
variable address space to expected address space? Presumably the language, by 
definition, will only be causing legal casts. For example from alloca address 
space to generic (which includes the alloca address space).

For OpenCL, can you explain how the local variable can have the constant 
address space and use an alloca for allocation? Wouldn't a constant address 
space mean it was static and so should not be using alloca? And if it is using 
an alloca, how can it then be accessed as if it was in constant address space?



Comment at: lib/CodeGen/CodeGenTypes.h:200
+  /// Get the LLVM pointer type of a variable.
+  llvm::PointerType *getVariableType(VarDecl D);
+

Should the name reflect that the type returned is not the variable type, but a 
pointer to the variable type? For example, getVariablePointerType or 
getPointerToVariableType.


https://reviews.llvm.org/D32248



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

[PATCH] D32199: [TBAASan] A TBAA Sanitizer (Clang)

[Hal Finkel via Phabricator via cfe-commits]

hfinkel added a comment.

In https://reviews.llvm.org/D32199#731249, @hfinkel wrote:

> In https://reviews.llvm.org/D32199#731144, @rsmith wrote:
>
> >
>
>
> ...
>
> > I would also expect that we will extend this in future to assign types to 
> > storage even in cases where there is no store (for instance, we should be 
> > able to catch `float f() { int n; return *(float*)&n; }` despite there 
> > being no TBAA violation in the naive IR).
>
> Yes. My thought was that we'd make Clang generate tbaa metadata on allocas 
> and lifetime.start intrinsics (and globals) so that we can mark the memory 
> types upon creation. Would that catch everything?


Also, we'd also want to do something similar for byval arguments. This may just 
be additional motivation to allow metadata on function arguments (there's an 
RFC on llvm-dev about this presently).

> 
> 
>> How about renaming this to something more like `-fsanitize=type`?
> 
> I'm fine with that. Do you like TypeSanitizer or TypeAccessSantizer or 
> TypeAliasingSanitizer best?
> 
> One potential concern with calling it the type sanitizer is that we have an 
> abbreviation overlap with the thread sanitizer.




https://reviews.llvm.org/D32199



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

[PATCH] D32210: [Sema][ObjC] Add support for attribute "noescape"

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In https://reviews.llvm.org/D32210#731192, @ahatanak wrote:

> In https://reviews.llvm.org/D32210#730777, @rjmccall wrote:
>
> > The rule we actually want is that no reference to the block derived from 
> > the parameter value will survive after the function returns.  You can 
> > include examples of things that would cause potential escapes, but trying 
> > to actually lock down the list seems ill-advised.
> >
> > Are you sure you want to try to enforce this in the frontend?  In Swift, we 
> > discovered that we needed some way of escaping the static checking in order 
> > to make this practical, because there are a lot of patterns that locally 
> > look like escapes but really aren't (e.g. passing the block to 
> > dispatch_async and then waiting for that to complete).  Seems more like a 
> > static analysis than a frontend warning.
>
>
> I'm actually not so sure we want to enforce this in the front-end. I was 
> following what I thought Swift was doing, which is to reject code that can 
> potentially cause noescape closures to escape. But that might be too 
> restrictive and can reject legitimate code in some cases as Adam pointed out. 
> I agree that static analysis would probably be better at detecting usages of 
> noescape blocks that are likely to cause them to escape.
>
> Do you think we should remove all the restrictions I listed and instead count 
> on the users to ensure noescape blocks do not escape?


Yes, I think so.  You'll need to document that, of course.

John.


https://reviews.llvm.org/D32210



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

[PATCH] D31885: Remove TBAA information from LValues representing union members

[Hal Finkel via Phabricator via cfe-commits]

hfinkel added a comment.

In https://reviews.llvm.org/D31885#730853, @hfinkel wrote:

>


...

> 
> 
>> (And the nonsensicality of the standard very much continues. The code that 
>> we've all agreed is obviously being miscompiled here is still a strict 
>> violation of the "improved" union rules in C++, and if we decide to treat it 
>> as ill-formed we're going to break a massive amount of code (and vector code 
>> in particular heavily uses unions), because trust me, nobody is reliably 
>> placement-newing union fields when they're non-trivial. C++'s new rules are 
>> unworkable in part because they rely on C++-specific features that cannot be 
>> easily adopted in headers which need to be language-neutral.)
> 
> I think we all agree that the vector types need to have their scalar 
> (element) types as parents in the current TBAA representation. That's a 
> separate change that we should just make. That, unfortunately, only fixes a 
> subset of the union-related miscompiles.

Looking at this in more detail, we actually generate builtin-vector types which 
use char* as their access type (always, even for floating-point vectors). It is 
only the struct-path info which causes the issue for the test case here. It 
might be reasonable to make this **less** conservative, but that's another 
story...


Repository:
  rL LLVM

https://reviews.llvm.org/D31885



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

[PATCH] D32199: [TBAASan] A TBAA Sanitizer (Clang)

[Richard Smith via Phabricator via cfe-commits]

rsmith added a comment.

> ! In https://reviews.llvm.org/D32199#731252, @hfinkel wrote:
> 
>> How about renaming this to something more like `-fsanitize=type`?
> 
> I'm fine with that. Do you like TypeSanitizer or TypeAccessSantizer or 
> TypeAliasingSanitizer best?

I think calling it a type aliasing sanitizer would somewhat conflate the 
details of the mechanism with the fundamentals of the check itself. For example:

  variant v;
  int &n = v.get;
  v = 1.3f;
  int m = n;

... is a lifetime bug, not an aliasing bug, but would be caught by this check 
just the same. I'd be tempted to suggest EffectiveTypeSanitizer, since we seem 
to be more-or-less directly implementing C's effective type rules, except that 
name isn't so good for the C++ case. And in the longer term we will probably 
want to provide an option to enforce the real C++ lifetime rules whereby a 
store with certain !tbaa metadata is not sufficient to change the type of 
storage.

> One potential concern with calling it the type sanitizer is that we have an 
> abbreviation overlap with the thread sanitizer.

Perhaps we could abbreviate it as "tysan"? *shrug*


https://reviews.llvm.org/D32199



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