[PATCH] D29724: [Driver] Report available language standards on user error

2017-02-10 Thread Paweł Żukowski via Phabricator via cfe-commits 
idlecode added a comment.

Indeed, thanks :)
I ran `make check-all` and had no errors so I thought there are no tests.




Comment at: lib/Frontend/CompilerInvocation.cpp:1709
+Diags.Report(diag::note_drv_supported_value_with_description)
+  << Std.getName() << Std.getDescription();
+  }

ahatanak wrote:
> Is it possible to change the diagnostic so that it's easier to tell which 
> part is the supported value and which part is the description?
> 
> The diagnostic looks like this, and I found it a little hard to tell at a 
> quick glance:
> 
> "c89 - ISO C 1990" 
Sure, I have tried few formats with quotes/colons but how about this (a bit 
verbose) version:
```
note: supported values are:
note: 'c89' for standard 'ISO C 1990'
note: 'c90' for standard 'ISO C 1990'
note: 'iso9899:1990' for standard 'ISO C 1990'
...
```
What do you think about it? Do you have any suggestions?


https://reviews.llvm.org/D29724



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

r294772 - clang-format: don't break code using __has_include, PR31908

2017-02-10 Thread Nico Weber via cfe-commits 
Author: nico
Date: Fri Feb 10 13:36:52 2017
New Revision: 294772

URL: http://llvm.org/viewvc/llvm-project?rev=294772=rev
Log:
clang-format: don't break code using __has_include, PR31908

Modified:
cfe/trunk/lib/Format/FormatToken.h
cfe/trunk/lib/Format/TokenAnnotator.cpp
cfe/trunk/unittests/Format/FormatTest.cpp

Modified: cfe/trunk/lib/Format/FormatToken.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/FormatToken.h?rev=294772=294771=294772=diff
==
--- cfe/trunk/lib/Format/FormatToken.h (original)
+++ cfe/trunk/lib/Format/FormatToken.h Fri Feb 10 13:36:52 2017
@@ -630,6 +630,8 @@ struct AdditionalKeywords {
 kw_synchronized = ("synchronized");
 kw_throws = ("throws");
 kw___except = ("__except");
+kw___has_include = ("__has_include");
+kw___has_include_next = ("__has_include_next");
 
 kw_mark = ("mark");
 
@@ -656,6 +658,8 @@ struct AdditionalKeywords {
   IdentifierInfo *kw_NS_ENUM;
   IdentifierInfo *kw_NS_OPTIONS;
   IdentifierInfo *kw___except;
+  IdentifierInfo *kw___has_include;
+  IdentifierInfo *kw___has_include_next;
 
   // JavaScript keywords.
   IdentifierInfo *kw_as;

Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=294772=294771=294772=diff
==
--- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp Fri Feb 10 13:36:52 2017
@@ -684,6 +684,12 @@ private:
   if (Contexts.back().IsForEachMacro)
 Contexts.back().IsExpression = true;
   break;
+case tok::identifier:
+  if (Tok->isOneOf(Keywords.kw___has_include,
+   Keywords.kw___has_include_next)) {
+parseHasInclude();
+  }
+  break;
 default:
   break;
 }
@@ -727,6 +733,14 @@ private:
 }
   }
 
+  void parseHasInclude() {
+if (!CurrentToken || !CurrentToken->is(tok::l_paren))
+  return;
+next();  // '('
+parseIncludeDirective();
+next();  // ')'
+  }
+
   LineType parsePreprocessorDirective() {
 bool IsFirstToken = CurrentToken->IsFirst;
 LineType Type = LT_PreprocessorDirective;
@@ -777,8 +791,14 @@ private:
 default:
   break;
 }
-while (CurrentToken)
+while (CurrentToken) {
+  FormatToken *Tok = CurrentToken;
   next();
+  if (Tok->isOneOf(Keywords.kw___has_include,
+   Keywords.kw___has_include_next)) {
+parseHasInclude();
+  }
+}
 return Type;
   }
 

Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=294772=294771=294772=diff
==
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Fri Feb 10 13:36:52 2017
@@ -5364,6 +5364,11 @@ TEST_F(FormatTest, HandlesIncludeDirecti
 
   verifyFormat("#define MY_IMPORT ");
 
+  verifyFormat("#if __has_include()");
+  verifyFormat("#if __has_include_next()");
+  verifyFormat("#define F __has_include()");
+  verifyFormat("#define F __has_include_next()");
+
   // Protocol buffer definition or missing "#".
   verifyFormat("import \"a/aaa\";",
getLLVMStyleWithColumns(30));


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

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

2017-02-10 Thread Hal Finkel via Phabricator via cfe-commits 
hfinkel added a comment.

In https://reviews.llvm.org/D28404#673260, @mehdi_amini wrote:

> Ping :)


To clarify my understanding of this thread, it seems like there are three ways 
forward here:

1. To have -O0 add optnone to the generated functions (enabling some degree of 
lack of optimization of those functions even when used with -flto)
2. To have -O0 -flto essentially turn off LTO (so that we get unoptimized 
objects directly for things we're debugging)
3. Add a separate flag to make optnone the default

(1) is this patch. The disadvantage of (2) is that it also precludes CFI (and 
other whole-program transformations). This seems highly unfortunate at best and 
a non-starter in the general case. The disadvantage of (3) is that it might 
seems confusing to users (i.e. how to explain the difference between -O0 and 
-foptimize-off?) and is an unnecessary exposure to users of implementation 
details. On this point I agree.

It is true that -O0 != optnone, in a technical sense, but in the end, both are 
best effort. Moreover, there is a tradeoff between disabling optimization of 
the functions you don't want to optimize and keeping the remainder of the code 
as similar as possible to how it would be if everything were being optimized. 
What optnone provides seems like a reasonable point in that tradeoff space. I 
think that we should move forward with this approach.


https://reviews.llvm.org/D28404



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

[PATCH] D29819: Introduce an 'external_source_symbol' attribute that describes the origin and the nature of a declaration

2017-02-10 Thread Aaron Ballman via Phabricator via cfe-commits 
aaron.ballman requested changes to this revision.
aaron.ballman added inline comments.
This revision now requires changes to proceed.



Comment at: include/clang/Basic/Attr.td:532
+  let Spellings = [GNU<"external_source_symbol">,
+   CXX11<"gnu", "external_source_symbol">];
+  let Args = [IdentifierArgument<"language", 1>,

Is this supported by GCC? If not, it should be under the clang namespace 
instead of the gnu namespace. If it is supported by GCC, then you should just 
use a single GCC spelling.



Comment at: include/clang/Basic/AttrDocs.td:1007
+
+defined_in=\ *string-literal*
+  The name of the source container in which the declaration was defined. The

Would this hold something like a file name? If so, I worry about conflicts 
between the comma separator and a file name -- you might want to pick a 
separator that can't be used in a file name (like | or :).



Comment at: include/clang/Basic/AttrDocs.td:1015
+  This declaration was automatically generated by some tool.
+  }];
+}

Are these clauses parsed in a strict order? If so, you may want to mention that 
the order matters (or doesn't).

Also, the code in SemaDeclAttr.cpp implies that some of these are optional. It 
should be made clear which (if any) arguments are optional.



Comment at: include/clang/Basic/DiagnosticParseKinds.td:863
+def err_external_source_symbol_expected_language : Error<
+  "expected a source language , e.g., 'Swift'">;
+

I would drop the e.g. and instead describe what's really expected: an 
identifier. The e.g. muddies the water because it suggests there's a list of 
supported languages, and it shows something that looks kind of like a string 
when it isn't one. Something like: `expected an identifier representing the 
source language` or some such?



Comment at: lib/Parse/ParseDecl.cpp:1141
+} else {
+  // Keyword must be 'Ident_defined_in'.
+  if (Tok.isNot(tok::string_literal)) {

You may want to assert this.



Comment at: lib/Sema/SemaDeclAttr.cpp:2414
+   const AttributeList ) {
+  if (!checkAttributeAtLeastNumArgs(S, Attr, 1))
+return;

You should also diagnose if there's more than 3 arguments, no?



Comment at: lib/Sema/SemaDeclAttr.cpp:2418
+  if (!isa(D)) {
+S.Diag(Attr.getLoc(), diag::warn_attribute_ignored) << Attr.getName();
+return;

This isn't really the right diagnostic for a mismatched attribute subject. It 
should be using `warn_attribute_wrong_decl_type` instead so that the user is 
more clear on why the attribute is ignored.



Comment at: lib/Sema/SemaDeclAttr.cpp:2428
+DefinedIn = SE->getString();
+  bool IsGeneratedDeclaration = Attr.getArgAsIdent(2);
+

Rather than rely on the implicit conversion to bool, I think this is a case 
where it should be tested against `nullptr` explicitly.



Comment at: test/Parser/attr-external-source-symbol-cxx11.cpp:6
+
+// expected-no-diagnostics

Please put this directly below the RUN line.



Comment at: test/Parser/attr-external-source-symbol.m:26
+void f6()
+__attribute__((external_source_symbol(defined_in=20))); // expected-error 
{{expected string literal for source container name in 'external_source_symbol' 
attribute}}

I think you can currently get away with writing 
`external_source_symbol(generated_declaration, generated_declaration, 
generated_declaration, defined_in="module"))` and other odd combinations.

There should be additional parser tests for totally crazy parsing scenarios. 
You may want to consider running a fuzzer to generate some of them.



Comment at: test/Sema/attr-external-source-symbol.c:5
+
+void f2() __attribute__((external_source_symbol(generated_declaration)));
+

Should also have a sema test for when there are 2 args, and 4 args.

There should also be a test under Misc that also checks that the args are 
properly lowered into the AST in the correct way with differing argument 
orders. e.g, `external_source_symbol(generated_declaration, language=Swift, 
defined_in="module")` vs `external_source_symbol(language=Swift, 
generated_declaration, defined_in="module")` vs 
`external_source_symbol(defined_in="module", language=Swift, 
generated_declaration)`, etc.


Repository:
  rL LLVM

https://reviews.llvm.org/D29819



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

[PATCH] D29839: [clang-tidy] New misc-istream-overflow check

2017-02-10 Thread Piotr Padlewski via Phabricator via cfe-commits 
Prazek added a comment.

Nice check! :)




Comment at: clang-tidy/misc/IstreamOverflowCheck.cpp:59-61
+  if (ConstType) {
+ArraySize = ConstType->getSize();
+  }

same here



Comment at: clang-tidy/misc/IstreamOverflowCheck.cpp:78-80
+if (!Arg->isIntegerConstantExpr(WidthSize, Context)) {
+  llvm::errs() << "Couldn't get width() size.\n";
+}

debug?



Comment at: clang-tidy/misc/IstreamOverflowCheck.cpp:111-116
+if (HasWidthCall && WidthSize != 0) {
+  Width = WidthSize;
+}
+if (HasSetwCall && SetwSize != 0) {
+  Width = SetwSize;
+}

please remove unnecessary braces



Comment at: clang-tidy/misc/IstreamOverflowCheck.cpp:125-132
+  if (HasSetwCall) {
+diag(SetwCall->getLocation(), "std::setw called here",
+ DiagnosticIDs::Note);
+  }
+  if (HasWidthCall) {
+diag(WidthCall->getExprLoc(), "width called here",
+ DiagnosticIDs::Note);

same here


Repository:
  rL LLVM

https://reviews.llvm.org/D29839



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

r294773 - [c++1z] Disallow deduction guides with deduced types that don't syntactically match the template being deduced.

2017-02-10 Thread Richard Smith via cfe-commits 
Author: rsmith
Date: Fri Feb 10 13:49:50 2017
New Revision: 294773

URL: http://llvm.org/viewvc/llvm-project?rev=294773=rev
Log:
[c++1z] Disallow deduction guides with deduced types that don't syntactically 
match the template being deduced.

Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/Sema/SemaDeclCXX.cpp
cfe/trunk/test/CXX/temp/temp.deduct.guide/p3.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=294773=294772=294773=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Fri Feb 10 13:49:50 
2017
@@ -1981,6 +1981,9 @@ def err_deduced_class_template_explicit
   "%select{constructor|deduction guide}1 for copy-list-initialization">;
 def err_deduction_guide_no_trailing_return_type : Error<
   "deduction guide declaration without trailing return type">;
+def err_deduction_guide_bad_trailing_return_type : Error<
+  "deduced type %1 of deduction guide is not %select{|written as }2"
+  "a specialization of template %0">;
 def err_deduction_guide_with_complex_decl : Error<
   "cannot specify any part of a return type in the "
   "declaration of a deduction guide">;

Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=294773=294772=294773=diff
==
--- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Fri Feb 10 13:49:50 2017
@@ -8192,12 +8192,46 @@ void Sema::CheckDeductionGuideDeclarator
diag::err_deduction_guide_no_trailing_return_type);
   break;
 }
+
+// Check that the return type is written as a specialization of
+// the template specified as the deduction-guide's name.
+ParsedType TrailingReturnType = Chunk.Fun.getTrailingReturnType();
+TemplateName GuidedTemplate = D.getName().TemplateName.get().get();
+TypeSourceInfo *TSI = nullptr;
+QualType RetTy = GetTypeFromParser(TrailingReturnType, );
+assert(TSI && "deduction guide has valid type but invalid return type?");
+bool AcceptableReturnType = false;
+bool MightInstantiateToSpecialization = false;
+if (auto RetTST =
+TSI->getTypeLoc().getAs()) {
+  TemplateName SpecifiedName = RetTST.getTypePtr()->getTemplateName();
+  bool TemplateMatches =
+  Context.hasSameTemplateName(SpecifiedName, GuidedTemplate);
+  if (SpecifiedName.getKind() == TemplateName::Template && TemplateMatches)
+AcceptableReturnType = true;
+  else {
+// This could still instantiate to the right type, unless we know it
+// names the wrong class template.
+auto *TD = SpecifiedName.getAsTemplateDecl();
+MightInstantiateToSpecialization = !(TD && isa(TD) 
&&
+ !TemplateMatches);
+  }
+} else if (!RetTy.hasQualifiers() && RetTy->isDependentType()) {
+  MightInstantiateToSpecialization = true;
+}
+
+if (!AcceptableReturnType) {
+  Diag(TSI->getTypeLoc().getLocStart(),
+   diag::err_deduction_guide_bad_trailing_return_type)
+<< GuidedTemplate << TSI->getType() << MightInstantiateToSpecialization
+<< TSI->getTypeLoc().getSourceRange();
+}
+
+// Keep going to check that we don't have any inner declarator pieces (we
+// could still have a function returning a pointer to a function).
 FoundFunction = true;
   }
 
-  // FIXME: Check that the return type can instantiate to a specialization of
-  // the template specified as the deduction-guide's name.
-
   if (D.isFunctionDefinition())
 Diag(D.getIdentifierLoc(), diag::err_deduction_guide_defines_function);
 }

Modified: cfe/trunk/test/CXX/temp/temp.deduct.guide/p3.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/temp/temp.deduct.guide/p3.cpp?rev=294773=294772=294773=diff
==
--- cfe/trunk/test/CXX/temp/temp.deduct.guide/p3.cpp (original)
+++ cfe/trunk/test/CXX/temp/temp.deduct.guide/p3.cpp Fri Feb 10 13:49:50 2017
@@ -32,11 +32,9 @@ template