[PATCH] D35782: [C++2a][Preprocessor] Implement p0306 __VA_OPT__ (Comma omission and comma deletion)

[Faisal Vali via Phabricator via cfe-commits]

faisalv abandoned this revision.
faisalv marked 12 inline comments as done.
faisalv added a comment.

committed as r315840.
https://reviews.llvm.org/rL315840


Repository:
  rL LLVM

https://reviews.llvm.org/D35782



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

[PATCH] D39166: [NFC] Add some assertions to placate my paranoia about sharing a variant bit across FunctionDecl and CXXDeductionGuideDecl - should I do this?

[Faisal Vali via Phabricator via cfe-commits]

faisalv created this revision.
faisalv added a project: clang.
Herald added a subscriber: eraman.

I'd like to harden my patch here: https://reviews.llvm.org/rL316292 by adding 
some assertions.

But since the assertions in Decl,.h (FunctionDecl) require knowledge from 
DeclCXX.h (CXXDeductionGuideDecl),-  my question is: In order to keep the 
member functions inline I factored them out into a separate header file that I 
included in certain areas.

Is this an acceptable pattern?

Or does anyone have any other preferred engineering suggestions?

Thanks!


Repository:
  rL LLVM

https://reviews.llvm.org/D39166

Files:
  include/clang/AST/Decl.h
  include/clang/AST/InlineDeclMembers.h
  include/clang/Sema/Sema.h
  lib/AST/Decl.cpp
  lib/AST/DeclCXX.cpp
  lib/Parse/ParseCXXInlineMethods.cpp

Index: lib/Parse/ParseCXXInlineMethods.cpp
===
--- lib/Parse/ParseCXXInlineMethods.cpp
+++ lib/Parse/ParseCXXInlineMethods.cpp
@@ -13,6 +13,7 @@
 
 #include "clang/Parse/Parser.h"
 #include "clang/AST/DeclTemplate.h"
+#include "clang/AST/InlineDeclMembers.h"
 #include "clang/Parse/ParseDiagnostic.h"
 #include "clang/Parse/RAIIObjectsForParser.h"
 #include "clang/Sema/DeclSpec.h"
Index: lib/AST/DeclCXX.cpp
===
--- lib/AST/DeclCXX.cpp
+++ lib/AST/DeclCXX.cpp
@@ -18,6 +18,7 @@
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/Expr.h"
 #include "clang/AST/ExprCXX.h"
+#include "clang/AST/InlineDeclMembers.h"
 #include "clang/AST/ODRHash.h"
 #include "clang/AST/TypeLoc.h"
 #include "clang/Basic/IdentifierTable.h"
Index: lib/AST/Decl.cpp
===
--- lib/AST/Decl.cpp
+++ lib/AST/Decl.cpp
@@ -23,6 +23,7 @@
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/Expr.h"
 #include "clang/AST/ExprCXX.h"
+#include "clang/AST/InlineDeclMembers.h"
 #include "clang/AST/PrettyPrinter.h"
 #include "clang/AST/Stmt.h"
 #include "clang/AST/TypeLoc.h"
Index: include/clang/Sema/Sema.h
===
--- include/clang/Sema/Sema.h
+++ include/clang/Sema/Sema.h
@@ -22,6 +22,7 @@
 #include "clang/AST/Expr.h"
 #include "clang/AST/ExprObjC.h"
 #include "clang/AST/ExternalASTSource.h"
+#include "clang/AST/InlineDeclMembers.h"
 #include "clang/AST/LocInfoType.h"
 #include "clang/AST/MangleNumberingContext.h"
 #include "clang/AST/NSAPI.h"
Index: include/clang/AST/InlineDeclMembers.h
===
--- include/clang/AST/InlineDeclMembers.h
+++ include/clang/AST/InlineDeclMembers.h
@@ -0,0 +1,37 @@
+//===- InlineDeclMembers.h - Decl.h Members that must be inlined -*- C++ -*-==//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+//  This file defines the Decl subclasses.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_AST_INLINEDECLMEMBERS_H
+#define LLVM_CLANG_AST_INLINEDECLMEMBERS_H
+
+#include "clang/AST/Decl.h"
+#include "clang/AST/DeclCXX.h"
+
+
+inline bool clang::FunctionDecl::willHaveBody() const {
+  assert(!isa(this) &&
+"must not be called on a deduction guide since we share this data "
+"member while giving it different semantics");
+  return WillHaveBody;
+}
+inline void clang::FunctionDecl::setWillHaveBody(bool V) {
+  assert(!isa(this) &&
+"must not be called on a deduction guide since we share this data "
+"member while giving it different semantics");
+  WillHaveBody = V;
+}
+
+
+#endif  //LLVM_CLANG_AST_INLINEDECLMEMBERS_H
+
+
Index: include/clang/AST/Decl.h
===
--- include/clang/AST/Decl.h
+++ include/clang/AST/Decl.h
@@ -1680,7 +1680,9 @@
 
 protected:
   // Since a Deduction Guide [C++17] will never have a body, we can share the
-  // storage, and use a different name.
+  // storage, and use a different name.  Since WillHaveBody is not serialized we
+  // don't need to worry about collisions there.
+
   union {
 /// Indicates if the function declaration will have a body, once we're done
 /// parsing it.
@@ -2074,8 +2076,8 @@
   void setHasSkippedBody(bool Skipped = true) { HasSkippedBody = Skipped; }
 
   /// True if this function will eventually have a body, once it's fully parsed.
-  bool willHaveBody() const { return WillHaveBody; }
-  void setWillHaveBody(bool V = true) { WillHaveBody = V; }
+  bool willHaveBody() const;
+  void setWillHaveBody(bool V = true);
 
   void setPreviousDeclaration(FunctionDecl * PrevDecl);
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-co

[PATCH] D39166: [NFC] Add some assertions to placate my paranoia about sharing a variant bit across FunctionDecl and CXXDeductionGuideDecl - should I do this?

[Faisal Vali via Phabricator via cfe-commits]

faisalv added a comment.

In https://reviews.llvm.org/D39166#903131, @lichray wrote:

> Isn't it already an UB if someone set `WillHaveBody` and but later 
> `IsCopyDeductionCandidate` being read, vice versa?


Yes that would be UB - but I'm not sure I see how that would happen w the 
current implementation.  Hence I thought I'd add the assertions to be certain.


Repository:
  rL LLVM

https://reviews.llvm.org/D39166



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

[PATCH] D41217: [Concepts] Concept Specialization Expressions

[Faisal Vali via Phabricator via cfe-commits]

faisalv added inline comments.



Comment at: lib/Parse/ParseExpr.cpp:223
 ExprResult Parser::ParseConstraintExpression() {
-  // FIXME: this may erroneously consume a function-body as the braced
-  // initializer list of a compound literal
-  //
-  // FIXME: this may erroneously consume a parenthesized rvalue reference
-  // declarator as a parenthesized address-of-label expression
-  ExprResult LHS(ParseCastExpression(/*isUnaryExpression=*/false));
-  ExprResult Res(ParseRHSOfBinaryExpression(LHS, prec::LogicalOr));
-
+  ExprResult Res(ParseExpression());
+  if (Res.isUsable() && !Actions.CheckConstraintExpression(Res.get())) {

Are you sure this only accepts logical-or-epxressions? Unlike Hubert's initial 
implementation here, this seems to parse any expression (i.e including 
unparenthesized commas and assignments)? What am I missing?



Repository:
  rC Clang

https://reviews.llvm.org/D41217



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

[PATCH] D40381: Parse concept definition

[Faisal Vali via Phabricator via cfe-commits]

faisalv added a comment.

Thanks for working on this! :)




Comment at: include/clang/AST/DeclTemplate.h:3035
+  SourceRange getSourceRange() const override LLVM_READONLY {
+return SourceRange(getLocation(), getLocation());
+  }

why not just fix it now?
  return SourceRange(getTemplateParameters()->getTemplateLoc(), 
ConstraintExpr->getLocEnd(); 

?



Comment at: include/clang/AST/RecursiveASTVisitor.h:1725
 
+DEF_TRAVERSE_DECL(ConceptDecl, {})
+

Perhaps something along these lines?
  DEF_TRAVERSE_DECL(ConceptDecl, {

TRY_TO(TraverseTemplateParameterListHelper(D->getTemplateParameters()));

TRY_TO(TraverseStmt(D->getConstraintExpr()));
// FIXME: Traverse all the concept specializations (one we implement 
forming template-ids with them).
  })




Comment at: include/clang/Sema/Sema.h:6196
+ const TemplateArgumentListInfo *TemplateArgs);
+
   ExprResult BuildTemplateIdExpr(const CXXScopeSpec &SS,

Have you considered just deferring formation of concept template-ids to the 
next patch (especially since it might require introduction of another AST node 
ConceptSpecializationDecl.  We could just focus on handling concept 
declarations/definitions in this one (and emit an unimplemented error if 
someone tries to form a template-id with a concept within BuildTemplateId etc.) 
- but perhaps consider making sure we handle name clashes/redeclarations with 
any other parsed names in the same namespace (within this patch)?



Comment at: lib/AST/DeclBase.cpp:773
+  return IDNS_Ordinary | IDNS_Tag;
+
 // Never have names.

Pls consider just lumping this case with 'Binding/VarTemplate' (which by the 
way also includes a comment for why we have to add the ugly IDNS_Tag hack here 
(personally i think we should refactor this functionality, currently it seems 
split between this and SemaLookup.cpp:getIDNS and would benefit from some sort 
of well-designed cohesion - but that's another (low-priority) patch)



Comment at: lib/CodeGen/CGDecl.cpp:108
   case Decl::Empty:
+  case Decl::Concept:
 // None of these decls require codegen support.

You would need to add this to EmitTopLevelDecl too to handle global-namespace 
concept declarations.



Comment at: lib/Parse/ParseTemplate.cpp:368
+
+  if (!TryConsumeToken(tok::equal)) {
+Diag(Tok.getLocation(), diag::err_expected) << "equal";

I think we should diagnose qualified-name errors here much more gracefully than 
we do.

i.e. template concept N::C = ...  

- perhaps try and parse a scope-specifier, and if found emit a diagnostic such 
as concepts must be defined within their namespace ...



Comment at: lib/Sema/SemaTemplate.cpp:3899
+  // constraint expressions right now.
+  return Template->getConstraintExpr();
+}

I don't think we want to just return the constraint expr? I think we would need 
to create another conceptspecializationDecl node and nest it within a 
DeclRefExpr?  But once again, I think we should defer this to another patch - 
no?



Comment at: lib/Sema/SemaTemplate.cpp:3923
   bool InstantiationDependent;
-  if (R.getAsSingle() &&
-  !TemplateSpecializationType::anyDependentTemplateArguments(
-   *TemplateArgs, InstantiationDependent)) {
+  bool DependentArguments =
+TemplateSpecializationType::anyDependentTemplateArguments(

const bool, no?



Comment at: lib/Sema/SemaTemplate.cpp:7692
+  MultiTemplateParamsArg TemplateParameterLists,
+   IdentifierInfo *Name, SourceLocation L,
+   Expr *ConstraintExpr) {

Rename L as NameLoc?



Comment at: lib/Sema/SemaTemplate.cpp:7706
+  }
+
+  ConceptDecl *NewDecl = ConceptDecl::Create(Context, DC, L, Name,

Perhaps add a lookup check here? (or a fixme that we are deferring to the next 
patch), something along these *pseudo-code* lines:
   LookupResult Previous(*this, 
DeclarationNameInfo(DeclarationName(Name),NameLoc), LookupOrdinaryName);
   LookupName(Previous, S);
   if (Previous.getRepresentativeDecl())... if the decl is a concept - give 
error regarding only one definition allowed in a TU, if a different kind of 
decl then declared w a different symbol type of error?



Comment at: lib/Sema/SemaTemplate.cpp:7713
+
+  if (!ConstraintExpr->isTypeDependent() &&
+  ConstraintExpr->getType() != Context.BoolTy) {

Consider refactoring these checks on constraint expressions into a separate 
function checkConstraintExpression (that we can also call from other contexts 
such as requires-clauses and nested requires expressions)?



Comment at: lib/Sema/SemaTemplate.cpp:7735
+  Act

[PATCH] D40705: [Parser] Diagnose storage classes in template parameter declarations

[Faisal Vali via Phabricator via cfe-commits]

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

Otherwise, I think this looks good enough to commit.

Do you have commit access? If not, let me know when you're ready for me to 
commit it on your behalf ...

Thank you for fixing this!




Comment at: include/clang/Basic/DiagnosticParseKinds.td:671
 
+def err_storage_class_template_parameter : Error<
+  "storage class specified for template parameter %0">;

What about folding both of these diagnostics into one with something along 
these lines:
def err_storage_class_template_parameter : Error<
  "storage class specified for template parameter %select{|%1}0">;

And for no identifier, do << 0, and for the valid identifier case: do << 1 << 
ParamDecl.getIdentifier()


https://reviews.llvm.org/D40705



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

[PATCH] D40381: Parse concept definition

[Faisal Vali via Phabricator via cfe-commits]

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



Comment at: lib/Sema/SemaTemplate.cpp:3899
+  // constraint expressions right now.
+  return Template->getConstraintExpr();
+}

saar.raz wrote:
> faisalv wrote:
> > I don't think we want to just return the constraint expr? I think we would 
> > need to create another conceptspecializationDecl node and nest it within a 
> > DeclRefExpr?  But once again, I think we should defer this to another patch 
> > - no?
> This was meant as a placeholder. The next patch (D41217) replaces this 
> function, I don't think it is that much of a big deal what happens here in 
> this patch.
This: " I don't think it is that much of a big deal what happens here in this 
patch." I think is poor practice in an open source project when you're not sure 
when the next patch will land.  

I think, when features are implemented incrementally - each patch should 
diagnose the yet to be implemented features - and error out as gracefully as 
possible.  So for instance replacing the body of this function with an 
appropriate diagnostic (that is then removed in future patches) would be the 
better thing to do here.




Comment at: lib/Sema/SemaTemplate.cpp:7713
+
+  if (!ConstraintExpr->isTypeDependent() &&
+  ConstraintExpr->getType() != Context.BoolTy) {

saar.raz wrote:
> faisalv wrote:
> > Consider refactoring these checks on constraint expressions into a separate 
> > function checkConstraintExpression (that we can also call from other 
> > contexts such as requires-clauses and nested requires expressions)?
> I did that in the upcoming patches, no need to do it here as well.
Once again - relying on a future patch (especially without a clear FIXME) to 
tweak the architectural/factorization issues that you have been made aware of 
(and especially those, such as this, that do not require too much effort), is 
not practice I would generally encourage.  

So changyu, if you agree that these suggestions would improve the quality of 
the patch and leave clang in a more robust state (maintenance-wise or 
execution-wise), then please make the changes.  If not, please share your 
thoughts as to why these suggestions would either not be an improvement or be 
inconsistent with the theme of this patch.

Thanks!


https://reviews.llvm.org/D40381



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

[PATCH] D41179: [Sema] Diagnose template specializations with C linkage

[Faisal Vali via Phabricator via cfe-commits]

faisalv added inline comments.



Comment at: lib/Sema/SemaTemplate.cpp:7029
+  }
+
   // C++ [temp.expl.spec]p2:

What do you think about factoring this out into a separate function that is 
then called both from here and CheckTemplateDeclScope (since it is exactly the 
same code?)


https://reviews.llvm.org/D41179



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

[PATCH] D40705: [Parser] Diagnose storage classes in template parameter declarations

[Faisal Vali via Phabricator via cfe-commits]

faisalv requested changes to this revision.
faisalv added a comment.
This revision now requires changes to proceed.

Hmm - I think i might make some tweaks to this patch (to be largely symmetric 
with the similar handling of invalid decl-specifiers on function parameters in 
Sema::Actions.ActOnParamDeclarator)...

Would you want to try to make those changes and update this diff - or would you 
rather I make them and commit a patch acknowledging your contribution?

Sorry about not suggesting the changes earlier - but it occurred to me while I 
was re-reviewing the patch prior to committing it, that we don't handle inline 
and virtual - which led me to ActOnParamDeclarator ...


https://reviews.llvm.org/D40705



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

[PATCH] D40705: [Parser] Diagnose storage classes in template parameter declarations

[Faisal Vali via Phabricator via cfe-commits]

faisalv added a comment.

Sounds good - if I don't get this done over the next seven days - would you 
mind just pinging me!

Thanks!


https://reviews.llvm.org/D40705



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

[PATCH] D40705: Diagnose invalid decl-specifiers in non-type template parameter declarations (original author miyuki!)

[Faisal Vali via Phabricator via cfe-commits]

faisalv updated this revision to Diff 127828.
faisalv retitled this revision from "[Parser] Diagnose storage classes in 
template parameter declarations" to "Diagnose invalid decl-specifiers in 
non-type template parameter declarations (original author miyuki!)".
faisalv edited the summary of this revision.
faisalv set the repository for this revision to rC Clang.
faisalv added projects: clang, clang-c.
faisalv added a comment.

Miyuki - please take a look at the patch and let me know if you agree with the 
changes - or have any concerns...

Thank you!

P.S. While I can see the argument for having these syntactical checks be done 
by the Parser - since they could be described as context sensitive syntax 
analysis, moving the changes to Sema also seems quite appropriate.


Repository:
  rC Clang

https://reviews.llvm.org/D40705

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaTemplate.cpp
  test/CXX/temp/temp.param/p2.cpp

Index: test/CXX/temp/temp.param/p2.cpp
===
--- test/CXX/temp/temp.param/p2.cpp
+++ test/CXX/temp/temp.param/p2.cpp
@@ -1,5 +1,6 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s
-// expected-no-diagnostics
+// RUN: %clang_cc1 -fsyntax-only -verify %s 
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s -DCPP11
+// RUN: %clang_cc1 -std=c++17 -fsyntax-only -verify %s -DCPP17
 
 // There is no semantic difference between class and typename in a
 // template-parameter. typename followed by an unqualified-id names a
@@ -13,8 +14,32 @@
 template::type Value> struct Y1;
 
 // A storage class shall not be specified in a template-parameter declaration.
-template struct Z; // FIXME: expect an error
+template struct Z; //expected-error{{invalid declaration specifier}}
+template struct Z0; //expected-error{{invalid declaration specifier}}
+template struct Z0; //expected-error2{{invalid declaration specifier}}
+template struct Z0; //expected-error{{invalid declaration specifier}}
+template struct Z0; //expected-error{{invalid declaration specifier}}
+template struct Z0; //expected-error{{invalid declaration specifier}}
+template struct Z0; //expected-error{{invalid declaration specifier}}
+template struct Z0;  //expected-error{{invalid declaration specifier}}
+template struct Z0; //expected-error{{invalid declaration specifier}}
+template struct Z0; //expected-error{{invalid declaration specifier}}
 
+template struct Z0; // OK 
+template struct Z0; // OK
+
+
+
+#ifdef CPP11
+template struct Z0; //expected-error{{invalid declaration specifier}}
+template struct Z0; //expected-error{{invalid declaration specifier}}
+
+#endif
+
+#ifdef CPP17
+template struct Z1; // OK
+#endif
+
 // Make sure that we properly disambiguate non-type template parameters that
 // start with 'class'.
 class X1 { };
Index: lib/Sema/SemaTemplate.cpp
===
--- lib/Sema/SemaTemplate.cpp
+++ lib/Sema/SemaTemplate.cpp
@@ -929,6 +929,60 @@
   Expr *Default) {
   TypeSourceInfo *TInfo = GetTypeForDeclarator(D, S);
 
+  // Check that we have valid decl-specifiers specified.
+  auto CheckValidDeclSpecifiers = [this, &D] {
+// C++ [temp.param]
+// p1 
+//   template-parameter:
+// ...
+// parameter-declaration
+// p2 
+//   ... A storage class shall not be specified in a template-parameter
+//   declaration.
+// [dcl.typedef]p1: 
+//   The typedef specifier [...] shall not be used in the decl-specifier-seq
+//   of a parameter-declaration
+const DeclSpec &DS = D.getDeclSpec();
+auto EmitDiag = [this](SourceLocation Loc) {
+  Diag(Loc, diag::err_invalid_decl_specifier_in_nontype_parm)
+  << FixItHint::CreateRemoval(Loc);
+};
+if (DS.getStorageClassSpec() != DeclSpec::SCS_unspecified)
+  EmitDiag(DS.getStorageClassSpecLoc());
+
+if (DeclSpec::TSCS TSCS = DS.getThreadStorageClassSpec())
+  EmitDiag(DS.getThreadStorageClassSpecLoc());
+
+// [dcl.inline]p1: 
+//   The inline specifier can be applied only to the declaration or 
+//   definition of a variable or function.
+
+if (DS.isInlineSpecified())
+  EmitDiag(DS.getInlineSpecLoc());
+
+// [dcl.constexpr]p1:
+//   The constexpr specifier shall be applied only to the definition of a 
+//   variable or variable template or the declaration of a function or 
+//   function template.
+
+if (DS.isConstexprSpecified())
+  EmitDiag(DS.getConstexprSpecLoc());
+
+// [dcl.fct.spec]p1:
+//   Function-specifiers can be used only in function declarations.
+
+if (DS.isVirtualSpecified())
+  EmitDiag(DS.getVirtualSpecLoc());
+
+if (DS.isExplicitSpecified())
+  EmitDiag(DS.getExplicitSpecLoc());
+
+if (DS.isNoreturnSpecified())
+  EmitDiag(DS.getNoreturnSpecLoc());
+  };
+
+  CheckValidDeclSpecifiers();
+  
   if (TInfo->getType()->isUnded

[PATCH] D40705: Diagnose invalid decl-specifiers in non-type template parameter declarations (original author miyuki!)

[Faisal Vali via Phabricator via cfe-commits]

faisalv closed this revision.
faisalv added a comment.

Added via https://reviews.llvm.org/rC321339


Repository:
  rC Clang

https://reviews.llvm.org/D40705



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

[PATCH] D40381: Parse concept definition

[Faisal Vali via Phabricator via cfe-commits]

faisalv added inline comments.



Comment at: lib/Sema/SemaTemplate.cpp:7735
+  ActOnDocumentableDecl(NewDecl);
+  CurContext->addDecl(NewDecl);
+  return NewDecl;

changyu wrote:
> faisalv wrote:
> > Why not use 'PushOnScopeChains' onto the enclosing scope?
> > Something along these lines:
> >assert(S->isTemplateParamScope() && S->getParent() && 
> > !S->getParent()->isTemplateParamScope() && "...");
> >PushOnScopeChains(NewDecl, S->getParent(),/*AddToContext*/true);
> The condition
> ```
> S->isTemplateParamScope()
> ```
> fails in this case
> ```
> template concept D1 = true; // expected-error {{expected template 
> parameter}}
> ```
> 
> `ParseTemplateDeclarationOrSpecialization` calls `ParseTemplateParameters` 
> which eventually calls `ParseNonTypeTemplateParameter`. 
> `ParseNonTypeTemplateParameter` prints the diag and fails but does not cause 
> `ParseTemplateParameters` to fail (I'm not sure why).  
> `ParseTemplateDeclarationOrSpecialization` proceeds to parse the concept 
> definition, and we get this
> 
> ```
> /home/changyu/test.cpp:1:10: error: expected template parameter
> template concept D1 = true; // expected-error {{expected template 
> parameter}}
>  ^
> clang: /home/changyu/git/llvm/tools/clang/lib/Sema/SemaTemplate.cpp:7747: 
> clang::Decl* clang::Sema::ActOnConceptDefinition(clang::Scope*, 
> clang::MultiTemplateParamsArg, clang::IdentifierInfo*, clang::SourceLocation, 
> clang::Expr*): Assertion `S->isTemplateParamScope() && "Not in template param 
> scope?"' failed.
> ```
> 
> What should we do?
I think this occurs because our template parameter list is incorrectly 
perceived as empty - i.e. once the error occurs - to continue processing errors 
clang assumes this case is an explicit-specialization and replaces the template 
parameter scope with the outer scope.  I think the thing to do here - which 
might also address the case where folks actually write 'template<> concept' is 
to actually check if template-parameter-list is empty - and emit a diagnostic 
about concepts can not be explicit specializations or some such ...

On a somewhat related note - i think the logic for signaling, handling and 
propagating failures of parsing the template parameter list might be a little 
broken (fixing which, might avoid triggering the assertion in template 
but not template<>).  I might submit a patch to fix that error handling-issue 
separately - but either way I think we should handle the 
explicit-specialization error?

Thoughts?


https://reviews.llvm.org/D40381



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

[PATCH] D40381: Parse concept definition

[Faisal Vali via Phabricator via cfe-commits]

faisalv added a comment.

I think this looks good enough to commit - do you have commit privileges - or 
do you need one of us to commit it for you?
thank you!


https://reviews.llvm.org/D40381



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

[PATCH] D40381: Parse concept definition

[Faisal Vali via Phabricator via cfe-commits]

faisalv added inline comments.



Comment at: lib/Parse/ParseTemplate.cpp:181
+TemplateParameterList *TPL = ParamLists[0];
+if (TPL->getLAngleLoc().getLocWithOffset(1) == TPL->getRAngleLoc()) {
+  Diag(TPL->getTemplateLoc(),

changyu wrote:
> There's one problem here.
> 
> ​I added this `if` in attempt to catch the following case (but it's wrong)
> ```
>   ​template<> concept D1 = true;  // expected-error {{expected template 
> parameter}}
> ```
> The problem is I'm not sure how to differentiate between the above situation 
> and the following
> ```
>   ​template concept D1 = true; // expected-error {{expected 
> template parameter}}
> ```
> Both have an empty template parameter list​. The latter case has diagnostic 
> printed by `ParseNonTypeTemplateParameter` while the former has not (so we 
> try to catch it here).
> 
> What should we do?
> 

I was thinking that we would just emit a (redundant in the case of a bad 
template parameter) message in Sema if the template-parameters are empty that 
explicit specializations are not allowed here.  while it would be a little 
misleading in the invalid template parameter case - to fix this robustly would 
require some fine-tuning and correcting some of the 
handshaking/error-propagation between the parsing of the template parameters 
and the code that calls it, I think.  I would vote for not holding up this 
patch for that, unless you feel strongly you'd like to fix that behavior - then 
we can try and work on that first?

Thoughts?




https://reviews.llvm.org/D40381



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

[PATCH] D41179: [Sema] Diagnose template specializations with C linkage

[Faisal Vali via Phabricator via cfe-commits]

faisalv added a comment.

In https://reviews.llvm.org/D41179#964522, @miyuki wrote:

> In https://reviews.llvm.org/D41179#957992, @sepavloff wrote:
>
> >   Classes do not have language linkage according to 10.5p1, just as 
> > templates, so this code is valid.
> >  
> >   It looks like defining templates inside extern "C" blocks is OK.
>
>
> Currently both Clang and GCC diagnose class templates declared inside an 
> 'extern "C"' block. I'm not sure how to proceed about this.


Given that Clause 17 p6 specifically uses 'shall' to call out templates - i 
believe the implementation is expected to diagnose this.  10.5p1 (or is there 
another mention in the standard?) does not explicitly call out 'classes' as not 
having c language linkage - so implementations can get away not diagnosing this 
- and perhaps even allow classes to have language linkage on certain 
implementations?? (at least that's my squinty interpretation)


https://reviews.llvm.org/D41179



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

[PATCH] D41217: [Concepts] Concept Specialization Expressions

[Faisal Vali via Phabricator via cfe-commits]

faisalv added inline comments.



Comment at: include/clang/AST/ExprCXX.h:4416
+/// According to C++2a [expr.prim.id]p3 an id-expression that denotes the
+/// specialization of a concepts results in a prvalue of type bool.
+class ConceptSpecializationExpr final : public Expr {

Typo (Hubert): concepts -> concept



Comment at: include/clang/AST/ExprCXX.h:4419
+protected:
+  /// \brief The concept specialization this represents.
+  ConceptDecl *SpecializedConcept;

Hubert: The concept named.



Comment at: include/clang/AST/ExprCXX.h:4420
+  /// \brief The concept specialization this represents.
+  ConceptDecl *SpecializedConcept;
+

NamedConcept



Comment at: include/clang/Sema/Sema.h:5577
+ ConceptDecl *CTD,
+   const TemplateArgumentListInfo *TALI);
+

Hubert: clang-format this



Comment at: include/clang/Sema/Sema.h:5580
+  /// Check whether the given expression is a valid constraint expression.
+  /// A diagnostic is emmited if it is not, and false is returned.
+  bool CheckConstraintExpression(Expr *CE);

Hubert: Typo: emitted



Comment at: lib/AST/ExprCXX.cpp:1441
+ConceptSpecializationExpr::ConceptSpecializationExpr(ASTContext &C, Sema &S,
+  SourceLocation 
ConceptNameLoc,
+ ConceptDecl *CD,

Hubert: clang-format



Comment at: lib/AST/ExprCXX.cpp:1478
+  {
+// We do not want error diagnostics escaping here.
+Sema::SFINAETrap Trap(S);

Hubert: This needs a TODO: the idea is not to drop SFINAE errors, but to avoid 
instantiation that may trigger errors not in the immediate context of 
instantiation. The substitution needs to happen piecewise.



Comment at: lib/AST/ExprCXX.cpp:1485
+  //   constraint is not satisfied.
+  IsSatisfied = false;
+  return true;

Hubert: The name of the function gives no indication that it modifies the 
expression node. The interface of the function extends past what is expected.


Repository:
  rC Clang

https://reviews.llvm.org/D41217



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

[PATCH] D40381: Parse concept definition

[Faisal Vali via Phabricator via cfe-commits]

faisalv added a comment.

I discussed this briefly w Hubert - and i'm planning on modifying this patch 
slightly so that it flows through ParseDeclSpecifier and handles attributes and 
other invalid decl-specifiers such as static etc. more gracefully on a concept 
decl.  I have this partially implemented - my hope is to get this done v 
soonish so feel free to ping me if you don't hear anything about this in a week 
or so ...


https://reviews.llvm.org/D40381



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

[PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas

[Faisal Vali via Phabricator via cfe-commits]

faisalv added inline comments.



Comment at: lib/AST/ExprCXX.cpp:979
 
+SourceRange LambdaExpr::getExplicitTemplateParameterListRange() const {
+  TemplateParameterList *List = getTemplateParameterList();

I think this should return an invalid range if getExplicitCount is 0.
might assert that when not 0, langleloc does not equal rangleloc.




Comment at: lib/Sema/SemaLambda.cpp:486
+ && "Already acted on explicit template parameters");
+  assert(LSI->TemplateParams.size() == 0
+ && "Explicit template parameters should come "

Perhaps also assert TParams.size should not be 0?



Comment at: lib/Sema/SemaLambda.cpp:495
+  reinterpret_cast(TParams.begin()),
+  reinterpret_cast(TParams.end()));
+  LSI->NumExplicitTemplateParams = TParams.size();

ack - avoid reinterpret cast please - why not just stick to Decl* for 
TemplateParams for now  - and add some fixme's that suggest we should consider 
refactoring ParseTemplateParameterList to accept a vector of nameddecls - and 
update this when that gets updated ?

Perhaps add an assert here that iterates through and checks to make sure each 
item in this list is some form of a template parameter decl - within an #ifndef 
NDEBUG block (or your conversion check to NameDecl should suffice?)


https://reviews.llvm.org/D36527



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

[PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas

[Faisal Vali via Phabricator via cfe-commits]

faisalv added a comment.

OK - I'll commit this on sunday if no one blocks it by then.  (I'll add the 
fixme's).

Nice Work!


https://reviews.llvm.org/D36527



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

[PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas

[Faisal Vali via Phabricator via cfe-commits]

faisalv added inline comments.



Comment at: lib/AST/ExprCXX.cpp:979
 
+SourceRange LambdaExpr::getExplicitTemplateParameterListRange() const {
+  TemplateParameterList *List = getTemplateParameterList();

hamzasood wrote:
> faisalv wrote:
> > I think this should return an invalid range if getExplicitCount is 0.
> > might assert that when not 0, langleloc does not equal rangleloc.
> > 
> I think it does return an invalid range in that case. Without explicit 
> template parameters, getLAngleLoc and getRAngleLoc return invalid locations. 
> And a range of invalid locations is an invalid range. But I suppose that 
> could be asserted to make sure.
fyi - Since getGenericLambdaTPL sets the LAngleLoc and RAngleLoc to the 
introducer range - unless one handles that case spearately- those funcs won't 
return an invalid loc?  


https://reviews.llvm.org/D36527



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

[PATCH] D36915: [Sema] Diagnose local variables and parameters captured by lambda and block expressions in a default argument

[Faisal Vali via Phabricator via cfe-commits]

faisalv added a comment.

In https://reviews.llvm.org/D36915#854317, @ahatanak wrote:

> In https://reviews.llvm.org/D36915#849622, @faisalv wrote:
>
> > I don't think this approach is entirely correct for at least the following 
> > reasons:
> >
> > 1. in the lambda case the machinery that diagnoses capture failures should 
> > be the machinery erroring on the lambda (when the parameter is odr-used)
>
>
> Does this mean that it is OK to have a parameter or local variable appear in 
> a potentially-evaluated expression as long as it is not odr-used? I think 
> this is slightly different from the standard, which says a parameter or local 
> variable cannot appear as a potentially-evaluated expression in a default 
> argument.
>
> For example:
>
>   void foo2(int);
>  
>   void func() {
> const int a = 1;
> void foo1(S s = [](){ foo2(a); }); // "a" is not in an unevaluated 
> context and is not odr-used.
>   }
>  
>
>
> I think I need clarification as it will affect how or where we detect this 
> error.


'a' is not captured (and does not need to be captured) above, so I think that 
code should be ok.

But I also think the following code should be ok at local scope within func.
void foo(int = a);

I thought there was a DR against the standard-draft with the intent of making 
these valid (if they are not already so)?


https://reviews.llvm.org/D36915



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

[PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas

[Faisal Vali via Phabricator via cfe-commits]

faisalv added a comment.

In https://reviews.llvm.org/D36527#854600, @rsmith wrote:

> This patch appears to be missing some necessary changes in the following 
> places:
>
> - lib/Serialization -- round-trip the new information through PCH / modules


Shouldn't the explicit template parameter information be automatically 
serialized within the non-implicitness of the corresponding template parameter 
decls within our TemplateParameterList?

> - lib/AST/StmtPrinter.cpp -- pretty-printing lambdas with explicit template 
> parameters

yes - this would be nice.

> - lib/Sema/TreeTransform.h -- template instantiation of lambdas with explicit 
> template arguments

yes - i had wondered about TreeTransform - I had hoped that the transformation 
of the template parameter list (for all generic lambdas) would be sufficient 
(perhaps only need to be tweaked to maintain the implicit flag on template 
parameters, if that's not transferred through?) - but I agree this does need to 
be tested well (substitution into default template arguments, non-type-template 
parameters for e.g.).

> - lib/AST/RecursiveASTVisitor.h -- recursive AST visitation should visit 
> explicit template parameters

Would we also want to (explicitly) visit the implicit template parameters?
Interestingly we don't (explicitly) visit the lambda's function parameters?

> We'll also need to agree within the cxx-abi-dev list how to mangle lambda 
> closure types for this form and lib/AST/ItaniumMangle.cpp's `mangleLambda` 
> will need to be updated to match. (The MS mangling is not based on the type 
> of the lambda call operator, so we probably don't need changes there.)




https://reviews.llvm.org/D36527



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

[PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas

[Faisal Vali via Phabricator via cfe-commits]

faisalv added a comment.

In https://reviews.llvm.org/D36527#854883, @faisalv wrote:

>




> Interestingly we don't (explicitly) visit the lambda's function parameters?

This is obviously false - sorry :)


https://reviews.llvm.org/D36527



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

[PATCH] D40705: [Parser] Diagnose storage classes in template parameter declarations

[Faisal Vali via Phabricator via cfe-commits]

faisalv added inline comments.



Comment at: lib/Parse/ParseTemplate.cpp:692
+  //   declaration.
+  auto ReportStorageClass = [&](SourceLocation Loc) {
+if (ParamDecl.getIdentifier())

I tend to prefer explicit captures (unless you have a good reason?) - favoring 
a clearer description of intention - and acknowledgement of required context - 
over programming convenience ...  


https://reviews.llvm.org/D40705



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

[PATCH] D35782: [C++2a][Preprocessor] Implement p0306 __VA_OPT__ (Comma omission and comma deletion)

[Faisal Vali via Phabricator via cfe-commits]

faisalv updated this revision to Diff 109883.
faisalv marked 7 inline comments as done.
faisalv added a comment.

Hi Richard,

  This patch attempts to incorporate all the changes you requested.  

You asked for some consideration on the rationale for sticking with my approach 
(as opposed to the alternative you posed), so here are my reasons (FWTW) for 
not modeling the VA_OPT as a macro argument:

- the standard does not specify it as behaving exactly as a macro argument 
(specifically no rescanning, after placemarker token removal) - and so i would 
have to special case the argument expansion logic - which currently composes 
well by just launching a tokenlexer on the argument.
- it would be the only argument that would need to have *other* arguments 
substituted into it
- unlike every other argument it would not be represented by just 1 token
- in regards to the source-location mapping, my sense is that the ExpansionInfo 
that represents the various expansions might need to add a representation for 
this 'fourth' kind of expansion (since expanded args are represented by a valid 
LocStart but an invalid LocEnd, and since this is more than one token, things 
could get hairy)
- it might require complicating 'Parsing' the macro definition (and the 
MacroArgs, MacroInfo objects) and the general tokenLexer, 
ExpandFunctionArguments() logic more than any gained conceptual elegance (in my 
opinion)

Thanks Richard - Look forward to your review and thoughts!


Repository:
  rL LLVM

https://reviews.llvm.org/D35782

Files:
  include/clang/Basic/DiagnosticLexKinds.td
  include/clang/Lex/Preprocessor.h
  include/clang/Lex/TokenLexer.h
  include/clang/Lex/VariadicMacroSupport.h
  lib/Lex/PPDirectives.cpp
  lib/Lex/Preprocessor.cpp
  lib/Lex/TokenLexer.cpp
  test/Preprocessor/macro_vaopt_check.cpp
  test/Preprocessor/macro_vaopt_expand.cpp

Index: test/Preprocessor/macro_vaopt_expand.cpp
===
--- test/Preprocessor/macro_vaopt_expand.cpp
+++ test/Preprocessor/macro_vaopt_expand.cpp
@@ -0,0 +1,148 @@
+// RUN: %clang_cc1 -E %s -pedantic -std=c++2a | FileCheck -strict-whitespace %s
+
+#define LPAREN ( 
+#define RPAREN ) 
+
+#define A0 expandedA0
+#define A1  expandedA1 A0
+#define A2  expandedA2 A1
+#define A3  expandedA3 A2
+
+#define A() B LPAREN )
+#define B() C LPAREN )
+#define C() D LPAREN )
+
+
+#define F(x, y) x + y 
+#define ELLIP_FUNC(...) __VA_OPT__(__VA_ARGS__)
+
+1: ELLIP_FUNC(F, LPAREN, 'a', 'b', RPAREN); 
+2: ELLIP_FUNC(F LPAREN 'a', 'b' RPAREN); 
+#undef F
+#undef ELLIP_FUNC
+
+// CHECK: 1: F, (, 'a', 'b', );
+// CHECK: 2: 'a' + 'b';
+
+#define F(...) f(0 __VA_OPT__(,) __VA_ARGS__)
+3: F(a, b, c) // replaced by f(0, a, b, c) 
+4: F() // replaced by f(0)
+
+// CHECK: 3: f(0 , a, b, c) 
+// CHECK: 4: f(0 )
+#undef F
+
+#define G(X, ...) f(0, X __VA_OPT__(,) __VA_ARGS__)
+
+5: G(a, b, c) // replaced by f(0, a , b, c) 
+6: G(a) // replaced by f(0, a) 
+7: G(a,) // replaced by f(0, a) 
+7.1: G(a,,)
+
+
+// CHECK: 5: f(0, a , b, c) 
+// CHECK: 6: f(0, a ) 
+// CHECK: 7: f(0, a ) 
+// CHECK: 7.1: f(0, a , ,)
+#undef G 
+
+#define HT_B() TONG
+
+#define F(x, ...) HT_ ## __VA_OPT__(x x A()  #x)
+
+8: F(1)
+9: F(A(),1)
+
+// CHECK: 8: HT_
+// CHECK: 9: TONG C ( ) B ( ) "A()"
+#undef HT_B
+#undef F
+
+#define F(a,...) #__VA_OPT__(A1 a)
+
+10: F(A())
+11: F(A1 A(), 1)
+// CHECK: 10: ""
+// CHECK: 11: "A1 expandedA1 expandedA0 B ( )"
+#undef F
+
+
+#define F(a,...) a ## __VA_OPT__(A1 a) ## __VA_ARGS__ ## a
+12.0: F()
+12: F(,)
+13: F(B,)
+// CHECK: 12.0: 
+// CHECK: 12: 
+// CHECK: 13: BB 
+#undef F
+
+#define F(...) #__VA_OPT__()  X ## __VA_OPT__()  #__VA_OPT__()
+
+14: F()
+15: F(1)
+
+// CHECK: 14: "" X ""
+// CHECK: 15: "" X ""
+
+#undef F
+
+#define SDEF(sname, ...) S sname __VA_OPT__(= { __VA_ARGS__ })
+
+16: SDEF(foo); // replaced by S foo; 
+17: SDEF(bar, 1, 2); // replaced by S bar = { 1, 2 }; 
+
+// CHECK: 16: S foo ;
+// CHECK: 17: S bar = { 1, 2 }; 
+#undef SDEF
+
+#define F(a,...) A() #__VA_OPT__(A3 __VA_ARGS__ a ## __VA_ARGS__ ## a ## C A3) A()
+
+18: F()
+19: F(,)
+20: F(,A3)
+21: F(A3, A(),A0)
+
+
+// CHECK: 18: B ( ) "" B ( ) 
+// CHECK: 19: B ( ) "" B ( ) 
+// CHECK: 20: B ( ) "A3 expandedA3 expandedA2 expandedA1 expandedA0 A3C A3" B ( )
+// CHECK: 21: B ( ) "A3 B ( ),expandedA0 A3A(),A0A3C A3" B ( )
+
+#undef F
+
+#define F(a,...) A() #__VA_OPT__(A3 __VA_ARGS__ a ## __VA_ARGS__ ## a ## C A3) a __VA_OPT__(A0 __VA_ARGS__ a ## __VA_ARGS__ ## a ## C A0) A()
+
+22: F()
+23: F(,)
+24: F(,A0)
+25: F(A0, A(),A0)
+
+
+// CHECK: 22: B ( ) "" B ( ) 
+// CHECK: 23: B ( ) "" B ( ) 
+// CHECK: 24: B ( ) "A3 expandedA0 A0C A3" expandedA0 expandedA0 A0C expandedA0 B ( )
+// CHECK: 25: B ( ) "A3 B ( ),expandedA0 A0A(),A0A0C A3" expandedA0 expandedA0 C ( ),expandedA0 A0A(),A0A0C expandedA0 B ( )
+
+#undef F
+
+#define F(a,...)  __VA_OPT__(B a ## a) ## 1
+#define G(a,...)  __VA_OPT__(B a) ## 1
+26: F(,1)
+26_1: G(,1)
+// CHECK: 26: B1
+// CHECK: 2

[PATCH] D36572: Implemented P0409R2 - Allow lambda capture [=, this]

[Faisal Vali via Phabricator via cfe-commits]

faisalv added inline comments.



Comment at: lib/Sema/SemaLambda.cpp:959
+ ? diag::ext_equals_this_lambda_capture_cxx2a
+ : diag::warn_cxx1z_compat_equals_this_lambda_capture);
 

Shouldn't we try and hit the 'continue' (that u deleted) if warnings (and 
extension warnings) are turned into errors? 



Comment at: test/SemaCXX/cxx2a-lambda-equals-this.cpp:6
+// Deleting the copy constructor ensures that an [=, this] capture doesn't 
copy the object.
+// Accessing a member variable from the lambda ensures that the capture 
actually works.
+class A {

Nice - I wish we had (and that I had placed) more comments such as these in our 
test files :) 


https://reviews.llvm.org/D36572



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

[PATCH] D36572: Implemented P0409R2 - Allow lambda capture [=, this]

[Faisal Vali via Phabricator via cfe-commits]

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

OK - looks good enough to me.  I say we give the rest of the reviewers until 
friday to chime in, and if no one blocks it, can you commit then?
nice work - thanks!


https://reviews.llvm.org/D36572



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

[PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas

[Faisal Vali via Phabricator via cfe-commits]

faisalv added a comment.

I'll try and get you some feedback on this over the next couple of days (unless 
someone else jumps in).

Thanks for working on this!


https://reviews.llvm.org/D36527



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

[PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas

[Faisal Vali via Phabricator via cfe-commits]

faisalv added inline comments.



Comment at: include/clang/Sema/Sema.h:5466
 
+  /// ActOnLambdaTemplateParameterList - This is called after parsing
+  /// the explicit template parameter list (if it exists) in C++2a.

Avoid listing the name of the function: replace a w a \brief comment.



Comment at: lib/Parse/ParseExprCXX.cpp:1112
 
+  ParseScope TemplateParamScope(this, Scope::TemplateParamScope);
+  if (getLangOpts().CPlusPlus2a && Tok.is(tok::less)) {

We always create a template parameter scope here - but it's only the right 
scope if we have explicit template parameters, correct?

What I think we should do here is :
  - factor out (preferably as a separate patch) the post 
explicit-template-parameter-list parsing into a separate function (F1) which is 
then called from here.
  - then in this patch factor out the explicit template-parameter-list parsing 
also into a separate function that then either calls the function above ('F1'), 
or sequenced these statements before a call to 'F1'
  - also since gcc has had explicit template parameters on their generic 
lambdas for a while, can we find out under what options they have it enabled, 
and consider enabling it under those options for our gcc emulation mode? (or 
add a fixme for it?)
  - should we enable these explicit template parameters for pre-C++2a modes and 
emit extension/compatibility warnings where appropriate?




Comment at: lib/Parse/ParseExprCXX.cpp:1123
+  Diag(RAngleLoc,
+   diag::err_expected_lambda_template_parameter_list);
+}

I think it might be more user friendly if you used a different error message 
for the <> case here - along the lines of: empty template parameter list is not 
allowed 


https://reviews.llvm.org/D36527



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

[PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas

[Faisal Vali via Phabricator via cfe-commits]

faisalv added inline comments.



Comment at: lib/Parse/ParseExprCXX.cpp:1112
 
+  ParseScope TemplateParamScope(this, Scope::TemplateParamScope);
+  if (getLangOpts().CPlusPlus2a && Tok.is(tok::less)) {

hamzasood wrote:
> faisalv wrote:
> > We always create a template parameter scope here - but it's only the right 
> > scope if we have explicit template parameters, correct?
> > 
> > What I think we should do here is :
> >   - factor out (preferably as a separate patch) the post 
> > explicit-template-parameter-list parsing into a separate function (F1) 
> > which is then called from here.
> >   - then in this patch factor out the explicit template-parameter-list 
> > parsing also into a separate function that then either calls the function 
> > above ('F1'), or sequenced these statements before a call to 'F1'
> >   - also since gcc has had explicit template parameters on their generic 
> > lambdas for a while, can we find out under what options they have it 
> > enabled, and consider enabling it under those options for our gcc emulation 
> > mode? (or add a fixme for it?)
> >   - should we enable these explicit template parameters for pre-C++2a modes 
> > and emit extension/compatibility warnings where appropriate?
> > 
> Good point. It isn’t the “wrong” scope in the sense that is breaks anything, 
> but it is a bit wasteful to unconditionally push a potentially unneeded scope.
> 
> I have another idea that could be less intrusive, which is to replace this 
> line and the start of the if statement with:
> 
>   bool HasExplicitTemplateParams = getLangOpts().CPlusPlus2a && 
> Tok.is(tok::less);
>   ParseScope TemplateParamScope(this, Scope::TemplateParamScope, 
> HasExplicitTemplateParams);
>   if (HasExplicitTemplateParams) {
> // same code as before
>   }
> 
> That way the scope is only entered when needed, but no restructuring is 
> required (which I personally think would make things a bit harder to follow). 
> Could that work?
good idea - i think that should work too.  (Although i do still like the idea 
of refactoring this long function via extract-method - but i can always do that 
refactoring post this patch). 


https://reviews.llvm.org/D36527



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

[PATCH] D36572: Implemented P0409R2 - Allow lambda capture [=, this]

[Faisal Vali via Phabricator via cfe-commits]

faisalv closed this revision.
faisalv added a comment.

Committed as r311224, on behalf of Hamza.

https://reviews.llvm.org/rL311224


https://reviews.llvm.org/D36572



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

[PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas

[Faisal Vali via Phabricator via cfe-commits]

faisalv added inline comments.



Comment at: include/clang/Sema/ScopeInfo.h:774
+  /// \brief The number of parameters in the template parameter list that were
+  /// explicitely specified by the user, as opposed to being invented by use
+  /// of an auto parameter.

typo: explicitly



Comment at: lib/Parse/ParseExprCXX.cpp:1090
+  TemplateParameterDepthRAII CurTemplateDepthTracker(TemplateParameterDepth);
+  Actions.RecordParsingTemplateParameterDepth(TemplateParameterDepth);
+

Since you only really need to pass this information on for computing the depth 
of the 'auto' parameters - why not just leave the 
RecordParsingTEmplateParameterDepth call where it was, increment the template 
depth once we parse the TPL, and just pass in the right depth (minus one if 
ExplicitTemplateParameters) and increment the tracker if we getGenericLambda 
but no explicit TPL?

I wonder if that might be the safer way to do it - especially if you have 
generic lambdas in default arguments of generic lambdas - each of which have 
explicit template parameters also??

thoughts?




Comment at: lib/Parse/ParseExprCXX.cpp:1113
+  // FIXME: Consider allowing this as an extension for GCC compatibiblity.
+  bool HasExplicitTemplateParams = getLangOpts().CPlusPlus2a
+   && Tok.is(tok::less);

make this const pls.



Comment at: lib/Parse/ParseExprCXX.cpp:1305
 
+  TemplateParamScope.Exit();
+

Why do you exit the scope here, and then re-add the template parameters to the 
current scope?  What confusion (if any) occurs if you leave this scope on?



https://reviews.llvm.org/D36527



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

[PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas

[Faisal Vali via Phabricator via cfe-commits]

faisalv added a comment.

In regards to that failing test (that was added since review began) - could you 
fix that test pls (i.e. rename the nested ttp 'U' to something else) and move 
it into the function 'f' as requested by the author?
Might want to include a similar (but not same) example of matching complexity 
to your tests too.

Thanks!


https://reviews.llvm.org/D36527



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

[PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas

[Faisal Vali via Phabricator via cfe-commits]

faisalv added inline comments.



Comment at: lib/Parse/ParseExprCXX.cpp:1090
+  TemplateParameterDepthRAII CurTemplateDepthTracker(TemplateParameterDepth);
+  Actions.RecordParsingTemplateParameterDepth(TemplateParameterDepth);
+

hamzasood wrote:
> faisalv wrote:
> > Since you only really need to pass this information on for computing the 
> > depth of the 'auto' parameters - why not just leave the 
> > RecordParsingTEmplateParameterDepth call where it was, increment the 
> > template depth once we parse the TPL, and just pass in the right depth 
> > (minus one if ExplicitTemplateParameters) and increment the tracker if we 
> > getGenericLambda but no explicit TPL?
> > 
> > I wonder if that might be the safer way to do it - especially if you have 
> > generic lambdas in default arguments of generic lambdas - each of which 
> > have explicit template parameters also??
> > 
> > thoughts?
> > 
> While the depth is currently only used for auto parameters, that could change 
> in the future (especially since the function name is quite general. I.e. 
> record depth instead of record auto depth or whatever). Because of that, it 
> wouldn't seem right to not record it in the case where there's an explicit 
> template param list but no function parameters. So the options were to record 
> it twice (parsing the explicit TPL and function parameters) or just do it 
> once for every lambda. Since it's a cheap operation (pretty much just setting 
> a variable), I went for the latter.
> 
> Good point about incrementing after seeing an explicit template param list. 
> I'll change that in the next update.
I think If the depth will be needed in the future for something other than 
determining the depth of the auto parameter - then that change (of moving the 
call outside) should occur in that 'future' patch.  I don't see why you say 
that it would ever need to be called twice in your patch?

Just like we localize our variable declarations and limit their scope (to ease 
comprehension) similarly, i think changes to state should be as local to the 
part of the program that uses that state (once again for comprehension:  If you 
move the call, folks now might have to put in more effort when trying to 
comprehend the effects of that call, since the set of potential operations 
relying on that set grows).




Comment at: lib/Parse/ParseExprCXX.cpp:1305
 
+  TemplateParamScope.Exit();
+

hamzasood wrote:
> faisalv wrote:
> > Why do you exit the scope here, and then re-add the template parameters to 
> > the current scope?  What confusion (if any) occurs if you leave this scope 
> > on?
> > 
> That was my original attempt, but I just couldn't get it to work.
> 
> E.g.
> 
> ```
> auto l = [](T t) { };
> ```
> 
> triggers an assertion:
> 
> ```
> (!A->getDeducedType().isNull() && "cannot request the size of an undeduced or 
> dependent auto type"), function getTypeInfoImpl, file 
> /Users/hamzasood/LLVM/src/tools/clang/lib/AST/ASTContext.cpp, line 1883.
> ```
> 
> A lot of other things broke too.
> 
> I figured since function parameters are explicitly added to scope, it would 
> make sense to introduce explicit template parameters in the same place.
Hmm - I think at the very least we should understand exactly what assumptions 
are being violated - and then decide whether we wish to temporarily 
bypass/shortcut the way scopes are supposed to 'seamlessly' work (w a fixme or 
comment prior to the 'false exit' that explains why we are doing so) or adjust 
the assumptions...


https://reviews.llvm.org/D36527



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

[PATCH] D35782: [C++2a][Preprocessor] Implement p0306 __VA_OPT__ (Comma omission and comma deletion)

[Faisal Vali via Phabricator via cfe-commits]

faisalv added a comment.

*ping*


Repository:
  rL LLVM

https://reviews.llvm.org/D35782



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

[PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas

[Faisal Vali via Phabricator via cfe-commits]

faisalv added a comment.

In regards to representing this in the AST - I think (based on precedence) that 
the number of explicit template parameters should be stored in 
LambdaDefinitionData - and the interface exposed through LambdaExpr (where the 
source information of the template parameter list should be stored too i think 
- Richard you agree?).

Also can you add examples of such generic lambdas that are nested within either 
other generic lambdas or templates - and make sure that they 
instantiate/substitute correctly - and that we really don't have to touch the 
template instantiation machinery?

Thanks!




Comment at: include/clang/Sema/ScopeInfo.h:955
+  }
+
   // When passed the index, returns the VarDecl and Expr associated

This function doesn't seem to be called anywhere?



Comment at: lib/Parse/ParseExprCXX.cpp:1116
+  if (HasExplicitTemplateParams) {
+SmallVector TemplateParams;
+SourceLocation LAngleLoc, RAngleLoc;

Why not Just use/pass LSI->TemplateParams?



Comment at: lib/Sema/SemaLambda.cpp:501
+  LAngleLoc, TParams, RAngleLoc,
+  /*RequiresClause=*/nullptr);
+

I'm not sure i see the point in creating the template parameter list here and 
returning it, when it's not used anywhere?  The TPL is created and cached in 
GLTemplateParameterList when needed it seems.




Comment at: lib/Sema/SemaLambda.cpp:858
+KnownDependent = CurScope->getTemplateParamParent() != nullptr;
+  }
 

Hmm - now that you drew my attention to this ;) - I'm pretty sure this is 
broken - but (embarrassingly) it broke back when i implemented generic lambdas 
(and interestingly is less broken w generic lambdas w explicit TPLs) - could I 
ask you to add a FIXME here that states something along the lines of:  

When parsing default arguments that contain lambdas, it seems necessary to know 
whether the containing parameter-declaration clause is that of a template to 
mark the closure type created in the default-argument as dependent.  Using 
template params to detect dependence is not enough for all generic lambdas 
since you can have generic lambdas without explicit template parameters, and 
whose default arguments contain lambdas that should be dependent - and you can 
not rely on the existence of a template parameter scope to detect those cases.  
Consider:
   auto L = [](int I = [] { return 5; }(), auto a) { };  
The above nested closure type (of the default argument) occurs within a 
dependent context and is therefore dependent - but we won't know that until we 
parse the second parameter.  

p.s. I'll try and get around to fixing this if no one else does.



https://reviews.llvm.org/D36527



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

[PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas

[Faisal Vali via Phabricator via cfe-commits]

faisalv added inline comments.



Comment at: lib/Parse/ParseExprCXX.cpp:1116
+  if (HasExplicitTemplateParams) {
+SmallVector TemplateParams;
+SourceLocation LAngleLoc, RAngleLoc;

hamzasood wrote:
> faisalv wrote:
> > Why not Just use/pass LSI->TemplateParams?
> I thought that Parser and Sema stay separate, and communicate through various 
> ActOn functions? Directly accessing LSI would violate that.
Aah yes - you're right.  Still it does seem a little wasteful to create two of 
those (and then append).  What are your thoughts about passing the argument by 
(moved from) value, and then swapping their guts within ActOn (i..e 
value-semantics) ?  (I suppose the only concern would be the small array case - 
but i think that would be quite easy for any optimizer to inline).   



https://reviews.llvm.org/D36527



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

[PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas

[Faisal Vali via Phabricator via cfe-commits]

faisalv added inline comments.



Comment at: lib/Sema/SemaLambda.cpp:858
+KnownDependent = CurScope->getTemplateParamParent() != nullptr;
+  }
 

hamzasood wrote:
> faisalv wrote:
> > Hmm - now that you drew my attention to this ;) - I'm pretty sure this is 
> > broken - but (embarrassingly) it broke back when i implemented generic 
> > lambdas (and interestingly is less broken w generic lambdas w explicit 
> > TPLs) - could I ask you to add a FIXME here that states something along the 
> > lines of:  
> > 
> > When parsing default arguments that contain lambdas, it seems necessary to 
> > know whether the containing parameter-declaration clause is that of a 
> > template to mark the closure type created in the default-argument as 
> > dependent.  Using template params to detect dependence is not enough for 
> > all generic lambdas since you can have generic lambdas without explicit 
> > template parameters, and whose default arguments contain lambdas that 
> > should be dependent - and you can not rely on the existence of a template 
> > parameter scope to detect those cases.  Consider:
> >auto L = [](int I = [] { return 5; }(), auto a) { };  
> > The above nested closure type (of the default argument) occurs within a 
> > dependent context and is therefore dependent - but we won't know that until 
> > we parse the second parameter.  
> > 
> > p.s. I'll try and get around to fixing this if no one else does.
> > 
> Good point. Now you mention it, isn't it even more broken than than?
> E.g:
> 
> ```
>  auto L = [](auto A, int I = [] { return 5; }());
> ```
> 
> L is known to be generic before we parse the nested lambda, but template 
> param scopes aren't established for auto parameters (I think), so the nested 
> lambda won't get marked as dependent
I was aware of that - but I think that's the easier case to fix (where you see 
the auto first - hence i reversed it in my example).


https://reviews.llvm.org/D36527



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

[PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas

[Faisal Vali via Phabricator via cfe-commits]

faisalv added inline comments.



Comment at: lib/Parse/ParseExprCXX.cpp:1116
+  if (HasExplicitTemplateParams) {
+SmallVector TemplateParams;
+SourceLocation LAngleLoc, RAngleLoc;

hamzasood wrote:
> faisalv wrote:
> > hamzasood wrote:
> > > faisalv wrote:
> > > > Why not Just use/pass LSI->TemplateParams?
> > > I thought that Parser and Sema stay separate, and communicate through 
> > > various ActOn functions? Directly accessing LSI would violate that.
> > Aah yes - you're right.  Still it does seem a little wasteful to create two 
> > of those (and then append).  What are your thoughts about passing the 
> > argument by (moved from) value, and then swapping their guts within ActOn 
> > (i..e value-semantics) ?  (I suppose the only concern would be the small 
> > array case - but i think that would be quite easy for any optimizer to 
> > inline).   
> > 
> I don't think a SmallVectorImpl can be passed by value.
> 
> So to make that work, the function would either needed to be templated 
> (SmallVector) or only accept a SmallVector. And I don't 
> think either of those options are worthwhile.
OK - add a FIXME that alerts folks that we currently make two copies of this 
and ideally we shouldn't need to.


https://reviews.llvm.org/D36527



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

[PATCH] D36915: [Sema] Diagnose local variables and parameters captured by lambda and block expressions in a default argument

[Faisal Vali via Phabricator via cfe-commits]

faisalv added a comment.

I don't think this approach is entirely correct for at least the following 
reasons:

1. in the lambda case the machinery that diagnoses capture failures should be 
the machinery erroring on the lambda (when the parameter is odr-used)
2. in the unevaluated case, once you disable the error, the instantiation 
machinery will fail to find the corresponding instantiated parmvardecl.

I think - in addition to allowing unevaluated uses of parmvardecls by tweaking 
the DefaultArgumentChecker, you would also need to add the instantiated 
mappings of the parameters, prior to instantiating the default argument (to 
avoid the assertion) and perhaps need to tweak DoMarkVarDeclReferenced and/or 
tryCaptureVariable to make sure such cases for lambdas produce errors (if they 
don't, w the above fix).

Thanks for working on this!


https://reviews.llvm.org/D36915



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

[PATCH] D92733: Fix PR25627 - false positive diagnostics involving implicit-captures in dependent lambda expressions.

[Faisal Vali via Phabricator via cfe-commits]

faisalv added a comment.

*ping*


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92733

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

[PATCH] D91011: [NFC, Refactor] Rename the (scoped) enum DeclaratorContext enumerator's to avoid redundancy

[Faisal Vali via Phabricator via cfe-commits]

faisalv created this revision.
faisalv added reviewers: aaron.ballman, bruno.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
faisalv requested review of this revision.

Since these are scoped enumerators, they have to be prefixed by 
DeclaratorContext, so lets remove Context from the name, and return some 
characters to the multiverse.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D91011

Files:
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/DeclSpec.h
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Parse/ParseExprCXX.cpp
  clang/lib/Parse/ParseObjc.cpp
  clang/lib/Parse/ParseOpenMP.cpp
  clang/lib/Parse/ParseStmt.cpp
  clang/lib/Parse/ParseTemplate.cpp
  clang/lib/Parse/Parser.cpp
  clang/lib/Sema/DeclSpec.cpp
  clang/lib/Sema/SemaCodeComplete.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaDeclObjC.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaType.cpp

Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -50,7 +50,7 @@
 /// isOmittedBlockReturnType - Return true if this declarator is missing a
 /// return type because this is a omitted return type on a block literal.
 static bool isOmittedBlockReturnType(const Declarator &D) {
-  if (D.getContext() != DeclaratorContext::BlockLiteralContext ||
+  if (D.getContext() != DeclaratorContext::BlockLiteral ||
   D.getDeclSpec().hasTypeSpecifier())
 return false;
 
@@ -1348,12 +1348,12 @@
 // The declspec is always missing in a lambda expr context; it is either
 // specified with a trailing return type or inferred.
 if (S.getLangOpts().CPlusPlus14 &&
-declarator.getContext() == DeclaratorContext::LambdaExprContext) {
+declarator.getContext() == DeclaratorContext::LambdaExpr) {
   // In C++1y, a lambda's implicit return type is 'auto'.
   Result = Context.getAutoDeductType();
   break;
 } else if (declarator.getContext() ==
-   DeclaratorContext::LambdaExprContext ||
+   DeclaratorContext::LambdaExpr ||
checkOmittedBlockReturnType(S, declarator,
Context.DependentTy)) {
   Result = Context.DependentTy;
@@ -1741,7 +1741,7 @@
 
   // Before we process any type attributes, synthesize a block literal
   // function declarator if necessary.
-  if (declarator.getContext() == DeclaratorContext::BlockLiteralContext)
+  if (declarator.getContext() == DeclaratorContext::BlockLiteral)
 maybeSynthesizeBlockSignature(state, Result);
 
   // Apply any type attributes from the decl spec.  This may cause the
@@ -3306,21 +3306,21 @@
 bool IsDeducedReturnType = false;
 
 switch (D.getContext()) {
-case DeclaratorContext::LambdaExprContext:
+case DeclaratorContext::LambdaExpr:
   // Declared return type of a lambda-declarator is implicit and is always
   // 'auto'.
   break;
-case DeclaratorContext::ObjCParameterContext:
-case DeclaratorContext::ObjCResultContext:
+case DeclaratorContext::ObjCParameter:
+case DeclaratorContext::ObjCResult:
   Error = 0;
   break;
-case DeclaratorContext::RequiresExprContext:
+case DeclaratorContext::RequiresExpr:
   Error = 22;
   break;
-case DeclaratorContext::PrototypeContext:
-case DeclaratorContext::LambdaExprParameterContext: {
+case DeclaratorContext::Prototype:
+case DeclaratorContext::LambdaExprParameter: {
   InventedTemplateParameterInfo *Info = nullptr;
-  if (D.getContext() == DeclaratorContext::PrototypeContext) {
+  if (D.getContext() == DeclaratorContext::Prototype) {
 // With concepts we allow 'auto' in function parameters.
 if (!SemaRef.getLangOpts().CPlusPlus20 || !Auto ||
 Auto->getKeyword() != AutoTypeKeyword::Auto) {
@@ -3350,7 +3350,7 @@
 T = InventTemplateParameter(state, T, nullptr, Auto, *Info).first;
   break;
 }
-case DeclaratorContext::MemberContext: {
+case DeclaratorContext::Member: {
   if (D.getDeclSpec().getStorageClassSpec() == DeclSpec::SCS_static ||
   D.isFunctionDeclarator())
 break;
@@ -3370,21 +3370,21 @@
 Error = 20; // Friend type
   break;
 }
-case DeclaratorContext::CXXCatchContext:
-case DeclaratorContext::ObjCCatchContext:
+case DeclaratorContext::CXXCatch:
+case DeclaratorContext::ObjCCatch:
   Error = 7; // Exception declaration
   break;
-case DeclaratorContext::TemplateParamContext:
+case DeclaratorContext::TemplateParam:
   if (isa(Deduced) &&
   !SemaRef.getLangOpts().CPlusPlus20)
 Error = 19; // Template parameter (until C++20)
   else if (!SemaRef.getLangOpts().CPlusPlus17)
 Error = 8; // Template parameter (until C++1

[PATCH] D91035: [NFC, Refactor] Convert FunctionDefinitionKind from DeclSpech.h to a scoped enum

[Faisal Vali via Phabricator via cfe-commits]

faisalv created this revision.
faisalv added reviewers: aaron.ballman, bruno, BRevzin, wchilders.
faisalv added a project: clang.
faisalv requested review of this revision.

[NFC, Refactor] Convert FunctionDefinitionKind from DeclSpech.h to a scoped enum


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D91035

Files:
  clang/include/clang/Sema/DeclSpec.h
  clang/lib/Parse/ParseCXXInlineMethods.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Parse/Parser.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Sema/SemaType.cpp

Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -3579,7 +3579,7 @@
   // Only warn if this declarator is declaring a function at block scope, and
   // doesn't have a storage class (such as 'extern') specified.
   if (!D.isFunctionDeclarator() ||
-  D.getFunctionDefinitionKind() != FDK_Declaration ||
+  D.getFunctionDefinitionKind() != FDK::Declaration ||
   !S.CurContext->isFunctionOrMethod() ||
   D.getDeclSpec().getStorageClassSpec()
 != DeclSpec::SCS_unspecified)
@@ -5039,7 +5039,7 @@
   !(S.getLangOpts().CPlusPlus &&
 (T->isDependentType() || T->isRecordType( {
 if (T->isVoidType() && !S.getLangOpts().CPlusPlus &&
-D.getFunctionDefinitionKind() == FDK_Definition) {
+D.getFunctionDefinitionKind() == FDK::Definition) {
   // [6.9.1/3] qualified void return is invalid on a C
   // function definition.  Apparently ok on declarations and
   // in C++ though (!)
@@ -5410,7 +5410,8 @@
   //   The empty list in a function declarator that is not part of a definition
   //   of that function specifies that no information about the number or types
   //   of the parameters is supplied.
-  if (!LangOpts.CPlusPlus && D.getFunctionDefinitionKind() == FDK_Declaration) {
+  if (!LangOpts.CPlusPlus &&
+  D.getFunctionDefinitionKind() == FDK::Declaration) {
 bool IsBlock = false;
 for (const DeclaratorChunk &DeclType : D.type_objects()) {
   switch (DeclType.Kind) {
Index: clang/lib/Sema/SemaOpenMP.cpp
===
--- clang/lib/Sema/SemaOpenMP.cpp
+++ clang/lib/Sema/SemaOpenMP.cpp
@@ -5939,7 +5939,7 @@
   llvm::omp::TraitProperty::implementation_extension_disable_implicit_base);
   // If no base was found we create a declaration that we use as base.
   if (Bases.empty() && UseImplicitBase) {
-D.setFunctionDefinitionKind(FDK_Declaration);
+D.setFunctionDefinitionKind(FDK::Declaration);
 Decl *BaseD = HandleDeclarator(S, D, TemplateParamLists);
 BaseD->setImplicit(true);
 if (auto *BaseTemplD = dyn_cast(BaseD))
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -5543,7 +5543,7 @@
 }
 
 Decl *Sema::ActOnDeclarator(Scope *S, Declarator &D) {
-  D.setFunctionDefinitionKind(FDK_Declaration);
+  D.setFunctionDefinitionKind(FDK::Declaration);
   Decl *Dcl = HandleDeclarator(S, D, MultiTemplateParamsArg());
 
   if (OriginalLexicalContext && OriginalLexicalContext->isObjCContainer() &&
@@ -9160,15 +9160,15 @@
 // If a function is defined as defaulted or deleted, mark it as such now.
 // We'll do the relevant checks on defaulted / deleted functions later.
 switch (D.getFunctionDefinitionKind()) {
-  case FDK_Declaration:
-  case FDK_Definition:
+  case FDK::Declaration:
+  case FDK::Definition:
 break;
 
-  case FDK_Defaulted:
+  case FDK::Defaulted:
 NewFD->setDefaulted();
 break;
 
-  case FDK_Deleted:
+  case FDK::Deleted:
 NewFD->setDeletedAsWritten();
 break;
 }
@@ -9871,17 +9871,17 @@
   // because Sema::ActOnStartOfFunctionDef has not been called yet.
   if (const auto *NBA = NewFD->getAttr())
 switch (D.getFunctionDefinitionKind()) {
-case FDK_Defaulted:
-case FDK_Deleted:
+case FDK::Defaulted:
+case FDK::Deleted:
   Diag(NBA->getLocation(),
diag::err_attribute_no_builtin_on_defaulted_deleted_function)
   << NBA->getSpelling();
   break;
-case FDK_Declaration:
+case FDK::Declaration:
   Diag(NBA->getLocation(), diag::err_attribute_no_builtin_on_non_definition)
   << NBA->getSpelling();
   break;
-case FDK_Definition:
+case FDK::Definition:
   break;
 }
 
@@ -13786,7 +13786,7 @@
 ActOnStartOfFunctionDefinitionInOpenMPDeclareVariantScope(
 ParentScope, D, TemplateParameterLists, Bases);
 
-  D.setFunctionDefinitionKind(FDK_Definition);
+  D.setFunctionDefinitionKind(FDK::Definition);
   Decl *DP = HandleDeclarator(ParentScope, D, TemplateParameterLists);
   Decl *Dcl = ActOnStartOfFunctionDef(FnBodyScope, DP, S

[PATCH] D91035: [NFC, Refactor] Convert FunctionDefinitionKind from DeclSpech.h to a scoped enum

[Faisal Vali via Phabricator via cfe-commits]

faisalv added a comment.

In D91035#2383167 , @wchilders wrote:

> Generally agree with this direction; Are there plans for migrating 
> https://github.com/llvm/llvm-project/blob/master/clang/include/clang/Basic/Specifiers.h
>  in a similar fashion, for consistency?

yup - i plan to get around to many of these - assuming time cooperates.

Am currently working on Decl::Kind...




Comment at: clang/include/clang/Sema/DeclSpec.h:1837
   /// Actually a FunctionDefinitionKind.
-  unsigned FunctionDefinition : 2;
+  FunctionDefinitionKind FunctionDefinition : 2;
 

aaron.ballman wrote:
> I think we need to keep this as `unsigned` because some compilers struggle 
> with bit-fields of enumeration types (even when the enumeration underlying 
> type is fixed): https://godbolt.org/z/P8x8Kz
As Barry had reminded me - this warning was deemed a bug: 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=51242.  Are you sure we should 
still tailor our code to appease it? Is there a config file we can use to 
#define an ENUM_UNSIGNED_BITFIELD(x) or some such - that does the right thing 
for most compilers - (and are we even comfortable from a style-guide 
perpective, with such a conditional-define strategy?

Your thoughts?

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91035

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

[PATCH] D91011: [NFC, Refactor] Rename the (scoped) enum DeclaratorContext's enumerators to avoid redundancy

[Faisal Vali via Phabricator via cfe-commits]

faisalv closed this revision.
faisalv added a comment.

Committed: 
https://github.com/llvm/llvm-project/commit/e4d27932a59fb61aaba3ff7a3ccd1b5bc9215fb9


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91011

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

[PATCH] D91035: [NFC, Refactor] Convert FunctionDefinitionKind from DeclSpech.h to a scoped enum

[Faisal Vali via Phabricator via cfe-commits]

faisalv added inline comments.



Comment at: clang/include/clang/Sema/DeclSpec.h:1837
   /// Actually a FunctionDefinitionKind.
-  unsigned FunctionDefinition : 2;
+  FunctionDefinitionKind FunctionDefinition : 2;
 

aaron.ballman wrote:
> faisalv wrote:
> > aaron.ballman wrote:
> > > I think we need to keep this as `unsigned` because some compilers 
> > > struggle with bit-fields of enumeration types (even when the enumeration 
> > > underlying type is fixed): https://godbolt.org/z/P8x8Kz
> > As Barry had reminded me - this warning was deemed a bug: 
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=51242.  Are you sure we should 
> > still tailor our code to appease it? Is there a config file we can use to 
> > #define an ENUM_UNSIGNED_BITFIELD(x) or some such - that does the right 
> > thing for most compilers - (and are we even comfortable from a style-guide 
> > perpective, with such a conditional-define strategy?
> > 
> > Your thoughts?
> > 
> > Thanks!
> The warning in GCC was a bug, but the fact that GCC issues the warning means 
> `-Werror` builds will not be able to handle it. GCC 9.2 is really recent, so 
> we can't just bump the supported version of GCC to 9.3 to avoid the issue. We 
> could use macros to work around it for GCC, but IIRC, MSVC also had some 
> hiccups over the years with using an enumeration as a bit-field member (I 
> seem to recall it not wanting to pack the bits with surrounding fields, but I 
> could be remembering incorrectly). I'm not certain whether macros are worth 
> the effort, but my personal inclination is to just stick with `unsigned` 
> unless there's a really big win from coming up with something more complex.
Well - the biggest downside of making it unsigned (vs leaving it as an enum) is 
that each assignment or initialization now requires a static_cast.  

What would you folks suggest:
1) do not modernize such enums into scoped enums
2) scope these enums - sticking to unsigned bit-fields - and add static_casts
3) teach the bots to ignore that gcc warning? (is this even an option)

Thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91035

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

[PATCH] D91035: [NFC, Refactor] Convert FunctionDefinitionKind from DeclSpech.h to a scoped enum

[Faisal Vali via Phabricator via cfe-commits]

faisalv updated this revision to Diff 305020.
faisalv added a comment.

This revision includes the following changes to the initial patch:

- revert the bit-field to unsigned from enum (so as to avoid that nettlesome 
gcc warning)
- specified a fixed underlying type of 'unsigned char' for the enum 
FunctionDefinitionKind
- added static_casts when initiatilizing or assigning to the bit-field (which 
as Aaron astutely noticed was confined to the ctor and setter)

Thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91035

Files:
  clang/include/clang/Sema/DeclSpec.h
  clang/lib/Parse/ParseCXXInlineMethods.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Parse/Parser.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Sema/SemaType.cpp

Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -3577,7 +3577,7 @@
   // Only warn if this declarator is declaring a function at block scope, and
   // doesn't have a storage class (such as 'extern') specified.
   if (!D.isFunctionDeclarator() ||
-  D.getFunctionDefinitionKind() != FDK_Declaration ||
+  D.getFunctionDefinitionKind() != FDK::Declaration ||
   !S.CurContext->isFunctionOrMethod() ||
   D.getDeclSpec().getStorageClassSpec()
 != DeclSpec::SCS_unspecified)
@@ -5034,7 +5034,7 @@
   !(S.getLangOpts().CPlusPlus &&
 (T->isDependentType() || T->isRecordType( {
 if (T->isVoidType() && !S.getLangOpts().CPlusPlus &&
-D.getFunctionDefinitionKind() == FDK_Definition) {
+D.getFunctionDefinitionKind() == FDK::Definition) {
   // [6.9.1/3] qualified void return is invalid on a C
   // function definition.  Apparently ok on declarations and
   // in C++ though (!)
@@ -5405,7 +5405,8 @@
   //   The empty list in a function declarator that is not part of a definition
   //   of that function specifies that no information about the number or types
   //   of the parameters is supplied.
-  if (!LangOpts.CPlusPlus && D.getFunctionDefinitionKind() == FDK_Declaration) {
+  if (!LangOpts.CPlusPlus &&
+  D.getFunctionDefinitionKind() == FDK::Declaration) {
 bool IsBlock = false;
 for (const DeclaratorChunk &DeclType : D.type_objects()) {
   switch (DeclType.Kind) {
Index: clang/lib/Sema/SemaOpenMP.cpp
===
--- clang/lib/Sema/SemaOpenMP.cpp
+++ clang/lib/Sema/SemaOpenMP.cpp
@@ -5939,7 +5939,7 @@
   llvm::omp::TraitProperty::implementation_extension_disable_implicit_base);
   // If no base was found we create a declaration that we use as base.
   if (Bases.empty() && UseImplicitBase) {
-D.setFunctionDefinitionKind(FDK_Declaration);
+D.setFunctionDefinitionKind(FDK::Declaration);
 Decl *BaseD = HandleDeclarator(S, D, TemplateParamLists);
 BaseD->setImplicit(true);
 if (auto *BaseTemplD = dyn_cast(BaseD))
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -5543,7 +5543,7 @@
 }
 
 Decl *Sema::ActOnDeclarator(Scope *S, Declarator &D) {
-  D.setFunctionDefinitionKind(FDK_Declaration);
+  D.setFunctionDefinitionKind(FDK::Declaration);
   Decl *Dcl = HandleDeclarator(S, D, MultiTemplateParamsArg());
 
   if (OriginalLexicalContext && OriginalLexicalContext->isObjCContainer() &&
@@ -9160,15 +9160,15 @@
 // If a function is defined as defaulted or deleted, mark it as such now.
 // We'll do the relevant checks on defaulted / deleted functions later.
 switch (D.getFunctionDefinitionKind()) {
-  case FDK_Declaration:
-  case FDK_Definition:
+  case FDK::Declaration:
+  case FDK::Definition:
 break;
 
-  case FDK_Defaulted:
+  case FDK::Defaulted:
 NewFD->setDefaulted();
 break;
 
-  case FDK_Deleted:
+  case FDK::Deleted:
 NewFD->setDeletedAsWritten();
 break;
 }
@@ -9871,17 +9871,17 @@
   // because Sema::ActOnStartOfFunctionDef has not been called yet.
   if (const auto *NBA = NewFD->getAttr())
 switch (D.getFunctionDefinitionKind()) {
-case FDK_Defaulted:
-case FDK_Deleted:
+case FDK::Defaulted:
+case FDK::Deleted:
   Diag(NBA->getLocation(),
diag::err_attribute_no_builtin_on_defaulted_deleted_function)
   << NBA->getSpelling();
   break;
-case FDK_Declaration:
+case FDK::Declaration:
   Diag(NBA->getLocation(), diag::err_attribute_no_builtin_on_non_definition)
   << NBA->getSpelling();
   break;
-case FDK_Definition:
+case FDK::Definition:
   break;
 }
 
@@ -13786,7 +13786,7 @@
 ActOnStartOfFunctionDefinitionInOpenMPDeclareVariantScop

[PATCH] D91035: [NFC, Refactor] Convert FunctionDefinitionKind from DeclSpech.h to a scoped enum

[Faisal Vali via Phabricator via cfe-commits]

faisalv updated this revision to Diff 305317.
faisalv added a comment.

This diff makes the following changes to the previous patch (based on feedback 
from Richard, Aaron and Wyatt):

- avoid introducing an initialism (FDK) into the clang namespace and 
unabbreviated each corresponding use to 'FunctionDefinitionKind'.  Let me know 
if it seems too verbose -  if so, perhaps a compromise along Wyatt's suggestion 
might behoove our source.
- changed the destination type from 'unsigned' to 'unsigned char' in our 
static_casts.
  - is that preferred, or should i have left it as 'unsigned'?
  - is there any real benefit here to specifying an underlying type of 
'unsigned char' for our enum (that is never used as an opaque enum).

Richard, thanks for stepping in and enlightening us as to why the world is 
still not ready for type-safe enum bit-fields! (spoiler: MSVC's outré  ABI 
choice in layout)

P.S. Also, for those of you like me, who tend to be sloppy with their English 
(unlike Richard in all his meticulous glory ;) - and were unfamiliar with the 
nuances behind the term 'initialism' - let me direct you to a lesson from one 
of the greatest broadcasters of our time: https://youtu.be/FyJsvT3Eo4c


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91035

Files:
  clang/include/clang/Sema/DeclSpec.h
  clang/lib/Parse/ParseCXXInlineMethods.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Parse/Parser.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Sema/SemaType.cpp

Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -3577,10 +3577,9 @@
   // Only warn if this declarator is declaring a function at block scope, and
   // doesn't have a storage class (such as 'extern') specified.
   if (!D.isFunctionDeclarator() ||
-  D.getFunctionDefinitionKind() != FDK_Declaration ||
+  D.getFunctionDefinitionKind() != FunctionDefinitionKind::Declaration ||
   !S.CurContext->isFunctionOrMethod() ||
-  D.getDeclSpec().getStorageClassSpec()
-!= DeclSpec::SCS_unspecified)
+  D.getDeclSpec().getStorageClassSpec() != DeclSpec::SCS_unspecified)
 return;
 
   // Inside a condition, a direct initializer is not permitted. We allow one to
@@ -5034,7 +5033,8 @@
   !(S.getLangOpts().CPlusPlus &&
 (T->isDependentType() || T->isRecordType( {
 if (T->isVoidType() && !S.getLangOpts().CPlusPlus &&
-D.getFunctionDefinitionKind() == FDK_Definition) {
+D.getFunctionDefinitionKind() ==
+FunctionDefinitionKind::Definition) {
   // [6.9.1/3] qualified void return is invalid on a C
   // function definition.  Apparently ok on declarations and
   // in C++ though (!)
@@ -5405,7 +5405,8 @@
   //   The empty list in a function declarator that is not part of a definition
   //   of that function specifies that no information about the number or types
   //   of the parameters is supplied.
-  if (!LangOpts.CPlusPlus && D.getFunctionDefinitionKind() == FDK_Declaration) {
+  if (!LangOpts.CPlusPlus &&
+  D.getFunctionDefinitionKind() == FunctionDefinitionKind::Declaration) {
 bool IsBlock = false;
 for (const DeclaratorChunk &DeclType : D.type_objects()) {
   switch (DeclType.Kind) {
Index: clang/lib/Sema/SemaOpenMP.cpp
===
--- clang/lib/Sema/SemaOpenMP.cpp
+++ clang/lib/Sema/SemaOpenMP.cpp
@@ -5939,7 +5939,7 @@
   llvm::omp::TraitProperty::implementation_extension_disable_implicit_base);
   // If no base was found we create a declaration that we use as base.
   if (Bases.empty() && UseImplicitBase) {
-D.setFunctionDefinitionKind(FDK_Declaration);
+D.setFunctionDefinitionKind(FunctionDefinitionKind::Declaration);
 Decl *BaseD = HandleDeclarator(S, D, TemplateParamLists);
 BaseD->setImplicit(true);
 if (auto *BaseTemplD = dyn_cast(BaseD))
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -5543,7 +5543,7 @@
 }
 
 Decl *Sema::ActOnDeclarator(Scope *S, Declarator &D) {
-  D.setFunctionDefinitionKind(FDK_Declaration);
+  D.setFunctionDefinitionKind(FunctionDefinitionKind::Declaration);
   Decl *Dcl = HandleDeclarator(S, D, MultiTemplateParamsArg());
 
   if (OriginalLexicalContext && OriginalLexicalContext->isObjCContainer() &&
@@ -9160,15 +9160,15 @@
 // If a function is defined as defaulted or deleted, mark it as such now.
 // We'll do the relevant checks on defaulted / deleted functions later.
 switch (D.getFunctionDefinitionKind()) {
-  case FDK_Declaration:
-  case FDK_Definition:
+  case FunctionDefinitionKind::Declaration:
+

[PATCH] D91651: [clang] Add a warning (à la gcc) for too small enum bitfields

[Faisal Vali via Phabricator via cfe-commits]

faisalv created this revision.
faisalv added reviewers: aaron.ballman, wchilders, bruno, rnk, BRevzin, thakis.
faisalv added a project: clang.
faisalv requested review of this revision.

Currently clang warns on 'assigning' to an enum bit-field that can not 
accommodate all its enumerators - but not when such a bit-field is defined.

GCC warns at definition time (https://godbolt.org/z/sKescn) - which seems like 
a useful thing to do, given certain programmer's propensities for memcpying.

This patch warns when such enum bit-fields are defined - and warns on unnamed 
bit-fields too (is that the right thing to do here?) building on work done by 
@rnk here 
https://github.com/llvm/llvm-project/commit/329f24d6f6e733fcadfd1be7cd3b430d63047c2e

Implementation Strategy:

- add the check, modeled after Reid's check in his patch, into VerifyBitField 
(checks if the width expression is an ICE and it's a worthy type etc.) that 
gets called from CheckFieldDecl

Questions for reviewers:

- Should we be emitting such a warning for unnamed bitfields?
- When comparing an APSInt to an unsigned value - should i prefer using the 
overloaded operator (i.e. Value < BitsNeeded) or stick with BitsNeeded > 
Value.getZExtValue().

Thank you!


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D91651

Files:
  clang/lib/Sema/SemaDecl.cpp
  clang/test/SemaCXX/warn-bitfield-enum-conversion.cpp


Index: clang/test/SemaCXX/warn-bitfield-enum-conversion.cpp
===
--- clang/test/SemaCXX/warn-bitfield-enum-conversion.cpp
+++ clang/test/SemaCXX/warn-bitfield-enum-conversion.cpp
@@ -1,3 +1,4 @@
+// RUN: %clang_cc1 -std=c++11 -triple x86_64-windows-msvc -verify %s 
-Wbitfield-enum-conversion -DFV_UNNAMED_BITFIELD
 // RUN: %clang_cc1 -std=c++11 -triple x86_64-windows-msvc -verify %s 
-Wbitfield-enum-conversion
 // RUN: %clang_cc1 -std=c++11 -triple x86_64-linux -verify %s 
-Wbitfield-enum-conversion
 
@@ -57,3 +58,29 @@
   f.two_bits = (unsigned)three_bits_signed;
   f.two_bits = static_cast(three_bits_signed);
 }
+
+#ifdef FV_UNNAMED_BITFIELD
+#define NAME(x)
+//expected-warning@#1 {{bit-field  is not wide enough}} 
+//expected-warning@#2 {{bit-field  is not wide enough}} 
+//expected-note@#1 {{widen this field to 63}}
+//expected-note@#2 {{widen this field to 64}}
+#else
+#define NAME(x) x
+//expected-warning@#1 {{bit-field 'b1' is not wide enough}} 
+//expected-warning@#2 {{bit-field 'b2' is not wide enough}} 
+//expected-note@#1 {{widen this field to 63}}
+//expected-note@#2 {{widen this field to 64}}
+#endif
+
+
+enum class E_UNSIGNED_LONG_LONG : long long { PLUS_MAX = 9223372036854775807 };
+enum class E_SIGNED_LONG_LONG : long long { PLUS_MAX = 9223372036854775807, 
MINUS = -1 };
+
+struct FV {
+  E_UNSIGNED_LONG_LONG NAME(b1) : 5; //#1
+  E_SIGNED_LONG_LONG NAME(b2) : 5;   //#2
+  E_UNSIGNED_LONG_LONG NAME(b3) : (sizeof(long long) * 8) - 1;
+  E_SIGNED_LONG_LONG NAME(b4) : (sizeof(long long) * 8);
+
+};
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -16430,6 +16430,38 @@
   }
 
   if (!FieldTy->isDependentType()) {
+// Check if an enum bit-field has enough bits to accomodate all its
+// enumerators. Skip the check for an unnamed 0 bit bit-field (alignment
+// signifier).
+if (FieldTy->isEnumeralType() && Value != 0) {
+  EnumDecl *ED = FieldTy->castAs()->getDecl();
+
+  const bool IsSignedEnum = ED->getNumNegativeBits() > 0;
+  // Compute the required bitwidth. If the enum has negative values, we 
need
+  // one more bit than the normal number of positive bits to represent the
+  // sign bit.
+  const unsigned BitsNeeded =
+  IsSignedEnum
+  ? std::max(ED->getNumPositiveBits() + 1, 
ED->getNumNegativeBits())
+  : ED->getNumPositiveBits();
+  
+  if (BitsNeeded > Value.getZExtValue()) {
+// TODO: Should we be emitting diagnostics for unnamed bitfields that
+// can never be assigned to?  Maybe, since they can be memcopied into?
+if (FieldName) {
+  Diag(FieldLoc, diag::warn_bitfield_too_small_for_enum)
+  << FieldName << ED;
+} else {
+  Diag(FieldLoc, diag::warn_bitfield_too_small_for_enum)
+  << "" << ED;
+  //<< FieldName << ED;
+  //
+}
+Diag(BitWidth->getExprLoc(), diag::note_widen_bitfield)
+<< BitsNeeded << ED << BitWidth->getSourceRange();
+  }
+}
+
 uint64_t TypeStorageSize = Context.getTypeSize(FieldTy);
 uint64_t TypeWidth = Context.getIntWidth(FieldTy);
 bool BitfieldIsOverwide = Value.ugt(TypeWidth);


Index: clang/test/SemaCXX/warn-bitfield-enum-conversion.cpp
===
--- clang/test/SemaCXX/warn-bitfield-enum-conversion.cpp
+++ clang/test/SemaCXX/wa

[PATCH] D91651: [clang] Add a warning (à la gcc) for too small enum bitfields

[Faisal Vali via Phabricator via cfe-commits]

faisalv updated this revision to Diff 305970.
faisalv added a comment.

Based on Richards Feedback, this update includes the following changes:

- avoids calling the fragile getZextValue() for comparing against bit-field 
width, and uses APSInt's comparison operator overload
- suppresses/avoids the warning for unnamed bit-fields
- simplifies the test case and avoids preprocessor cleverness (and thus an 
extra pass).

Thank you for taking the time to review this Richard!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91651

Files:
  clang/include/clang/AST/Decl.h
  clang/lib/Sema/SemaDecl.cpp
  clang/test/SemaCXX/warn-bitfield-enum-conversion.cpp


Index: clang/test/SemaCXX/warn-bitfield-enum-conversion.cpp
===
--- clang/test/SemaCXX/warn-bitfield-enum-conversion.cpp
+++ clang/test/SemaCXX/warn-bitfield-enum-conversion.cpp
@@ -57,3 +57,18 @@
   f.two_bits = (unsigned)three_bits_signed;
   f.two_bits = static_cast(three_bits_signed);
 }
+
+enum class E_UNSIGNED_LONG_LONG : long long { PLUS_MAX = (1LL << (sizeof(long 
long) * 8 - 2)) };
+enum class E_SIGNED_LONG_LONG : long long { PLUS_MAX = (1LL << (sizeof(long 
long) * 8 - 2)), MINUS = -1 };
+
+struct E64 {
+  E_UNSIGNED_LONG_LONG b3 : (sizeof(long long) * 8) - 1; // OK.
+  E_SIGNED_LONG_LONG   b4 : (sizeof(long long) * 8); // OK.
+
+  E_UNSIGNED_LONG_LONG b1 : 5; //expected-warning {{bit-field 'b1' is not wide 
enough}} expected-note {{widen this field to 63}}
+  E_SIGNED_LONG_LONG   b2 : 5; //expected-warning {{bit-field 'b2' is not wide 
enough}} expected-note {{widen this field to 64}}
+  
+  E_UNSIGNED_LONG_LONG : 5; // OK.  no warning for unnamed bit fields.
+  E_SIGNED_LONG_LONG   : 5; // OK.  no warning for unnamed bit fields.
+  
+};
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -16430,6 +16430,22 @@
   }
 
   if (!FieldTy->isDependentType()) {
+// Check if a named enum bit-field has enough bits to accomodate all its
+// enumerators. We can ignore unnamed enum bit-fields since they do not
+// represent values.
+if (FieldName && FieldTy->isEnumeralType()) {
+  const EnumDecl *const ED = FieldTy->castAs()->getDecl();
+
+  const auto BitsNeeded = ED->getTotalBitsNeeded();
+  if (BitsNeeded > Value) {
+Diag(FieldLoc, diag::warn_bitfield_too_small_for_enum)
+<< FieldName << ED;
+
+Diag(BitWidth->getExprLoc(), diag::note_widen_bitfield)
+<< BitsNeeded << ED << BitWidth->getSourceRange();
+  }
+}
+
 uint64_t TypeStorageSize = Context.getTypeSize(FieldTy);
 uint64_t TypeWidth = Context.getIntWidth(FieldTy);
 bool BitfieldIsOverwide = Value.ugt(TypeWidth);
Index: clang/include/clang/AST/Decl.h
===
--- clang/include/clang/AST/Decl.h
+++ clang/include/clang/AST/Decl.h
@@ -3723,6 +3723,14 @@
   /// -101 1001011 8
   unsigned getNumNegativeBits() const { return EnumDeclBits.NumNegativeBits; }
 
+  /// Returns the least width in bits required to store all the enumerators.
+  unsigned getTotalBitsNeeded() const {
+// If the enum has negative values, we need one more bit than the normal
+// number of positive bits to represent the sign bit.
+return getNumNegativeBits() > 0
+   ? std::max(getNumPositiveBits() + 1, getNumNegativeBits())
+   : getNumPositiveBits();
+  }
   /// Returns true if this is a C++11 scoped enumeration.
   bool isScoped() const { return EnumDeclBits.IsScoped; }
 


Index: clang/test/SemaCXX/warn-bitfield-enum-conversion.cpp
===
--- clang/test/SemaCXX/warn-bitfield-enum-conversion.cpp
+++ clang/test/SemaCXX/warn-bitfield-enum-conversion.cpp
@@ -57,3 +57,18 @@
   f.two_bits = (unsigned)three_bits_signed;
   f.two_bits = static_cast(three_bits_signed);
 }
+
+enum class E_UNSIGNED_LONG_LONG : long long { PLUS_MAX = (1LL << (sizeof(long long) * 8 - 2)) };
+enum class E_SIGNED_LONG_LONG : long long { PLUS_MAX = (1LL << (sizeof(long long) * 8 - 2)), MINUS = -1 };
+
+struct E64 {
+  E_UNSIGNED_LONG_LONG b3 : (sizeof(long long) * 8) - 1; // OK.
+  E_SIGNED_LONG_LONG   b4 : (sizeof(long long) * 8); // OK.
+
+  E_UNSIGNED_LONG_LONG b1 : 5; //expected-warning {{bit-field 'b1' is not wide enough}} expected-note {{widen this field to 63}}
+  E_SIGNED_LONG_LONG   b2 : 5; //expected-warning {{bit-field 'b2' is not wide enough}} expected-note {{widen this field to 64}}
+  
+  E_UNSIGNED_LONG_LONG : 5; // OK.  no warning for unnamed bit fields.
+  E_SIGNED_LONG_LONG   : 5; // OK.  no warning for unnamed bit fields.
+  
+};
Index: clang/lib/Sema/SemaDecl.cpp

[PATCH] D91651: [clang] Add a warning (à la gcc) for too small enum bitfields

[Faisal Vali via Phabricator via cfe-commits]

faisalv planned changes to this revision.
faisalv marked 3 inline comments as done.
faisalv added inline comments.



Comment at: clang/lib/Sema/SemaDecl.cpp:16443-16446
+  const unsigned BitsNeeded =
+  IsSignedEnum
+  ? std::max(ED->getNumPositiveBits() + 1, 
ED->getNumNegativeBits())
+  : ED->getNumPositiveBits();

rsmith wrote:
> Please add a member function to `EnumDecl` to compute this. SemaChecking.cpp 
> does the same calculation twice, and CGExpr.cpp does it once too, so it seems 
> past time to factor out this calculation.
Done.  Will try and commit a separate patch that replaces those uses, once this 
one has been approved.



Comment at: clang/lib/Sema/SemaDecl.cpp:16448
+  
+  if (BitsNeeded > Value.getZExtValue()) {
+// TODO: Should we be emitting diagnostics for unnamed bitfields that

rsmith wrote:
> > When comparing an APSInt to an unsigned value - should i prefer using the 
> > overloaded operator (i.e. Value < BitsNeeded) or stick with BitsNeeded > 
> > Value.getZExtValue().
> 
> Never use `get*ExtValue` unless you've already checked that the integer fits 
> in 64 bits (and even then, it's better to avoid it when you can). For 
> example, due to the incorrect pre-existing use of `getZExtValue` in this 
> function, we asserted on:
> 
> ```
> enum E {};
> struct X { E e : (__int128)0x7fff''' * 
> (__int128)0x7fff'''; };
> ```
> 
> I've fixed that in rG8e923ec2a803d54154aaa0079c1cfcf146b7a22f, but we should 
> avoid reintroducing uses of `getZExtValue` here.
Aah!  I'm glad I asked.  Thanks!



Comment at: clang/test/SemaCXX/warn-bitfield-enum-conversion.cpp:62
+
+#ifdef FV_UNNAMED_BITFIELD
+#define NAME(x)

rsmith wrote:
> Please remove the `FV_` prefix here -- authorship information is in the 
> version control history if we need it.
> 
> It would also be better to write out the test twice (once with named and once 
> with unnamed bit-fields) instead of using a macro and running the entire test 
> file an additional time. Each `clang` invocation adds to our total test time 
> much more substantially than a few more lines of testcase do, and 
> non-macro-expanded testcases are easier to understand and maintain.
> Please remove the `FV_` prefix here -- authorship information is in the 
> version control history if we need it.
> 

Hah - that wasn't a signifier of authorship but rather employment of a well 
known acronym in military circles (Field Verification :  
https://www.acronymfinder.com/Field-Verification-(Census)-(FV).html )  Lol - 
can't slip anything by you Richard ;)

> It would also be better to write out the test twice (once with named and once 
> with unnamed bit-fields) instead of using a macro and running the entire test 
> file an additional time. Each `clang` invocation adds to our total test time 
> much more substantially than a few more lines of testcase do, and 
> non-macro-expanded testcases are easier to understand and maintain.

Good to know.   Thanks. Suppression of the warning for unnamed bitfields made 
that test case even simpler.  



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91651

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

[PATCH] D91821: Fix PR42049 - Crash when parsing bad decltype use within template argument list after name assumed to be a function template

[Faisal Vali via Phabricator via cfe-commits]

faisalv created this revision.
faisalv added a reviewer: rsmith.
faisalv added a project: clang.
Herald added a subscriber: cfe-commits.
faisalv requested review of this revision.

https://bugs.llvm.org/show_bug.cgi?id=42049

Currently clang, in the following code, when tentatively parsing the 
template-argument-list recognizes the error, and skips till the semi-colon.  
But when annotating the broken decltype specifier token (in an effort to 
backtrack gracefully and continue parsing), it appears we should include all 
the cached tokens  that were skipped until the sem-colon as an annotated subset 
of our annot_decltype token.  Not doing so results in a book-keeping assertion 
failure.

void f() {

  g();

}

This situation did not seem to arise prior to 
https://github.com/llvm/llvm-project/commit/b23c5e8c3df850177449268c5ca7dbf986157525
 - which implements assumption of names as function templates even if no 
function with that name is found.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D91821

Files:
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/test/Parser/PR42049.cpp


Index: clang/test/Parser/PR42049.cpp
===
--- /dev/null
+++ clang/test/Parser/PR42049.cpp
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+// https://bugs.llvm.org/show_bug.cgi?id=42049
+
+void f() {
+  g(); // expected-error 2 {{expected}} expected-note {{to match}}
+  g2 void f2() {
+  g(); // expected-error 2 {{expected}} expected-note {{to match}}
+  g2Index: clang/test/Parser/PR42049.cpp
===
--- /dev/null
+++ clang/test/Parser/PR42049.cpp
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+// https://bugs.llvm.org/show_bug.cgi?id=42049
+
+void f() {
+  g(); // expected-error 2 {{expected}} expected-note {{to match}}
+  g2 void f2() {
+  g(); // expected-error 2 {{expected}} expected-note {{to match}}
+  g2___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D91035: [NFC, Refactor] Convert FunctionDefinitionKind from DeclSpech.h to a scoped enum

[Faisal Vali via Phabricator via cfe-commits]

faisalv added a comment.

*ping*


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91035

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

[PATCH] D91035: [NFC, Refactor] Convert FunctionDefinitionKind from DeclSpech.h to a scoped enum

[Faisal Vali via Phabricator via cfe-commits]

faisalv closed this revision.
faisalv added a comment.

committed here: 
https://github.com/llvm/llvm-project/commit/9930d4dff31a130890f21a64f43d530a83ae3d0a

Thank you Aaron, Richard and Wyat!!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91035

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

[PATCH] D91651: [clang] Add a warning (à la gcc) for too small enum bitfields

[Faisal Vali via Phabricator via cfe-commits]

faisalv requested review of this revision.
faisalv marked 4 inline comments as done.
faisalv added a comment.

*ping*


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91651

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

[PATCH] D91651: [clang] Add a warning (à la gcc) for too small enum bitfields

[Faisal Vali via Phabricator via cfe-commits]

faisalv added a comment.

In D91651#2416423 , @thakis wrote:

> Do you have any numbers on false positives / true positives uncovered by this 
> tweak?

That's a great question - and unfortunately not only do I have no hard data to 
support or discourage the addition of such a warning - I don't even know how 
some of those incredible folks (who make data based claims about programming 
smells and needs) gather such data (leave alone making sure the data is a 
representative sample for the "right" kind of programmers) - and would love to 
be shown how to run such studies :)

> In general, warning at use time instead of at declaration time tends to be 
> much better for this rate, and we do this differently than gcc in several 
> instances, because the gcc way has so many false positives that it makes 
> warnings unusable, while warning on use makes the warning useful. So I'd like 
> to see a better justification than "gcc does it" :)

Aah - i did not realize that it was a deliberate decision not to implement such 
a warning at definition time.  My justification (asides from gcc being the 
light for us in dark places, when all other lights go out ;) for implementing a 
warning at definition time probably just stems from an instinctual preference 
for early diagnostics - i suppose there is no one size that fits all here - for 
e.g. some folks prefer run-time (duck-typing) operation checking vs 
compile-time checking, and others who prefer diagnosing fundamental issues with 
template-code when they are first parsed, as opposed to waiting for some 
instantiation - and then there is the entire C++0X concepts vs concepts-lite 
discussion ...

So, since I feel I lack the authority to justify such a warning (asides from my 
ideological propensities) in the face of your valid concerns (either anecdotal 
or fueled by sufficient data) - I would prefer to defer to you and withdraw the 
patch :)  (unless someone else feels they are able to provide justification 
that might resonate with you).

Thanks for chiming in!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91651

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

[PATCH] D92733: Fix PR25627 - false positive diagnostics involving implicit-captures in dependent lambda expressions.

[Faisal Vali via Phabricator via cfe-commits]

faisalv created this revision.
faisalv added reviewers: rsmith, aaron.ballman, wchilders, BRevzin.
faisalv added a project: clang.
faisalv requested review of this revision.

This patch attempts to address the following bugs involving lambda-captures:

1. /https://bugs.llvm.org/show_bug.cgi?id=25627 which emits a false positive 
diagnostic for the following code: ```void f() { constexpr int I = 10; [](auto 
a) { return I; }; };```
  - this occurs because our current logic does not recognize that even though 
our expressions are not instantiation dependent, if that expression is being 
used to initialize some dependent entity, an lvalue-to-rvalue conversion might 
still be introduced during the instantiation, which might avert odr-use.  (we 
do not do return type deduction until instantiation).
  - to address this, I pass in the destination type to ActOnFullExpr, if the 
expression might be used in such an initialization, and use a 
PotentialResultsVisitor to determine the potential results of the 
full-expression, and then use all that information to avoid such false positive 
diagnostics.
2. it fixes any unnecessary odr-uses triggered in discarded-value expressions 
and thus some tests marked as FIXMES.
  - we simply make a call to CheckLValueToRValueConversionOperand() that 
rebuilds the relevant AST marking each potential result as non-odr-used for 
discarded value expressions even in the absence of an lvalue-to-rvalue 
conversion.
3. I do have some questions, regarding the intended behavior:

  [] {
if (false) {
  const int& i = I; <-- This is diagnosed, should it be - even though it is 
never instantiated?
}
  };
  
  [](auto a) {
if (sizeof(a) > 0) {
  const int &i = I; <-- This is diagnosed, should it be - even though it is 
never instantiated?
}
  }
  
  struct X {
constexpr static int s = 10;
int i = 10;
  };
  
  void f() {
constexpr X x;
[] { 
  int i = x.s;  // <-- This is diagnosed, should it be - even though it 
does not really use 'x' since 's' is static
  int j = x.i;  // this does not odr-use x.
}; 
  }
   

Thank you!


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92733

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/CXX/basic/basic.def.odr/p2.cpp
  clang/test/CXX/drs/dr7xx.cpp
  clang/test/SemaCXX/PR25627-generic-lambdas-capturing.cpp

Index: clang/test/SemaCXX/PR25627-generic-lambdas-capturing.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/PR25627-generic-lambdas-capturing.cpp
@@ -0,0 +1,326 @@
+// RUN: %clang_cc1 -std=c++17 -verify -fsyntax-only -fblocks -emit-llvm-only -Wno-unused-value %s
+// DONTRUNYET: %clang_cc1 -std=c++1y -verify -fsyntax-only -fblocks -fdelayed-template-parsing %s -DDELAYED_TEMPLATE_PARSING
+// DONTRUNYET: %clang_cc1 -std=c++1y -verify -fsyntax-only -fblocks -fms-extensions %s -DMS_EXTENSIONS
+// DONTRUNYET: %clang_cc1 -std=c++1y -verify -fsyntax-only -fblocks -fdelayed-template-parsing -fms-extensions %s -DMS_EXTENSIONS -DDELAYED_TEMPLATE_PARSING
+
+constexpr int ODRUSE_SZ = sizeof(char);
+
+template
+void f(T, const int (&)[N]) { }
+
+template
+void f(const T&, const int (&)[ODRUSE_SZ]) { }
+
+#define DEFINE_SELECTOR(x)   \
+  int selector_ ## x[sizeof(x) == ODRUSE_SZ ? ODRUSE_SZ : ODRUSE_SZ + 5]
+
+#define F_CALL(x, a) f(x, selector_ ## a)
+
+// This is a risky assumption, because if an empty class gets captured by value
+// the lambda's size will still be '1' 
+#define ASSERT_NO_CAPTURES(L) static_assert(sizeof(L) == 1, "size of closure with no captures must be 1")
+#define ASSERT_CLOSURE_SIZE_EXACT(L, N) static_assert(sizeof(L) == (N), "size of closure must be " #N)
+#define ASSERT_CLOSURE_SIZE(L, N) static_assert(sizeof(L) >= (N), "size of closure must be >=" #N)
+
+
+
+namespace sample {
+  struct X {  
+int i;
+constexpr X(int i) : i(i) { }
+  };
+  struct XVal {
+constexpr XVal(X) { }
+constexpr XVal() = default;
+  };
+  struct XRef {
+ constexpr XRef(const X&) { }
+  };
+} 
+
+namespace ns105 {
+
+template V f(V);
+void f(...);
+template void f(T&, T&, T);
+struct X { };
+template auto foo() {
+  extern int ei;
+  extern const U eu;
+  constexpr X x;
+  constexpr U u;
+  constexpr int I = 10;
+  auto L = [](auto a) {
+ return f(I);
+ return f(x,a);
+ return f(x);
+ U& u = eu;
+ const int &ir = ei;
+ return f(u);
+  };
+  return L;
+}
+
+}
+
+
+struct X72 {
+  constexpr static int s = 10;
+  int i;
+  int arr[4] = {1, 2, 3, 4};
+};
+void foo1_72() {
+   constexpr X72 x{5};
+   constexpr X72 y{15};
+   constexpr X72 z{25};
+   constexpr X72 w{25};
+   constexpr int arr[] = { 1, 2, 3 };
+   constexpr auto pMemI = &X72::i;
+   constexpr auto pMemArr = &X72::arr;
+   conste

[PATCH] D91821: Fix PR42049 - Crash when parsing bad decltype use within template argument list after name assumed to be a function template

[Faisal Vali via Phabricator via cfe-commits]

faisalv updated this revision to Diff 309809.
faisalv edited the summary of this revision.
faisalv added a comment.

Per Richard's suggestion, instead of including the cached tokens into the 
decltype annotation, i revert the cache to match the end of where we think the 
(broken) decltype annotated token should end.

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91821

Files:
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/test/Parser/PR42049.cpp


Index: clang/test/Parser/PR42049.cpp
===
--- /dev/null
+++ clang/test/Parser/PR42049.cpp
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+// https://bugs.llvm.org/show_bug.cgi?id=42049
+
+void f() {
+  g(); // expected-error  {{use of undeclared identifier}} 
expected-error {{expected}}
+  g2 void f2() {
+  g(); // expected-error {{use of undeclared identifier}} 
expected-error {{expected}}
+  g2Index: clang/test/Parser/PR42049.cpp
===
--- /dev/null
+++ clang/test/Parser/PR42049.cpp
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+// https://bugs.llvm.org/show_bug.cgi?id=42049
+
+void f() {
+  g(); // expected-error  {{use of undeclared identifier}} expected-error {{expected}}
+  g2 void f2() {
+  g(); // expected-error {{use of undeclared identifier}} expected-error {{expected}}
+  g2___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D150075: Fix PR#62594 : static lambda call operator is not convertible to function pointer on win32

[Faisal Vali via Phabricator via cfe-commits]

faisalv closed this revision.
faisalv added a comment.

Sorry about the delay - this patch has landed - 
https://github.com/llvm/llvm-project/commit/5bdd5d064d5171b2d5ff6268528cfffd2f86b8ea
Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150075

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

[PATCH] D92733: Fix PR25627 - false positive diagnostics involving implicit-captures in dependent lambda expressions.

[Faisal Vali via Phabricator via cfe-commits]

faisalv added a comment.

In D92733#4638319 , @cor3ntin wrote:

> @Fznamznon This might be of interest to you
> @faisalv  are you still working on this?

Yeah - sorry - been a little distracted with other stuff.
I tried uploading an updated patch to phabricator - but i get an exception - do 
i need to figure out how to convert this review to a git pull request to move 
this fwd?  Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92733

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

[PATCH] D45194: [Sema] Defer checking constexpr lambda until after we've finished the lambda class.

[Faisal Vali via Phabricator via cfe-commits]

faisalv added a comment.

Thanks for working on this fairly embarrassing bug (let's fix this before the 
week is over :)




Comment at: clang/lib/Sema/SemaDecl.cpp:12886
 if (!IsInstantiation && FD && FD->isConstexpr() && !FD->isInvalidDecl() &&
+!isLambdaCallOperator(FD) &&
 (!CheckConstexprFunctionDecl(FD) ||

Hmm - as opposed to further leaking our lambda's implementation-details into 
ActOnFinishFunctionBody - and duplicating these checks for a lambda marked 
constexpr, I think I would prefer teaching the expression-evaluator 
(ExprConstant.cpp) to know how to check a lambda's function call operator for 
constexprness (i.e. avoid creating the capture-to-field-map when 
'checkingPotentialConstantExpression()' and ignore evaluating expressions that 
refer to enclosing variables since their evaluation should not affect inferring 
constexprness).  

Thoughts?




Repository:
  rC Clang

https://reviews.llvm.org/D45194



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

[PATCH] D45194: [Sema] Defer checking constexpr lambda until after we've finished the lambda class.

[Faisal Vali via Phabricator via cfe-commits]

faisalv added a comment.

Thanks Erik!




Comment at: clang/lib/AST/ExprConstant.cpp:4312
+  } else if (MD && isLambdaCallOperator(MD)
+ && !Info.checkingPotentialConstantExpression()) {
 // We're in a lambda; determine the lambda capture field maps.

How about we add a comment here along the lines of: Do not attempt to create 
the variable-reference to closure data member map while 'constexpr' checking a 
lambda's function call operator (standard reference).  Currently constpexpr 
checking is done right after the end of the function definition for the 
syntehsized call operator marked explicitly constexpr - which occurs prior to 
adding the captures map to the closure object.  Alternatively we could have 
conditioned the check at the end of the function body to bypass lambda call 
operators and then invoke the constexpr check once the lambda is completely 
processed.  

Between you and me, I'm a little torn about this approach - if you can make an 
argument to consider your approach over this one - i think i could be swayed 
(if i'm not already ;) - unless of course richard weighs in as a tie breaker.



Comment at: clang/lib/AST/ExprConstant.cpp:5205
   // field or what the field refers to.
   if (Info.CurrentCall && isLambdaCallOperator(Info.CurrentCall->Callee)) {
+// We don't track the lambda's captures in a potential constant expression.

I think you might want to add a check here to determine if E 
refersToEnclosingVariableOrCapture()?


https://reviews.llvm.org/D45194



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

[PATCH] D45194: [Sema] Defer checking constexpr lambda until after we've finished the lambda class.

[Faisal Vali via Phabricator via cfe-commits]

faisalv added a comment.

LGTM - can you commit?
Thank you!




Comment at: clang/lib/AST/ExprConstant.cpp:4313
+// We're in a lambda; determine the lambda capture field maps unless we're
+// just constexpr checking a lambda's call operator. constexpr checking is
+// done before the captures have been added to the closure object, so we

should we say something about only explicitly specified constexpr checking (as 
opposed to constexpr inference) is done prior to capture information being 
stored in the CXXRecordDecl? *shrug*.

Also the upside of having the expression evaluator evaluate these expressions 
is we could diagnose such lambdas as ill-formed (though no-diagnostic required 
per the standard): 
  void f() {
volatile int v = 1;
[&] () constexpr { return v; };  // can never be a core constant 
expression, right?
  }

I think I agree w you though - so, for now lets stick with this version of the 
patch?



Comment at: clang/lib/AST/ExprConstant.cpp:5212
+  cast(E)->refersToEnclosingVariableOrCapture()) {
+// We don't track the lambda's captures in a potential constant expression.
+if (Info.checkingPotentialConstantExpression())

How about something along the lines of : We don't always have a complete 
capture-map when checking or inferring if the function call operator meets the 
requirements of a constexpr function - and we don't need to evaluate the 
captures to determine constexprness (dcl.constexpr C++17)?


https://reviews.llvm.org/D45194



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

[PATCH] D39166: [NFC] Add some assertions to placate my paranoia about sharing a variant bit across FunctionDecl and CXXDeductionGuideDecl - should I do this?

[Faisal Vali via Phabricator via cfe-commits]

faisalv updated this revision to Diff 120346.
faisalv marked 4 inline comments as done.
faisalv added a comment.

Incorporated Aaron's feedback (although not sure if I caugh tall the white 
space issues).

Additionally, I was thinking of reordering the bits - and using UseSEHTry (this 
bit really makes no sense and even the chance of someone triggering its use 
erroneously (which can happen with the current bit if someone gives the 
deduction guide a body, should be zero) as my fusion bit.

I'm also not opposed to just adding a bit to the end of the FunctionDecl bit 
sequence.

Thanks for taking a look at the patch Aaron!


Repository:
  rL LLVM

https://reviews.llvm.org/D39166

Files:
  include/clang/AST/Decl.h
  include/clang/AST/InlineDeclMembers.h
  include/clang/Sema/Sema.h
  lib/AST/Decl.cpp
  lib/AST/DeclCXX.cpp
  lib/Parse/ParseCXXInlineMethods.cpp
  lib/Sema/SemaDecl.cpp

Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -12175,7 +12175,8 @@
 
   // See if this is a redefinition. If 'will have body' is already set, then
   // these checks were already performed when it was set.
-  if (!FD->willHaveBody() && !FD->isLateTemplateParsed()) {
+  if (!isa(FD) && !FD->willHaveBody() &&
+  !FD->isLateTemplateParsed()) {
 CheckForFunctionRedefinition(FD, nullptr, SkipBody);
 
 // If we're skipping the body, we're done. Don't enter the scope.
@@ -12186,7 +12187,8 @@
   // Mark this function as "will have a body eventually".  This lets users to
   // call e.g. isInlineDefinitionExternallyVisible while we're still parsing
   // this function.
-  FD->setWillHaveBody();
+  if (!isa(FD))
+FD->setWillHaveBody();
 
   // If we are instantiating a generic lambda call operator, push
   // a LambdaScopeInfo onto the function stack.  But use the information
@@ -12373,7 +12375,8 @@
 
   if (FD) {
 FD->setBody(Body);
-FD->setWillHaveBody(false);
+if (!isa(FD))
+  FD->setWillHaveBody(false);
 
 if (getLangOpts().CPlusPlus14) {
   if (!FD->isInvalidDecl() && Body && !FD->isDependentContext() &&
Index: lib/Parse/ParseCXXInlineMethods.cpp
===
--- lib/Parse/ParseCXXInlineMethods.cpp
+++ lib/Parse/ParseCXXInlineMethods.cpp
@@ -13,6 +13,7 @@
 
 #include "clang/Parse/Parser.h"
 #include "clang/AST/DeclTemplate.h"
+#include "clang/AST/InlineDeclMembers.h"
 #include "clang/Parse/ParseDiagnostic.h"
 #include "clang/Parse/RAIIObjectsForParser.h"
 #include "clang/Sema/DeclSpec.h"
Index: lib/AST/DeclCXX.cpp
===
--- lib/AST/DeclCXX.cpp
+++ lib/AST/DeclCXX.cpp
@@ -18,6 +18,7 @@
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/Expr.h"
 #include "clang/AST/ExprCXX.h"
+#include "clang/AST/InlineDeclMembers.h"
 #include "clang/AST/ODRHash.h"
 #include "clang/AST/TypeLoc.h"
 #include "clang/Basic/IdentifierTable.h"
Index: lib/AST/Decl.cpp
===
--- lib/AST/Decl.cpp
+++ lib/AST/Decl.cpp
@@ -23,6 +23,7 @@
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/Expr.h"
 #include "clang/AST/ExprCXX.h"
+#include "clang/AST/InlineDeclMembers.h"
 #include "clang/AST/PrettyPrinter.h"
 #include "clang/AST/Stmt.h"
 #include "clang/AST/TypeLoc.h"
Index: include/clang/Sema/Sema.h
===
--- include/clang/Sema/Sema.h
+++ include/clang/Sema/Sema.h
@@ -22,6 +22,7 @@
 #include "clang/AST/Expr.h"
 #include "clang/AST/ExprObjC.h"
 #include "clang/AST/ExternalASTSource.h"
+#include "clang/AST/InlineDeclMembers.h"
 #include "clang/AST/LocInfoType.h"
 #include "clang/AST/MangleNumberingContext.h"
 #include "clang/AST/NSAPI.h"
Index: include/clang/AST/InlineDeclMembers.h
===
--- include/clang/AST/InlineDeclMembers.h
+++ include/clang/AST/InlineDeclMembers.h
@@ -0,0 +1,39 @@
+//===- InlineDeclMembers.h - Decl.h Members that must be inlined -*- C++ -*-==//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+//  This file defines inline members of classes defined in Decl.h that have
+//  (circular) dependencies on other files and should only be included
+//  in .cpp files (as opposed to .h).
+//
+//===--===//
+
+#ifndef LLVM_CLANG_AST_INLINEDECLMEMBERS_H
+#define LLVM_CLANG_AST_INLINEDECLMEMBERS_H
+
+#include "clang/AST/Decl.h"
+#include "clang/AST/DeclCXX.h"
+namespace clang {
+
+inline bool FunctionDecl::willHaveBody() const {
+  assert(!isa(this) &&
+ "must not be called on a deduction guide since we share this d

[PATCH] D39166: [NFC] Add some assertions to placate my paranoia about sharing a variant bit across FunctionDecl and CXXDeductionGuideDecl - should I do this?

[Faisal Vali via Phabricator via cfe-commits]

faisalv abandoned this revision.
faisalv added a comment.

Just added an additional bit-field to FunctionDecl in 
https://reviews.llvm.org/rL317984




Comment at: include/clang/AST/InlineDeclMembers.h:35
+
+#endif  //LLVM_CLANG_AST_INLINEDECLMEMBERS_H
+

aaron.ballman wrote:
> Whitespace is incorrect here.
not sure I follow?



Repository:
  rL LLVM

https://reviews.llvm.org/D39166



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

[PATCH] D150075: Fix PR#62594 : static lambda call operator is not convertible to function pointer on win32

[Faisal Vali via Phabricator via cfe-commits]

faisalv created this revision.
faisalv added reviewers: aaron.ballman, shafik, royjacobson.
faisalv added a project: clang.
Herald added a subscriber: pengfei.
Herald added a project: All.
faisalv requested review of this revision.
Herald added a subscriber: cfe-commits.

See issue https://github.com/llvm/llvm-project/issues/62594

This code does not work on win32:

  cpp
auto lstatic = []()  static  { return 0;  };
int (*f2)(void) = lstatic; 

Since a calling convention such as CC_X86ThisCall can rightly interfere with 
the implicit pointer to function conversion if erroneously marked on a static 
function, the fix entails checking the 'static' specifier on the lambda 
declarator prior to assigning it a calling convention of an non-static member 
(which pre-c++23 made sense).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D150075

Files:
  clang/lib/Sema/SemaType.cpp
  clang/test/SemaCXX/cxx23-static-callop-lambda-expression.cpp


Index: clang/test/SemaCXX/cxx23-static-callop-lambda-expression.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/cxx23-static-callop-lambda-expression.cpp
@@ -0,0 +1,32 @@
+// RUN: %clang_cc1 -std=c++23 -fsyntax-only -verify %s
+
+
+namespace ns1 {
+  auto lstatic = []() static { return 3; }; 
+  int (*f2)(void) = lstatic;   
+
+}
+
+namespace ns1_1 {
+  
+  auto lstatic = []() static consteval  //expected-note {{declared here}} 
expected-error{{cannot take address of consteval call}}
+  { return 3; };   
+  
+  // FIXME: the above error should indicate that it was triggered below.
+  int (*f2)(void) = lstatic;   
+
+}
+
+
+namespace ns2 {
+  auto lstatic = []() static { return 3; }; 
+  constexpr int (*f2)(void) = lstatic;  
+  static_assert(lstatic() == f2());
+}
+
+namespace ns3 {
+  void main() {
+static int x = 10;
+auto L = []() static { return x; };
+  }
+}
\ No newline at end of file
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -4057,8 +4057,9 @@
   D.getTypeObject(I).Kind == DeclaratorChunk::MemberPointer;
 } else if (D.getContext() == DeclaratorContext::LambdaExpr) {
   // This can only be a call operator for a lambda, which is an instance
-  // method.
-  IsCXXInstanceMethod = true;
+  // method, unless explicitly specified as 'static'.
+  IsCXXInstanceMethod =
+  D.getDeclSpec().getStorageClassSpec() != DeclSpec::SCS_static;
 } else {
   // We're the innermost decl chunk, so must be a function declarator.
   assert(D.isFunctionDeclarator());


Index: clang/test/SemaCXX/cxx23-static-callop-lambda-expression.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/cxx23-static-callop-lambda-expression.cpp
@@ -0,0 +1,32 @@
+// RUN: %clang_cc1 -std=c++23 -fsyntax-only -verify %s
+
+
+namespace ns1 {
+  auto lstatic = []() static { return 3; }; 
+  int (*f2)(void) = lstatic;   
+
+}
+
+namespace ns1_1 {
+  
+  auto lstatic = []() static consteval  //expected-note {{declared here}} expected-error{{cannot take address of consteval call}}
+  { return 3; };   
+  
+  // FIXME: the above error should indicate that it was triggered below.
+  int (*f2)(void) = lstatic;   
+
+}
+
+
+namespace ns2 {
+  auto lstatic = []() static { return 3; }; 
+  constexpr int (*f2)(void) = lstatic;  
+  static_assert(lstatic() == f2());
+}
+
+namespace ns3 {
+  void main() {
+static int x = 10;
+auto L = []() static { return x; };
+  }
+}
\ No newline at end of file
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -4057,8 +4057,9 @@
   D.getTypeObject(I).Kind == DeclaratorChunk::MemberPointer;
 } else if (D.getContext() == DeclaratorContext::LambdaExpr) {
   // This can only be a call operator for a lambda, which is an instance
-  // method.
-  IsCXXInstanceMethod = true;
+  // method, unless explicitly specified as 'static'.
+  IsCXXInstanceMethod =
+  D.getDeclSpec().getStorageClassSpec() != DeclSpec::SCS_static;
 } else {
   // We're the innermost decl chunk, so must be a function declarator.
   assert(D.isFunctionDeclarator());
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D22997: [cxx1z-constexpr-lambda] Make conversion function constexpr, and teach the expression-evaluator to evaluate the static-invoker.

[Faisal Vali via Phabricator via cfe-commits]

faisalv closed this revision.
faisalv added a comment.

Committed as https://reviews.llvm.org/rL291397


https://reviews.llvm.org/D22997



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

[PATCH] D22997: [cxx1z-constexpr-lambda] Make conversion function constexpr, and teach the expression-evaluator to evaluate the static-invoker.

[Faisal Vali via Phabricator via cfe-commits]

faisalv accepted this revision.
faisalv added a reviewer: faisalv.
faisalv added a comment.
This revision is now accepted and ready to land.

committed as https://reviews.llvm.org/rL291397


https://reviews.llvm.org/D22997



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

[PATCH] D18510: [cxx1z-constexpr-lambda] Make conversion function constexpr

[Faisal Vali via Phabricator via cfe-commits]

faisalv abandoned this revision.
faisalv added a comment.

See richard's comment for why this attempt at implementing a compatibility 
warning for the constexpr conversion operator for lambda's is being abandoned.


https://reviews.llvm.org/D18510



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

[PATCH] D28510: Reinstate CWG1607 restrictions on lambdas appearing inside certain constant-expressions

[Faisal Vali via Phabricator via cfe-commits]

faisalv created this revision.
faisalv added a reviewer: rsmith.
faisalv added a subscriber: cfe-commits.
faisalv set the repository for this revision to rL LLVM.
faisalv added a project: clang-c.

This patch disables lambda expressions (especially Immediately Invoked Lambda 
Expressions (IILEs, to borrow a term from the all-mighty ecmascript ;)) from 
appearing within potentially mangled contexts.

The approach is a rather primitive/inelegant one.  Instead of enumerating all 
the various syntactic constant-expression contexts at a finer level of 
granularity and then passing that information through to each call of 
ParseConstantExpression, we simply set a flag that forbids lambda expressions 
(when we know we are in the forbidden context).  There is probably an 
opportunity here to streamline the machinery that prohibits lambdas in certain 
contexts across the various versions of C++ further - but that could be 
re-engineered if/when Louis Dionne's paper on Lambdas in Unevaluated contexts 
gets incorporated into the working draft.

Would appreciate some feedback here - thanks!


Repository:
  rL LLVM

https://reviews.llvm.org/D28510

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Parse/Parser.h
  include/clang/Sema/Sema.h
  lib/Parse/ParseDecl.cpp
  lib/Parse/ParseExpr.cpp
  lib/Parse/ParseTemplate.cpp
  lib/Sema/Sema.cpp
  lib/Sema/SemaExpr.cpp
  test/SemaCXX/cxx1z-constexpr-lambdas.cpp

Index: test/SemaCXX/cxx1z-constexpr-lambdas.cpp
===
--- test/SemaCXX/cxx1z-constexpr-lambdas.cpp
+++ test/SemaCXX/cxx1z-constexpr-lambdas.cpp
@@ -157,6 +157,38 @@
 
 } // end ns1_simple_lambda
 
+namespace test_forbidden_lambda_expressions {
+
+template struct X { }; //expected-error{{lambda expression may not appear}}
+X<[]{return 10; }()> x; //expected-error{{lambda expression may not appear}}
+void f(int arr[([] { return 5; }())]); //expected-error{{lambda expression may not appear}}
+// FIXME: Should this be ok?
+auto L = [](int arr[([] { return 5; }())]) { }; // OK
+
+// These should be allowed:
+struct A {
+  int : ([] { return 5; }());
+};
+
+int arr[([] { return 5; }())];
+enum { E = [] { return 5; }() };
+static_assert([]{return 5; }() == 5);
+int *ip = new int[([] { return 5; })()];
+
+int test_case(int x) {
+  switch(x) {
+case [] { return 5; }(): //OK
+  break;
+case [] (auto a) { return a; }(6): //expected-note{{previous}}
+  break;
+case 6:  //expected-error{{duplicate}}
+  break;
+  }
+  return x;
+}
+
+} // end ns forbidden_lambda_expressions
+
 namespace ns1_unimplemented {
 namespace ns1_captures {
 constexpr auto f(int i) {
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -13083,9 +13083,11 @@
 void
 Sema::PushExpressionEvaluationContext(ExpressionEvaluationContext NewContext,
   Decl *LambdaContextDecl,
-  bool IsDecltype) {
+  bool IsDecltype,
+  bool IsLambdaExprForbidden) {
   ExprEvalContexts.emplace_back(NewContext, ExprCleanupObjects.size(), Cleanup,
-LambdaContextDecl, IsDecltype);
+LambdaContextDecl, IsDecltype,
+IsLambdaExprForbidden);
   Cleanup.reset();
   if (!MaybeODRUseExprs.empty())
 std::swap(MaybeODRUseExprs, ExprEvalContexts.back().SavedMaybeODRUseExprs);
@@ -13094,9 +13096,11 @@
 void
 Sema::PushExpressionEvaluationContext(ExpressionEvaluationContext NewContext,
   ReuseLambdaContextDecl_t,
-  bool IsDecltype) {
+  bool IsDecltype,
+  bool IsLambdaExprForbidden) {
   Decl *ClosureContextDecl = ExprEvalContexts.back().ManglingContextDecl;
-  PushExpressionEvaluationContext(NewContext, ClosureContextDecl, IsDecltype);
+  PushExpressionEvaluationContext(NewContext, ClosureContextDecl, IsDecltype,
+  IsLambdaExprForbidden);
 }
 
 void Sema::PopExpressionEvaluationContext() {
@@ -13117,6 +13121,8 @@
 //   evaluation of e, following the rules of the abstract machine, would
 //   evaluate [...] a lambda-expression.
 D = diag::err_lambda_in_constant_expression;
+if (getLangOpts().CPlusPlus1z && Rec.IsLambdaExprForbidden)
+  D = diag::err_lambda_in_potentially_mangled_constant_expression;
   }
 
   // C++1z allows lambda expressions as core constant expressions.
@@ -13125,9 +13131,11 @@
   // are part of function-signatures.  Be mindful that P0315 (Lambdas in
   // unevaluated contexts) might lift some of these restrictions in a 
   // future version.
-  if (Rec.Context != ConstantEvaluated || !getLangOp

[PATCH] D28510: Reinstate CWG1607 restrictions on lambdas appearing inside certain constant-expressions

[Faisal Vali via Phabricator via cfe-commits]

faisalv added a comment.

Yes - I'll modify the patch to reflect that approach.  Any thoughts on whether 
they should also be forbidden in Lamda parameter declaration clauses?
Thanks!


Repository:
  rL LLVM

https://reviews.llvm.org/D28510



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

[PATCH] D28510: Reinstate CWG1607 restrictions on lambdas appearing inside certain constant-expressions

[Faisal Vali via Phabricator via cfe-commits]

faisalv added inline comments.



Comment at: include/clang/Sema/Sema.h:894-895
 
+/// \brief Whether lambda expressions are forbidden here.
+bool IsLambdaExprForbidden;
+

rsmith wrote:
> Rather than adding a flag, how about we have two different kinds of 
> `ExpressionEvaluationContext` for constant expressions: a generic "constant 
> expression" context and a "constant expression in signature" context?
But not all lambdas that appear in template arguments are in signatures - so 
should we split it into ConstantEvaluated, 
ConstantEvaluatedInSignatureOrTemplateArg ? 



Comment at: lib/Parse/ParseDecl.cpp:6254
+  NumElements = ParseConstantExpression(NotTypeCast,
+   /*IsLambdaExprForbidden=*/D.getContext() == D.PrototypeContext);
 } else {

I suppose we should forbid these in a lambda parameter declaration clause too?


Repository:
  rL LLVM

https://reviews.llvm.org/D28510



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

[PATCH] D31414: [NFC, Refactor] Modernize TemplateIdAnnotation using TrailingObjects

[Faisal Vali via Phabricator via cfe-commits]

faisalv created this revision.
faisalv added a project: clang-c.

Refactor TemplateIdAnnotation to use TrailingObjects and add assorted comments.


Repository:
  rL LLVM

https://reviews.llvm.org/D31414

Files:
  include/clang/Basic/TemplateKinds.h
  include/clang/Sema/ParsedTemplate.h
  lib/Parse/ParseExprCXX.cpp
  lib/Parse/ParseTemplate.cpp

Index: lib/Parse/ParseTemplate.cpp
===
--- lib/Parse/ParseTemplate.cpp
+++ lib/Parse/ParseTemplate.cpp
@@ -1023,25 +1023,21 @@
 // Build a template-id annotation token that can be processed
 // later.
 Tok.setKind(tok::annot_template_id);
-TemplateIdAnnotation *TemplateId
-  = TemplateIdAnnotation::Allocate(TemplateArgs.size(), TemplateIds);
-TemplateId->TemplateNameLoc = TemplateNameLoc;
-if (TemplateName.getKind() == UnqualifiedId::IK_Identifier) {
-  TemplateId->Name = TemplateName.Identifier;
-  TemplateId->Operator = OO_None;
-} else {
-  TemplateId->Name = nullptr;
-  TemplateId->Operator = TemplateName.OperatorFunctionId.Operator;
-}
-TemplateId->SS = SS;
-TemplateId->TemplateKWLoc = TemplateKWLoc;
-TemplateId->Template = Template;
-TemplateId->Kind = TNK;
-TemplateId->LAngleLoc = LAngleLoc;
-TemplateId->RAngleLoc = RAngleLoc;
-ParsedTemplateArgument *Args = TemplateId->getTemplateArgs();
-for (unsigned Arg = 0, ArgEnd = TemplateArgs.size(); Arg != ArgEnd; ++Arg)
-  Args[Arg] = ParsedTemplateArgument(TemplateArgs[Arg]);
+
+IdentifierInfo *TemplateII =
+TemplateName.getKind() == UnqualifiedId::IK_Identifier
+? TemplateName.Identifier
+: nullptr;
+
+OverloadedOperatorKind OpKind =
+TemplateName.getKind() == UnqualifiedId::IK_Identifier
+? OO_None
+: TemplateName.OperatorFunctionId.Operator;
+
+TemplateIdAnnotation *TemplateId = TemplateIdAnnotation::Create(
+  SS, TemplateKWLoc, TemplateNameLoc, TemplateII, OpKind, Template, TNK,
+  LAngleLoc, RAngleLoc, TemplateArgs, TemplateIds);
+
 Tok.setAnnotationValue(TemplateId);
 if (TemplateKWLoc.isValid())
   Tok.setLocation(TemplateKWLoc);
Index: lib/Parse/ParseExprCXX.cpp
===
--- lib/Parse/ParseExprCXX.cpp
+++ lib/Parse/ParseExprCXX.cpp
@@ -2124,31 +2124,18 @@
   Id.getKind() == UnqualifiedId::IK_LiteralOperatorId) {
 // Form a parsed representation of the template-id to be stored in the
 // UnqualifiedId.
-TemplateIdAnnotation *TemplateId
-  = TemplateIdAnnotation::Allocate(TemplateArgs.size(), TemplateIds);
 
 // FIXME: Store name for literal operator too.
-if (Id.getKind() == UnqualifiedId::IK_Identifier) {
-  TemplateId->Name = Id.Identifier;
-  TemplateId->Operator = OO_None;
-  TemplateId->TemplateNameLoc = Id.StartLocation;
-} else {
-  TemplateId->Name = nullptr;
-  TemplateId->Operator = Id.OperatorFunctionId.Operator;
-  TemplateId->TemplateNameLoc = Id.StartLocation;
-}
+IdentifierInfo *TemplateII =
+Id.getKind() == UnqualifiedId::IK_Identifier ? Id.Identifier : nullptr;
+OverloadedOperatorKind OpKind = Id.getKind() == UnqualifiedId::IK_Identifier
+? OO_None
+: Id.OperatorFunctionId.Operator;
 
-TemplateId->SS = SS;
-TemplateId->TemplateKWLoc = TemplateKWLoc;
-TemplateId->Template = Template;
-TemplateId->Kind = TNK;
-TemplateId->LAngleLoc = LAngleLoc;
-TemplateId->RAngleLoc = RAngleLoc;
-ParsedTemplateArgument *Args = TemplateId->getTemplateArgs();
-for (unsigned Arg = 0, ArgEnd = TemplateArgs.size(); 
- Arg != ArgEnd; ++Arg)
-  Args[Arg] = TemplateArgs[Arg];
-
+TemplateIdAnnotation *TemplateId = TemplateIdAnnotation::Create(
+SS, TemplateKWLoc, Id.StartLocation, TemplateII, OpKind, Template, TNK,
+LAngleLoc, RAngleLoc, TemplateArgs, TemplateIds);
+
 Id.setTemplateId(TemplateId);
 return false;
   }
Index: include/clang/Sema/ParsedTemplate.h
===
--- include/clang/Sema/ParsedTemplate.h
+++ include/clang/Sema/ParsedTemplate.h
@@ -145,12 +145,15 @@
   /// expressions, or template names, and the source locations for important 
   /// tokens. All of the information about template arguments is allocated 
   /// directly after this structure.
-  struct TemplateIdAnnotation {
+  struct TemplateIdAnnotation final
+  : private llvm::TrailingObjects {
+friend TrailingObjects;
 /// \brief The nested-name-specifier that precedes the template name.
 CXXScopeSpec SS;
 
-/// TemplateKWLoc - The location of the template keyword within the
-/// source.
+/// TemplateKWLoc - The location of the template keyword.
+/// For e.g. typename T::template Y
 SourceLocation TemplateKWLoc;
 
   

[PATCH] D31414: [NFC, Refactor] Modernize TemplateIdAnnotation using TrailingObjects

[Faisal Vali via Phabricator via cfe-commits]

faisalv updated this revision to Diff 93207.
faisalv added a comment.

Don't forget to destroy the trailing objects.


https://reviews.llvm.org/D31414

Files:
  include/clang/Basic/TemplateKinds.h
  include/clang/Sema/ParsedTemplate.h
  lib/Parse/ParseExprCXX.cpp
  lib/Parse/ParseTemplate.cpp

Index: lib/Parse/ParseTemplate.cpp
===
--- lib/Parse/ParseTemplate.cpp
+++ lib/Parse/ParseTemplate.cpp
@@ -1023,25 +1023,21 @@
 // Build a template-id annotation token that can be processed
 // later.
 Tok.setKind(tok::annot_template_id);
-TemplateIdAnnotation *TemplateId
-  = TemplateIdAnnotation::Allocate(TemplateArgs.size(), TemplateIds);
-TemplateId->TemplateNameLoc = TemplateNameLoc;
-if (TemplateName.getKind() == UnqualifiedId::IK_Identifier) {
-  TemplateId->Name = TemplateName.Identifier;
-  TemplateId->Operator = OO_None;
-} else {
-  TemplateId->Name = nullptr;
-  TemplateId->Operator = TemplateName.OperatorFunctionId.Operator;
-}
-TemplateId->SS = SS;
-TemplateId->TemplateKWLoc = TemplateKWLoc;
-TemplateId->Template = Template;
-TemplateId->Kind = TNK;
-TemplateId->LAngleLoc = LAngleLoc;
-TemplateId->RAngleLoc = RAngleLoc;
-ParsedTemplateArgument *Args = TemplateId->getTemplateArgs();
-for (unsigned Arg = 0, ArgEnd = TemplateArgs.size(); Arg != ArgEnd; ++Arg)
-  Args[Arg] = ParsedTemplateArgument(TemplateArgs[Arg]);
+
+IdentifierInfo *TemplateII =
+TemplateName.getKind() == UnqualifiedId::IK_Identifier
+? TemplateName.Identifier
+: nullptr;
+
+OverloadedOperatorKind OpKind =
+TemplateName.getKind() == UnqualifiedId::IK_Identifier
+? OO_None
+: TemplateName.OperatorFunctionId.Operator;
+
+TemplateIdAnnotation *TemplateId = TemplateIdAnnotation::Create(
+  SS, TemplateKWLoc, TemplateNameLoc, TemplateII, OpKind, Template, TNK,
+  LAngleLoc, RAngleLoc, TemplateArgs, TemplateIds);
+
 Tok.setAnnotationValue(TemplateId);
 if (TemplateKWLoc.isValid())
   Tok.setLocation(TemplateKWLoc);
Index: lib/Parse/ParseExprCXX.cpp
===
--- lib/Parse/ParseExprCXX.cpp
+++ lib/Parse/ParseExprCXX.cpp
@@ -2124,31 +2124,18 @@
   Id.getKind() == UnqualifiedId::IK_LiteralOperatorId) {
 // Form a parsed representation of the template-id to be stored in the
 // UnqualifiedId.
-TemplateIdAnnotation *TemplateId
-  = TemplateIdAnnotation::Allocate(TemplateArgs.size(), TemplateIds);
 
 // FIXME: Store name for literal operator too.
-if (Id.getKind() == UnqualifiedId::IK_Identifier) {
-  TemplateId->Name = Id.Identifier;
-  TemplateId->Operator = OO_None;
-  TemplateId->TemplateNameLoc = Id.StartLocation;
-} else {
-  TemplateId->Name = nullptr;
-  TemplateId->Operator = Id.OperatorFunctionId.Operator;
-  TemplateId->TemplateNameLoc = Id.StartLocation;
-}
+IdentifierInfo *TemplateII =
+Id.getKind() == UnqualifiedId::IK_Identifier ? Id.Identifier : nullptr;
+OverloadedOperatorKind OpKind = Id.getKind() == UnqualifiedId::IK_Identifier
+? OO_None
+: Id.OperatorFunctionId.Operator;
 
-TemplateId->SS = SS;
-TemplateId->TemplateKWLoc = TemplateKWLoc;
-TemplateId->Template = Template;
-TemplateId->Kind = TNK;
-TemplateId->LAngleLoc = LAngleLoc;
-TemplateId->RAngleLoc = RAngleLoc;
-ParsedTemplateArgument *Args = TemplateId->getTemplateArgs();
-for (unsigned Arg = 0, ArgEnd = TemplateArgs.size(); 
- Arg != ArgEnd; ++Arg)
-  Args[Arg] = TemplateArgs[Arg];
-
+TemplateIdAnnotation *TemplateId = TemplateIdAnnotation::Create(
+SS, TemplateKWLoc, Id.StartLocation, TemplateII, OpKind, Template, TNK,
+LAngleLoc, RAngleLoc, TemplateArgs, TemplateIds);
+
 Id.setTemplateId(TemplateId);
 return false;
   }
Index: include/clang/Sema/ParsedTemplate.h
===
--- include/clang/Sema/ParsedTemplate.h
+++ include/clang/Sema/ParsedTemplate.h
@@ -145,12 +145,15 @@
   /// expressions, or template names, and the source locations for important 
   /// tokens. All of the information about template arguments is allocated 
   /// directly after this structure.
-  struct TemplateIdAnnotation {
+  struct TemplateIdAnnotation final
+  : private llvm::TrailingObjects {
+friend TrailingObjects;
 /// \brief The nested-name-specifier that precedes the template name.
 CXXScopeSpec SS;
 
-/// TemplateKWLoc - The location of the template keyword within the
-/// source.
+/// TemplateKWLoc - The location of the template keyword.
+/// For e.g. typename T::template Y
 SourceLocation TemplateKWLoc;
 
 /// TemplateNameLoc - The location of the template

[PATCH] D31588: Fix PR25627: Certain constant local variables must be usable as template arguments (without being odr-used)

[Faisal Vali via Phabricator via cfe-commits]

faisalv created this revision.

This patch ensures that clang processes the expression-nodes that are generated 
when disambiguating between types and expressions within template arguments, as 
if they were truly constant-expressions.  Currently, trunk correctly 
disambiguates, and identifies the expression as an expression - and while it 
annotates the token with the expression - it fails to complete the odr-use 
processing (specifically, failing to trigger 
Sema::UpdateMarkingForLValueToRValue as is done so for all Constant 
Expressions, which removes it from being considered odr-used).

For e.g:
template struct X { };
void f() {

  const int N = 10;
  X x; // should be OK.
  [] { return X{}; }; // also OK - no capture.

}
See a related bug: https://bugs.llvm.org//show_bug.cgi?id=25627

The fix is as follows:

- Remove the EnteredConstantEvaluatedContext action from 
ParseTemplateArgumentList (relying that ParseTemplateArgument will get it right)
- Add the EnteredConstantEvaluatedContext action just prior to undergoing the 
disambiguating parse, and if the parse succeeds for an expression, make sure it 
doesn't linger within MaybeODRUsedExprs by clearing it (while asserting that it 
only contains the offending expression)

I need to add some tests... and fix one regression.

Does the approach look sound?
Thanks!


Repository:
  rL LLVM

https://reviews.llvm.org/D31588

Files:
  lib/Parse/ParseTemplate.cpp

Index: lib/Parse/ParseTemplate.cpp
===
--- lib/Parse/ParseTemplate.cpp
+++ lib/Parse/ParseTemplate.cpp
@@ -1198,42 +1198,73 @@
   //   expression is resolved to a type-id, regardless of the form of
   //   the corresponding template-parameter.
   //
-  // Therefore, we initially try to parse a type-id.  
-  if (isCXXTypeId(TypeIdAsTemplateArgument)) {
-SourceLocation Loc = Tok.getLocation();
-TypeResult TypeArg = ParseTypeName(/*Range=*/nullptr,
-   Declarator::TemplateTypeArgContext);
-if (TypeArg.isInvalid())
-  return ParsedTemplateArgument();
-
-return ParsedTemplateArgument(ParsedTemplateArgument::Type,
-  TypeArg.get().getAsOpaquePtr(), 
-  Loc);
-  }
-  
+  // Therefore, we initially try to parse a type-id.
+  {
+EnterExpressionEvaluationContext EnterConstantEvaluated(
+Actions, Sema::ExpressionEvaluationContext::ConstantEvaluated);
+if (isCXXTypeId(TypeIdAsTemplateArgument)) {
+  SourceLocation Loc = Tok.getLocation();
+  TypeResult TypeArg =
+  ParseTypeName(/*Range=*/nullptr, Declarator::TemplateTypeArgContext);
+  if (TypeArg.isInvalid())
+return ParsedTemplateArgument();
+
+  return ParsedTemplateArgument(ParsedTemplateArgument::Type,
+TypeArg.get().getAsOpaquePtr(), Loc);
+} else {
+  // If we disambiguated as an expression that we identified as potentially
+  // not being odr-used (consistent with a template argument context), and
+  // annotated our token as that expression, then remove it from the
+  // MaybeODRUsedExprs so that it doesn't trigger a false error, since it
+  // would otherwise have been removed when completing processing of a
+  // constant expression.
+  if (Actions.MaybeODRUseExprs.size()) {
+assert(Actions.MaybeODRUseExprs.size() == 1 &&
+   "we should only need to parse one expression to determine an "
+   "error or typeid");
+assert(Tok.getKind() == tok::annot_primary_expr &&
+   getExprAnnotation(Tok).get() ==
+   *Actions.MaybeODRUseExprs.begin() &&
+   "The expression (DeclRefExpr or MemExpr) stored within "
+   "MaybeODRUseExprs for later checking whether we perform an "
+   "lvalue-to-rvalue conversion before determining odr-use must be "
+   "the same as the annotated token during ambiguity resolution");
+// We clear this state, since this lingering partially processed
+// expression should not trigger an error now and we don't need to save
+// it for later since we can be certain that this expression would
+// eventually have been removed during ActOnConstantExpression called
+// from ParseConstantExpression when parsing the non-type template
+// argument below.  Eitherway, the appropriate checking for an
+// appropriate constant-expression that matches the
+// non-type-template-parameter occurs later in CheckTemplateArgument.
+Actions.MaybeODRUseExprs.clear();
+  }
+}
+  } // End ExpressionEvaluationContext
+
   // Try to parse a template template argument.
   {
 TentativeParsingAction TPA(*this);
 
-ParsedTemplateArgument TemplateTemplateArgument
-  = ParseTemplateTemplateArgument();
+ParsedTemplateArgument TemplateTemplateArgument =
+ParseTem

[PATCH] D28510: Reinstate CWG1607 restrictions on lambdas appearing inside certain constant-expressions

[Faisal Vali via Phabricator via cfe-commits]

faisalv updated this revision to Diff 84476.
faisalv added a comment.

The updated patch adds two additional enumerators to 
ExpressionEvaluationContext: ConstantEvaluatedInTemplateArgument and 
ConstantEvaluatedInFunctionSignature and sets them appropriately (as opposed to 
our previous approach of setting the IsLambdaExpressionForbidden flag in those 
locations).

When popping off the EvaluationContext, instead of checking the flag, we now 
check the EvaluationContext directly to determine if the Lambda Expression is 
valid.


Repository:
  rL LLVM

https://reviews.llvm.org/D28510

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Parse/Parser.h
  include/clang/Sema/Sema.h
  lib/Parse/ParseDecl.cpp
  lib/Parse/ParseExpr.cpp
  lib/Parse/ParseTemplate.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaExprMember.cpp
  lib/Sema/SemaLambda.cpp
  lib/Sema/SemaTemplate.cpp
  lib/Sema/SemaTemplateInstantiateDecl.cpp
  lib/Sema/TreeTransform.h
  test/SemaCXX/cxx1z-constexpr-lambdas.cpp

Index: test/SemaCXX/cxx1z-constexpr-lambdas.cpp
===
--- test/SemaCXX/cxx1z-constexpr-lambdas.cpp
+++ test/SemaCXX/cxx1z-constexpr-lambdas.cpp
@@ -157,6 +157,38 @@
 
 } // end ns1_simple_lambda
 
+namespace test_forbidden_lambda_expressions {
+
+template struct X { }; //expected-error{{lambda expression may not appear}}
+X<[]{return 10; }()> x; //expected-error{{lambda expression may not appear}}
+void f(int arr[([] { return 5; }())]); //expected-error{{lambda expression may not appear}}
+// FIXME: Should this be ok?
+auto L = [](int arr[([] { return 5; }())]) { }; // OK
+
+// These should be allowed:
+struct A {
+  int : ([] { return 5; }());
+};
+
+int arr[([] { return 5; }())];
+enum { E = [] { return 5; }() };
+static_assert([]{return 5; }() == 5);
+int *ip = new int[([] { return 5; })()];
+
+int test_case(int x) {
+  switch(x) {
+case [] { return 5; }(): //OK
+  break;
+case [] (auto a) { return a; }(6): //expected-note{{previous}}
+  break;
+case 6:  //expected-error{{duplicate}}
+  break;
+  }
+  return x;
+}
+
+} // end ns forbidden_lambda_expressions
+
 namespace ns1_unimplemented {
 namespace ns1_captures {
 constexpr auto f(int i) {
Index: lib/Sema/TreeTransform.h
===
--- lib/Sema/TreeTransform.h
+++ lib/Sema/TreeTransform.h
@@ -3781,7 +3781,8 @@
   case TemplateArgument::Expression: {
 // Template argument expressions are constant expressions.
 EnterExpressionEvaluationContext Unevaluated(
-getSema(), Uneval ? Sema::Unevaluated : Sema::ConstantEvaluated);
+getSema(),
+Uneval ? Sema::Unevaluated : Sema::ConstantEvaluatedInTemplateArgument);
 
 Expr *InputExpr = Input.getSourceExpression();
 if (!InputExpr) InputExpr = Input.getArgument().getAsExpr();
Index: lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -2236,8 +2236,8 @@
 Param->setInvalidDecl();
 
   if (D->hasDefaultArgument() && !D->defaultArgumentWasInherited()) {
-EnterExpressionEvaluationContext ConstantEvaluated(SemaRef,
-   Sema::ConstantEvaluated);
+EnterExpressionEvaluationContext ConstantEvaluated(
+SemaRef, Sema::ConstantEvaluatedInTemplateArgument);
 ExprResult Value = SemaRef.SubstExpr(D->getDefaultArgument(), TemplateArgs);
 if (!Value.isInvalid())
   Param->setDefaultArgument(Value.get());
Index: lib/Sema/SemaTemplate.cpp
===
--- lib/Sema/SemaTemplate.cpp
+++ lib/Sema/SemaTemplate.cpp
@@ -3520,8 +3520,8 @@
   for (unsigned i = 0, e = Param->getDepth(); i != e; ++i)
 TemplateArgLists.addOuterTemplateArguments(None);
 
-  EnterExpressionEvaluationContext ConstantEvaluated(SemaRef,
- Sema::ConstantEvaluated);
+  EnterExpressionEvaluationContext ConstantEvaluated(
+  SemaRef, Sema::ConstantEvaluatedInTemplateArgument);
   return SemaRef.SubstExpr(Param->getDefaultArgument(), TemplateArgLists);
 }
 
@@ -5152,8 +5152,8 @@
 
   // The initialization of the parameter from the argument is
   // a constant-evaluated context.
-  EnterExpressionEvaluationContext ConstantEvaluated(*this,
- Sema::ConstantEvaluated);
+  EnterExpressionEvaluationContext ConstantEvaluated(
+  *this, Sema::ConstantEvaluatedInTemplateArgument);
 
   if (getLangOpts().CPlusPlus1z) {
 // C++1z [temp.arg.nontype]p1:
Index: lib/Sema/SemaLambda.cpp
===
--- lib/Sema/SemaLambda.cpp
+++ lib/Sema/SemaLambda.cpp
@@ -1591,11 +1591,12 @@
 //   evaluation of e, following the rules of the abstract machine, would
 /

[PATCH] D28510: Reinstate CWG1607 restrictions on lambdas appearing inside certain constant-expressions

[Faisal Vali via Phabricator via cfe-commits]

faisalv added a comment.

In https://reviews.llvm.org/D28510#653794, @rsmith wrote:

> I don't think it's possible to check this in the way you're doing so here. In 
> general, there's no way to know whether a constant expression will be part of 
> a `typedef` declaration or function declaration until you've finished parsing 
> it (when you're parsing the decl-specifiers in a declaration you don't know 
> whether you're declaring a function or a variable, and the `typedef` keyword 
> might appear later).


I see.  The issue is that the current approach would forbid valid variable 
declarations such as:

void (*f)(int [([]{return 5;}())]) = 0;

... where lambdas can appear within the array declarators (I believe that's the 
only syntactic neighborhood that can cause this problem, right?).

> So I think you need a different approach here. How about tracking the set of 
> contained lambdas on the `Declarator` and `DeclSpec` objects, and diagnose 
> from `ActOnFunctionDeclarator` / `ActOnTypedefDeclarator` if the current 
> expression evaluation context contains any lambdas? (Maybe when entering an 
> expression evaluation context, pass an optional `SmallVectorImpl*` to 
> `Sema` to collect the lambdas contained within the expression.)

Yes - I can see something along these lines working well...

> There are some particularly "fun" cases to watch out for here:
> 
>   decltype([]{})
> a, // ok
> f(); // ill-formed
> 
> 
> ... that require us to track the lambdas in the `DeclSpec` separately from 
> the lambdas in the `Declarator`.

Well lambda's can't appear in unevaluated operands yet, so your example would 
be ill-formed?  If so, we don't have to worry about them showing up in 
decl-specifiers?
The only Declarator where they could be well formed is within an array 
declarator of a variable or a parameter of a function pointer variable (but no 
where else, i.e typedefs and function declarations), right?

Thanks!


Repository:
  rL LLVM

https://reviews.llvm.org/D28510



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

[PATCH] D28510: Reinstate CWG1607 restrictions on lambdas appearing inside certain constant-expressions

[Faisal Vali via Phabricator via cfe-commits]

faisalv updated this revision to Diff 85927.
faisalv added a comment.

I tried a different approach, not because I was convinced it was better, but 
because it seemed (at the time) a simpler tweak - I'm not sure it is robust 
enough - but would appreciate your thoughts on it.

The approach is predicated on the following:

- lambda expressions can only appear in types in a non-unevaluated context, 
within array brackets
- if we track within each parameter declarator, its parent function declarator, 
we can always get to the outer most declarator and if the outermost declarator 
has the form (ptr-op f)(...) it can not be a function declarator (but is a 
variable) - we can determine this without waiting for the recursion to complete 
(I set a special token on the declarator once we start parsing a parens 
declarator and the token is a ptr-operator)
- additionally we can check the context of the outermost declarator to ensure 
it is not within an alias-declaration or a template-argument

Based on the test cases, it seems to work well - but not sure if the test cases 
are exhaustive.
Also the patch needs some cleanup if we agree that this direction is worth 
pursuing (and not too fragile an approach).

Thanks!

Would appreciate feedback!


https://reviews.llvm.org/D28510

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Parse/Parser.h
  include/clang/Sema/DeclSpec.h
  include/clang/Sema/Sema.h
  lib/Parse/ParseDecl.cpp
  lib/Parse/ParseExpr.cpp
  lib/Parse/ParseTemplate.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaExprMember.cpp
  lib/Sema/SemaLambda.cpp
  lib/Sema/SemaTemplate.cpp
  lib/Sema/SemaTemplateInstantiateDecl.cpp
  lib/Sema/TreeTransform.h
  test/SemaCXX/cxx1z-constexpr-lambdas.cpp

Index: test/SemaCXX/cxx1z-constexpr-lambdas.cpp
===
--- test/SemaCXX/cxx1z-constexpr-lambdas.cpp
+++ test/SemaCXX/cxx1z-constexpr-lambdas.cpp
@@ -157,6 +157,56 @@
 
 } // end ns1_simple_lambda
 
+namespace test_forbidden_lambda_expressions {
+
+template struct X { }; //expected-error{{lambda expression may not appear}}
+X<[]{return 10; }()> x; //expected-error{{lambda expression may not appear}}
+void f(int arr[([] { return 5; }())]); //expected-error{{lambda expression may not appear}}
+// FIXME: Should this be ok?
+auto L = [](int arr[([] { return 5; }())]) { }; // OK
+
+// These should be allowed:
+struct A {
+  int : ([] { return 5; }());
+};
+
+int arr[([] { return 5; }())];
+enum { E = [] { return 5; }() };
+static_assert([]{return 5; }() == 5);
+int *ip = new int[([] { return 5; })()];
+
+int test_case(int x) {
+  switch(x) {
+case [] { return 5; }(): //OK
+  break;
+case [] (auto a) { return a; }(6): //expected-note{{previous}}
+  break;
+case 6:  //expected-error{{duplicate}}
+  break;
+  }
+  return x;
+}
+namespace ns2 {
+int (*f)(int a[([] { return 5; }())]);
+int fun(int a[([] { return 5; }())]);  //expected-error{{lambda expression}}
+int fun2(void (*)(int a[([] { return 5; }())])); //expected-error{{lambda expression}}
+int (*fun2p)(void f(int a[([] { return 5; }())]));
+
+int (*g(int))[([] { return 5; })()];  //expected-error{{lambda expression}}
+int *(fun3)(int a[([] { return 5; }())]); //expected-error{{lambda expression}}
+
+
+int *(&fun3r)(int a[([] { return 5; }())]) = ([] () -> auto& { int *fv(int *); return fv; })();
+
+int (*g2(int))[([] { return 5; })()]; //expected-error{{lambda expression}}
+
+int (*(*g3)(int))[([] { return 5; })()];
+template struct X { };
+X x; //expected-error{{lambda expression}}
+} //end ns2
+
+} // end ns forbidden_lambda_expressions
+
 namespace ns1_unimplemented {
 namespace ns1_captures {
 constexpr auto f(int i) {
Index: lib/Sema/TreeTransform.h
===
--- lib/Sema/TreeTransform.h
+++ lib/Sema/TreeTransform.h
@@ -3787,7 +3787,8 @@
   case TemplateArgument::Expression: {
 // Template argument expressions are constant expressions.
 EnterExpressionEvaluationContext Unevaluated(
-getSema(), Uneval ? Sema::Unevaluated : Sema::ConstantEvaluated);
+getSema(),
+Uneval ? Sema::Unevaluated : Sema::ConstantEvaluatedInTemplateArgument);
 
 Expr *InputExpr = Input.getSourceExpression();
 if (!InputExpr) InputExpr = Input.getArgument().getAsExpr();
Index: lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -2236,8 +2236,8 @@
 Param->setInvalidDecl();
 
   if (D->hasDefaultArgument() && !D->defaultArgumentWasInherited()) {
-EnterExpressionEvaluationContext ConstantEvaluated(SemaRef,
-   Sema::ConstantEvaluated);
+EnterExpressionEvaluationContext ConstantEvaluated(
+SemaRef, Sema::ConstantEvaluatedInTemplateArgument);
 ExprResult Value = SemaRef.SubstExpr(

[PATCH] D29748: [cxx1z-constexpr-lambda] Implement captures - thus completing implementation of constexpr lambdas.

[Faisal Vali via Phabricator via cfe-commits]

faisalv created this revision.
faisalv added a project: clang-c.
Herald added a subscriber: EricWF.

This patch attempts to enable evaluation of all forms of captures (however 
deeply nested) within constexpr lambdas.

Appreciate the feedback.

Thanks!


Repository:
  rL LLVM

https://reviews.llvm.org/D29748

Files:
  lib/AST/ExprConstant.cpp
  test/SemaCXX/cxx1z-constexpr-lambdas.cpp

Index: test/SemaCXX/cxx1z-constexpr-lambdas.cpp
===
--- test/SemaCXX/cxx1z-constexpr-lambdas.cpp
+++ test/SemaCXX/cxx1z-constexpr-lambdas.cpp
@@ -1,6 +1,6 @@
-// RUN: %clang_cc1 -std=c++1z -verify -fsyntax-only -fblocks %s
-// RUN: %clang_cc1 -std=c++1z -verify -fsyntax-only -fblocks -fdelayed-template-parsing %s 
-// RUN: %clang_cc1 -std=c++14 -verify -fsyntax-only -fblocks %s -DCPP14_AND_EARLIER
+// RUN: %clang_cc1 -std=c++1z -verify -fsyntax-only -fblocks %s -fcxx-exceptions
+// RUN: %clang_cc1 -std=c++1z -verify -fsyntax-only -fblocks -fdelayed-template-parsing %s -fcxx-exceptions
+// RUN: %clang_cc1 -std=c++14 -verify -fsyntax-only -fblocks %s -DCPP14_AND_EARLIER -fcxx-exceptions
 
 
 namespace test_lambda_is_literal {
@@ -157,18 +157,111 @@
 
 } // end ns1_simple_lambda
 
-namespace ns1_unimplemented {
-namespace ns1_captures {
+namespace test_captures_1 {
+namespace ns1 {
 constexpr auto f(int i) {
-  double d = 3.14;
-  auto L = [=](auto a) { //expected-note{{coming soon}}
-int Isz = i + d;
-return sizeof(i) + sizeof(a) + sizeof(d); 
+  struct S { int x; } s = { i * 2 };
+  auto L = [=](auto a) { 
+return i + s.x + a;
   };
   return L;
 }
-constexpr auto M = f(3);  //expected-error{{constant expression}} expected-note{{in call to}}
-} // end ns1_captures
+constexpr auto M = f(3);  
+
+static_assert(M(10) == 19);
+
+} // end test_captures_1::ns1
+
+namespace ns2 {
+
+constexpr auto foo(int n) {
+  auto L = [i = n] (auto N) mutable {
+if (!N(i)) throw "error";
+return [&i] {
+  return ++i;
+};
+  };
+  auto M = L([n](int p) { return p == n; });
+  M(); M();
+  L([n](int p) { return p == n + 2; });
+  
+  return L;
+}
+
+constexpr auto L = foo(3);
+
+} // end test_captures_1::ns2
+namespace ns3 {
+
+constexpr auto foo(int n) {
+  auto L = [i = n] (auto N) mutable {
+if (!N(i)) throw "error";
+return [&i] {
+  return [i]() mutable {
+return ++i;
+  };
+};
+  };
+  auto M = L([n](int p) { return p == n; });
+  M()(); M()();
+  L([n](int p) { return p == n; });
+  
+  return L;
+}
+
+constexpr auto L = foo(3);
+} // end test_captures_1::ns3
+
+namespace ns2_capture_this_byval {
+struct S {
+  int s;
+  constexpr S(int s) : s{s} { }
+  constexpr auto f(S o) {
+return [*this,o] (auto a) { return s + o.s + a.s; };
+  }
+};
+
+constexpr auto L = S{5}.f(S{10});
+static_assert(L(S{100}) == 115);
+} // end test_captures_1::ns2_capture_this_byval
+
+namespace ns2_capture_this_byref {
+
+struct S {
+  int s;
+  constexpr S(int s) : s{s} { }
+  constexpr auto f() const {
+return [this] { return s; };
+  }
+};
+
+constexpr S SObj{5};
+constexpr auto L = SObj.f();
+constexpr int I = L();
+static_assert(I == 5);
+
+} // end ns2_capture_this_byref
+
+} // end test_captures_1
+
+namespace test_capture_array {
+namespace ns1 {
+constexpr auto f(int I) {
+  int arr[] = { I, I *2, I * 3 };
+  auto L1 = [&] (auto a) { return arr[a]; };
+  int r = L1(2);
+  struct X { int x, y; };
+  return [=](auto a) { return X{arr[a],r}; };
+}
+constexpr auto L = f(3);
+static_assert(L(0).x == 3);
+static_assert(L(0).y == 9);
+static_assert(L(1).x == 6);
+static_assert(L(1).y == 9);
+} // end ns1
+
+} // end test_capture_array
+namespace ns1_unimplemented {
 } // end ns1_unimplemented 
 
 } // end ns test_lambda_is_cce
Index: lib/AST/ExprConstant.cpp
===
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -425,6 +425,9 @@
 /// Index - The call index of this call.
 unsigned Index;
 
+llvm::DenseMap LambdaCaptureFields;
+FieldDecl *LambdaThisCaptureField;
+
 CallStackFrame(EvalInfo &Info, SourceLocation CallLoc,
const FunctionDecl *Callee, const LValue *This,
APValue *Arguments);
@@ -2279,6 +2282,10 @@
   return true;
 }
 
+static bool handleLValueToRValueConversion(EvalInfo &Info, const Expr *Conv,
+   QualType Type, const LValue &LVal,
+   APValue &RVal);
+
 /// Try to evaluate the initializer for a variable declaration.
 ///
 /// \param Info   Information about the ongoing evaluation.
@@ -2290,6 +2297,7 @@
 static bool evaluateVarDeclInit(EvalInfo &Info, const Expr *E,
 const VarDecl *VD, CallStackFrame *Frame,
 APValue *&Result) {
+
   // If this is a parameter to an active constexpr function call, perform
   // argument substitution.
   if (const ParmVarD

[PATCH] D29748: [cxx1z-constexpr-lambda] Implement captures - thus completing implementation of constexpr lambdas.

[Faisal Vali via Phabricator via cfe-commits]

faisalv added inline comments.



Comment at: lib/AST/ExprConstant.cpp:5061
+  APValue RVal;
+  // FIXME: We need to make sure we're passing the right type that
+  // maintains cv-qualifiers.

I don't think we need this fixme - the type of the expression should be correct 
- all other const checks for mutability have already been performed, right?



Comment at: lib/AST/ExprConstant.cpp:5486
+  }
+  //Info.FFDiag(E);
+  return false;

I need to delete this comment...


Repository:
  rL LLVM

https://reviews.llvm.org/D29748



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

[PATCH] D35454: [c++2a] Add option -std=c++2a to enable support for potential/transitional C++2a features

[Faisal Vali via Phabricator via cfe-commits]

faisalv created this revision.
faisalv added a project: clang.

At the optimistic least, I'm hoping to get around to adding support for 
familiar template syntax to lambdas and vaopt to the preprocessor, under it.


Repository:
  rL LLVM

https://reviews.llvm.org/D35454

Files:
  include/clang/Basic/LangOptions.def
  include/clang/Frontend/LangStandard.h
  include/clang/Frontend/LangStandards.def
  lib/Frontend/CompilerInvocation.cpp
  lib/Frontend/InitPreprocessor.cpp
  lib/Sema/SemaDeclCXX.cpp
  lib/Sema/SemaExpr.cpp
  test/Driver/std.cpp
  test/Driver/unknown-std.cpp
  test/Preprocessor/init.c

Index: test/Preprocessor/init.c
===
--- test/Preprocessor/init.c
+++ test/Preprocessor/init.c
@@ -9,6 +9,15 @@
 // BLOCKS:#define __block __attribute__((__blocks__(byref)))
 //
 //
+// RUN: %clang_cc1 -x c++ -std=c++2a -E -dM < /dev/null | FileCheck -match-full-lines -check-prefix CXX2A %s
+//
+// CXX2A:#define __GNUG__ {{.*}}
+// CXX2A:#define __GXX_EXPERIMENTAL_CXX0X__ 1
+// CXX2A:#define __GXX_RTTI 1
+// CXX2A:#define __GXX_WEAK__ 1
+// CXX2A:#define __cplusplus 201707L
+// CXX2A:#define __private_extern__ extern
+//
 // RUN: %clang_cc1 -x c++ -std=c++1z -E -dM < /dev/null | FileCheck -match-full-lines -check-prefix CXX1Z %s
 //
 // CXX1Z:#define __GNUG__ {{.*}}
@@ -110,7 +119,14 @@
 // RUN: %clang_cc1 -ffreestanding -E -dM < /dev/null | FileCheck -match-full-lines -check-prefix FREESTANDING %s
 // FREESTANDING:#define __STDC_HOSTED__ 0
 //
+// RUN: %clang_cc1 -x c++ -std=gnu++2a -E -dM < /dev/null | FileCheck -match-full-lines -check-prefix GXX2A %s
 //
+// GXX2A:#define __GNUG__ {{.*}}
+// GXX2A:#define __GXX_WEAK__ 1
+// GXX2A:#define __cplusplus 201707L
+// GXX2A:#define __private_extern__ extern
+//
+//
 // RUN: %clang_cc1 -x c++ -std=gnu++1z -E -dM < /dev/null | FileCheck -match-full-lines -check-prefix GXX1Z %s
 //
 // GXX1Z:#define __GNUG__ {{.*}}
Index: test/Driver/unknown-std.cpp
===
--- test/Driver/unknown-std.cpp
+++ test/Driver/unknown-std.cpp
@@ -15,6 +15,8 @@
 // CHECK-NEXT: note: use 'gnu++14' for 'ISO C++ 2014 with amendments and GNU extensions' standard
 // CHECK-NEXT: note: use 'c++1z' for 'Working draft for ISO C++ 2017' standard
 // CHECK-NEXT: note: use 'gnu++1z' for 'Working draft for ISO C++ 2017 with GNU extensions' standard
+// CHECK-NEXT: note: use 'c++2a' for 'Working draft for ISO C++ 2020' standard
+// CHECK-NEXT: note: use 'gnu++2a' for 'Working draft for ISO C++ 2020 with GNU extensions' standard
 // CUDA-NEXT: note: use 'cuda' for 'NVIDIA CUDA(tm)' standard
 
 // Make sure that no other output is present.
Index: test/Driver/std.cpp
===
--- test/Driver/std.cpp
+++ test/Driver/std.cpp
@@ -9,7 +9,10 @@
 // RUN: not %clang -std=gnu++1y %s -fsyntax-only 2>&1 | FileCheck -check-prefix=GNUXX1Y %s
 // RUN: not %clang -std=c++1z %s -fsyntax-only 2>&1 | FileCheck -check-prefix=CXX1Z %s
 // RUN: not %clang -std=gnu++1z %s -fsyntax-only 2>&1 | FileCheck -check-prefix=GNUXX1Z %s
+// RUN: not %clang -std=c++2a %s -fsyntax-only 2>&1 | FileCheck -check-prefix=CXX2A %s
+// RUN: not %clang -std=gnu++2a %s -fsyntax-only 2>&1 | FileCheck -check-prefix=GNUXX2A %s
 
+
 void f(int n) {
   typeof(n)();
   decltype(n)();
@@ -38,3 +41,10 @@
 
 // GNUXX1Z-NOT: undeclared identifier 'typeof'
 // GNUXX1Z-NOT: undeclared identifier 'decltype'
+
+// CXX2A: undeclared identifier 'typeof'
+// CXX2A-NOT: undeclared identifier 'decltype'
+
+// GNUXX2A-NOT: undeclared identifier 'typeof'
+// GNUXX2A-NOT: undeclared identifier 'decltype'
+
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -4635,8 +4635,7 @@
   // We already type-checked the argument, so we know it works. 
   // Just mark all of the declarations in this potentially-evaluated expression
   // as being "referenced".
-  MarkDeclarationsReferencedInExpr(Param->getDefaultArg(),
-   /*SkipLocalVariables=*/true);
+  MarkDeclarationsReferencedInExpr(Param->getDefaultArg());
   return false;
 }
 
Index: lib/Sema/SemaDeclCXX.cpp
===
--- lib/Sema/SemaDeclCXX.cpp
+++ lib/Sema/SemaDeclCXX.cpp
@@ -101,9 +101,12 @@
   //   Local variables shall not be used in default argument
   //   expressions.
   if (VDecl->isLocalVarDecl())
-return S->Diag(DRE->getLocStart(),
-   diag::err_param_default_argument_references_local)
-  << VDecl->getDeclName() << DefaultArg->getSourceRange();
+if (!VDecl->isStaticLocal() &&
+!(VDecl->isUsableInConstantExpressions(VDecl->getASTContext()) &&
+  VDecl->checkInitIsICE()))
+  return S->Diag(DRE->getLocStart(),
+ diag::err_param_default_argume

[PATCH] D35454: [c++2a] Add option -std=c++2a to enable support for potential/transitional C++2a features

[Faisal Vali via Phabricator via cfe-commits]

faisalv updated this revision to Diff 106777.
faisalv added a comment.

Remove some erroneously included fragments (patches to Sema) of a future 
potential patch.


https://reviews.llvm.org/D35454

Files:
  include/clang/Basic/LangOptions.def
  include/clang/Frontend/LangStandard.h
  include/clang/Frontend/LangStandards.def
  lib/Frontend/CompilerInvocation.cpp
  lib/Frontend/InitPreprocessor.cpp
  test/Driver/std.cpp
  test/Driver/unknown-std.cpp
  test/Preprocessor/init.c

Index: test/Preprocessor/init.c
===
--- test/Preprocessor/init.c
+++ test/Preprocessor/init.c
@@ -9,6 +9,15 @@
 // BLOCKS:#define __block __attribute__((__blocks__(byref)))
 //
 //
+// RUN: %clang_cc1 -x c++ -std=c++2a -E -dM < /dev/null | FileCheck -match-full-lines -check-prefix CXX2A %s
+//
+// CXX2A:#define __GNUG__ {{.*}}
+// CXX2A:#define __GXX_EXPERIMENTAL_CXX0X__ 1
+// CXX2A:#define __GXX_RTTI 1
+// CXX2A:#define __GXX_WEAK__ 1
+// CXX2A:#define __cplusplus 201707L
+// CXX2A:#define __private_extern__ extern
+//
 // RUN: %clang_cc1 -x c++ -std=c++1z -E -dM < /dev/null | FileCheck -match-full-lines -check-prefix CXX1Z %s
 //
 // CXX1Z:#define __GNUG__ {{.*}}
@@ -110,7 +119,14 @@
 // RUN: %clang_cc1 -ffreestanding -E -dM < /dev/null | FileCheck -match-full-lines -check-prefix FREESTANDING %s
 // FREESTANDING:#define __STDC_HOSTED__ 0
 //
+// RUN: %clang_cc1 -x c++ -std=gnu++2a -E -dM < /dev/null | FileCheck -match-full-lines -check-prefix GXX2A %s
 //
+// GXX2A:#define __GNUG__ {{.*}}
+// GXX2A:#define __GXX_WEAK__ 1
+// GXX2A:#define __cplusplus 201707L
+// GXX2A:#define __private_extern__ extern
+//
+//
 // RUN: %clang_cc1 -x c++ -std=gnu++1z -E -dM < /dev/null | FileCheck -match-full-lines -check-prefix GXX1Z %s
 //
 // GXX1Z:#define __GNUG__ {{.*}}
Index: test/Driver/unknown-std.cpp
===
--- test/Driver/unknown-std.cpp
+++ test/Driver/unknown-std.cpp
@@ -15,6 +15,8 @@
 // CHECK-NEXT: note: use 'gnu++14' for 'ISO C++ 2014 with amendments and GNU extensions' standard
 // CHECK-NEXT: note: use 'c++1z' for 'Working draft for ISO C++ 2017' standard
 // CHECK-NEXT: note: use 'gnu++1z' for 'Working draft for ISO C++ 2017 with GNU extensions' standard
+// CHECK-NEXT: note: use 'c++2a' for 'Working draft for ISO C++ 2020' standard
+// CHECK-NEXT: note: use 'gnu++2a' for 'Working draft for ISO C++ 2020 with GNU extensions' standard
 // CUDA-NEXT: note: use 'cuda' for 'NVIDIA CUDA(tm)' standard
 
 // Make sure that no other output is present.
Index: test/Driver/std.cpp
===
--- test/Driver/std.cpp
+++ test/Driver/std.cpp
@@ -9,7 +9,10 @@
 // RUN: not %clang -std=gnu++1y %s -fsyntax-only 2>&1 | FileCheck -check-prefix=GNUXX1Y %s
 // RUN: not %clang -std=c++1z %s -fsyntax-only 2>&1 | FileCheck -check-prefix=CXX1Z %s
 // RUN: not %clang -std=gnu++1z %s -fsyntax-only 2>&1 | FileCheck -check-prefix=GNUXX1Z %s
+// RUN: not %clang -std=c++2a %s -fsyntax-only 2>&1 | FileCheck -check-prefix=CXX2A %s
+// RUN: not %clang -std=gnu++2a %s -fsyntax-only 2>&1 | FileCheck -check-prefix=GNUXX2A %s
 
+
 void f(int n) {
   typeof(n)();
   decltype(n)();
@@ -38,3 +41,10 @@
 
 // GNUXX1Z-NOT: undeclared identifier 'typeof'
 // GNUXX1Z-NOT: undeclared identifier 'decltype'
+
+// CXX2A: undeclared identifier 'typeof'
+// CXX2A-NOT: undeclared identifier 'decltype'
+
+// GNUXX2A-NOT: undeclared identifier 'typeof'
+// GNUXX2A-NOT: undeclared identifier 'decltype'
+
Index: lib/Frontend/InitPreprocessor.cpp
===
--- lib/Frontend/InitPreprocessor.cpp
+++ lib/Frontend/InitPreprocessor.cpp
@@ -374,10 +374,13 @@
 else if (!LangOpts.GNUMode && LangOpts.Digraphs)
   Builder.defineMacro("__STDC_VERSION__", "199409L");
   } else {
+// FIXME: Use correct value for C++20
+if (LangOpts.CPlusPlus2a)
+  Builder.defineMacro("__cplusplus", "201707L");
 // C++17 [cpp.predefined]p1:
 //   The name __cplusplus is defined to the value 201703L when compiling a
 //   C++ translation unit.
-if (LangOpts.CPlusPlus1z)
+else if (LangOpts.CPlusPlus1z)
   Builder.defineMacro("__cplusplus", "201703L");
 // C++1y [cpp.predefined]p1:
 //   The name __cplusplus is defined to the value 201402L when compiling a
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -1683,6 +1683,7 @@
   Opts.CPlusPlus11 = Std.isCPlusPlus11();
   Opts.CPlusPlus14 = Std.isCPlusPlus14();
   Opts.CPlusPlus1z = Std.isCPlusPlus1z();
+  Opts.CPlusPlus2a = Std.isCPlusPlus2a();
   Opts.Digraphs = Std.hasDigraphs();
   Opts.GNUMode = Std.isGNUMode();
   Opts.GNUInline = !Opts.C99 && !Opts.CPlusPlus;
Index: include/clang/Frontend/LangStandards.def
===

[PATCH] D35454: [c++2a] Add option -std=c++2a to enable support for potential/transitional C++2a features

[Faisal Vali via Phabricator via cfe-commits]

faisalv added a comment.

Hmm - good point - not sure about the best way to confirm that...

I'm fairly sure Jason said something during a discussion with me (about the 
additional template syntax to lambdas) in which he referred to gcc as already 
having a "C++2a feature" - and so i just inferred that that's what they were 
calling it ...


https://reviews.llvm.org/D35454



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

[PATCH] D35454: [c++2a] Add option -std=c++2a to enable support for potential/transitional C++2a features

[Faisal Vali via Phabricator via cfe-commits]

faisalv closed this revision.
faisalv marked an inline comment as done.
faisalv added a comment.

Committed here https://reviews.llvm.org/rL308118

Thanks!


https://reviews.llvm.org/D35454



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

[PATCH] D35782: [C++2a][Preprocessor] Implement p0306 __VA_OPT__ (Comma omission and comma deletion)

[Faisal Vali via Phabricator via cfe-commits]

faisalv created this revision.
faisalv added a project: clang.

This patch implements an extension to the preprocessor: __VA_OPT__ which 
expands into its contents if variadic arguments are supplied, or behaves as an 
empty token if none.

- currently it is only enabled for C++2a (we could always enable this for 
various other dialects with the appropriate extension or compatibility warnings)

Given:

  #define F(a,...) a #__VA_OPT__(a ## a)  a ## __VA_OPT__(__VA_ARGS__)

A few technicalities (most which were clarified through private correspondence 
between rsmith, hubert and thomas) are worth mentioning:

- the call F(,)  Does not supply any tokens for the variadic arguments and 
hence __VA_OPT__ behaves as a placeholder
- when expanding VA_OPT (for e.g. F(,1) token pasting occurs eagerly within its 
contents
- a hash or a hashhash prior to __VA_OPT__ does not inhibit expansion of 
arguments if they are the first token within VA_OPT.
- essentially, when a variadic argument is supplied, argument substitution 
occurs within the contents as does stringification and concatenation - and this 
is substituted, potentially strinigified and concatenated content (but not 
rescanned) is inserted into the macro expansions token stream just prior to the 
entire stream being rescanned and concatenated.

See wg21.link/P0306 for further details.
See the example files for more involved examples.

Would greatly appreciate timely feedback =)


Repository:
  rL LLVM

https://reviews.llvm.org/D35782

Files:
  include/clang/Basic/DiagnosticLexKinds.td
  include/clang/Lex/Preprocessor.h
  include/clang/Lex/TokenLexer.h
  include/clang/Lex/VariadicMacroSupport.h
  lib/Lex/PPDirectives.cpp
  lib/Lex/Preprocessor.cpp
  lib/Lex/TokenLexer.cpp
  test/Preprocessor/macro_vaopt_check.cpp
  test/Preprocessor/macro_vaopt_expand.cpp

Index: test/Preprocessor/macro_vaopt_expand.cpp
===
--- test/Preprocessor/macro_vaopt_expand.cpp
+++ test/Preprocessor/macro_vaopt_expand.cpp
@@ -0,0 +1,127 @@
+// RUN: %clang_cc1 -E %s -pedantic -std=c++2a | FileCheck -strict-whitespace %s
+
+#define LPAREN ( 
+#define RPAREN ) 
+
+#define A0 expandedA0
+#define A1  expandedA1 A0
+#define A2  expandedA2 A1
+#define A3  expandedA3 A2
+
+#define A() B LPAREN )
+#define B() C LPAREN )
+#define C() D LPAREN )
+
+
+#define F(x, y) x + y 
+#define ELLIP_FUNC(...) __VA_OPT__(__VA_ARGS__)
+
+1: ELLIP_FUNC(F, LPAREN, 'a', 'b', RPAREN); 
+2: ELLIP_FUNC(F LPAREN 'a', 'b' RPAREN); 
+#undef F
+#undef ELLIP_FUNC
+
+// CHECK: 1: F, (, 'a', 'b', );
+// CHECK: 2: 'a' + 'b';
+
+#define F(...) f(0 __VA_OPT__(,) __VA_ARGS__)
+3: F(a, b, c) // replaced by f(0, a, b, c) 
+4: F() // replaced by f(0)
+
+// CHECK: 3: f(0 , a, b, c) 
+// CHECK: 4: f(0 )
+#undef F
+
+#define G(X, ...) f(0, X __VA_OPT__(,) __VA_ARGS__)
+
+5: G(a, b, c) // replaced by f(0, a , b, c) 
+6: G(a) // replaced by f(0, a) 
+7: G(a,) // replaced by f(0, a) 
+7.1: G(a,,)
+
+
+// CHECK: 5: f(0, a , b, c) 
+// CHECK: 6: f(0, a ) 
+// CHECK: 7: f(0, a ) 
+// CHECK: 7.1: f(0, a , ,)
+#undef G 
+
+#define HT_B() TONG
+
+#define F(x, ...) HT_ ## __VA_OPT__(x x A()  #x)
+
+8: F(1)
+9: F(A(),1)
+
+// CHECK: 8: HT_
+// CHECK: 9: TONG C ( ) B ( ) "A()"
+#undef HT_B
+#undef F
+
+#define F(a,...) #__VA_OPT__(A1 a)
+
+10: F(A())
+11: F(A1 A(), 1)
+// CHECK: 10: ""
+// CHECK: 11: "A1 expandedA1 expandedA0 B ( )"
+#undef F
+
+
+#define F(a,...) a ## __VA_OPT__(A1 a) ## __VA_ARGS__ ## a
+12.0: F()
+12: F(,)
+13: F(B,)
+// CHECK: 12.0: 
+// CHECK: 12: 
+// CHECK: 13: BB 
+#undef F
+
+#define F(...) #__VA_OPT__()  X ## __VA_OPT__()  #__VA_OPT__()
+
+14: F()
+15: F(1)
+
+// CHECK: 14: "" X ""
+// CHECK: 15: "" X ""
+
+#undef F
+
+#define SDEF(sname, ...) S sname __VA_OPT__(= { __VA_ARGS__ })
+
+16: SDEF(foo); // replaced by S foo; 
+17: SDEF(bar, 1, 2); // replaced by S bar = { 1, 2 }; 
+
+// CHECK: 16: S foo ;
+// CHECK: 17: S bar = { 1, 2 }; 
+#undef SDEF
+
+#define F(a,...) A() #__VA_OPT__(A3 __VA_ARGS__ a ## __VA_ARGS__ ## a ## C A3) A()
+
+18: F()
+19: F(,)
+20: F(,A3)
+21: F(A3, A(),A0)
+
+
+// CHECK: 18: B ( ) "" B ( ) 
+// CHECK: 19: B ( ) "" B ( ) 
+// CHECK: 20: B ( ) "A3 expandedA3 expandedA2 expandedA1 expandedA0 A3C A3" B ( )
+// CHECK: 21: B ( ) "A3 B ( ),expandedA0 A3A(),A0A3C A3" B ( )
+
+#undef F
+
+#define F(a,...) A() #__VA_OPT__(A3 __VA_ARGS__ a ## __VA_ARGS__ ## a ## C A3) a __VA_OPT__(A0 __VA_ARGS__ a ## __VA_ARGS__ ## a ## C A0) A()
+
+22: F()
+23: F(,)
+24: F(,A0)
+25: F(A0, A(),A0)
+
+
+// CHECK: 22: B ( ) "" B ( ) 
+// CHECK: 23: B ( ) "" B ( ) 
+// CHECK: 24: B ( ) "A3 expandedA0 A0C A3" expandedA0 expandedA0 A0C expandedA0 B ( )
+// CHECK: 25: B ( ) "A3 B ( ),expandedA0 A0A(),A0A0C A3" expandedA0 expandedA0 C ( ),expandedA0 A0A(),A0A0C expandedA0 B ( )
+
+#undef F
+
Index: test/Preprocessor/macro_vaopt_check.cpp
===
--- test/Preprocessor/macro_vaopt_check.cpp
+++ test/

[PATCH] D29748: [cxx1z-constexpr-lambda] Implement captures - thus completing implementation of constexpr lambdas.

[Faisal Vali via Phabricator via cfe-commits]

faisalv updated this revision to Diff 87943.
faisalv marked 6 inline comments as done.
faisalv added a comment.

Incorporated Richard's feedback and added comments.


https://reviews.llvm.org/D29748

Files:
  lib/AST/ExprConstant.cpp
  test/SemaCXX/cxx1z-constexpr-lambdas.cpp

Index: test/SemaCXX/cxx1z-constexpr-lambdas.cpp
===
--- test/SemaCXX/cxx1z-constexpr-lambdas.cpp
+++ test/SemaCXX/cxx1z-constexpr-lambdas.cpp
@@ -1,6 +1,6 @@
-// RUN: %clang_cc1 -std=c++1z -verify -fsyntax-only -fblocks %s
-// RUN: %clang_cc1 -std=c++1z -verify -fsyntax-only -fblocks -fdelayed-template-parsing %s 
-// RUN: %clang_cc1 -std=c++14 -verify -fsyntax-only -fblocks %s -DCPP14_AND_EARLIER
+// RUN: %clang_cc1 -std=c++1z -verify -fsyntax-only -fblocks %s -fcxx-exceptions
+// RUN: %clang_cc1 -std=c++1z -verify -fsyntax-only -fblocks -fdelayed-template-parsing %s -fcxx-exceptions
+// RUN: %clang_cc1 -std=c++14 -verify -fsyntax-only -fblocks %s -DCPP14_AND_EARLIER -fcxx-exceptions
 
 
 namespace test_lambda_is_literal {
@@ -157,18 +157,111 @@
 
 } // end ns1_simple_lambda
 
-namespace ns1_unimplemented {
-namespace ns1_captures {
+namespace test_captures_1 {
+namespace ns1 {
 constexpr auto f(int i) {
-  double d = 3.14;
-  auto L = [=](auto a) { //expected-note{{coming soon}}
-int Isz = i + d;
-return sizeof(i) + sizeof(a) + sizeof(d); 
+  struct S { int x; } s = { i * 2 };
+  auto L = [=](auto a) { 
+return i + s.x + a;
   };
   return L;
 }
-constexpr auto M = f(3);  //expected-error{{constant expression}} expected-note{{in call to}}
-} // end ns1_captures
+constexpr auto M = f(3);  
+
+static_assert(M(10) == 19);
+
+} // end test_captures_1::ns1
+
+namespace ns2 {
+
+constexpr auto foo(int n) {
+  auto L = [i = n] (auto N) mutable {
+if (!N(i)) throw "error";
+return [&i] {
+  return ++i;
+};
+  };
+  auto M = L([n](int p) { return p == n; });
+  M(); M();
+  L([n](int p) { return p == n + 2; });
+  
+  return L;
+}
+
+constexpr auto L = foo(3);
+
+} // end test_captures_1::ns2
+namespace ns3 {
+
+constexpr auto foo(int n) {
+  auto L = [i = n] (auto N) mutable {
+if (!N(i)) throw "error";
+return [&i] {
+  return [i]() mutable {
+return ++i;
+  };
+};
+  };
+  auto M = L([n](int p) { return p == n; });
+  M()(); M()();
+  L([n](int p) { return p == n; });
+  
+  return L;
+}
+
+constexpr auto L = foo(3);
+} // end test_captures_1::ns3
+
+namespace ns2_capture_this_byval {
+struct S {
+  int s;
+  constexpr S(int s) : s{s} { }
+  constexpr auto f(S o) {
+return [*this,o] (auto a) { return s + o.s + a.s; };
+  }
+};
+
+constexpr auto L = S{5}.f(S{10});
+static_assert(L(S{100}) == 115);
+} // end test_captures_1::ns2_capture_this_byval
+
+namespace ns2_capture_this_byref {
+
+struct S {
+  int s;
+  constexpr S(int s) : s{s} { }
+  constexpr auto f() const {
+return [this] { return s; };
+  }
+};
+
+constexpr S SObj{5};
+constexpr auto L = SObj.f();
+constexpr int I = L();
+static_assert(I == 5);
+
+} // end ns2_capture_this_byref
+
+} // end test_captures_1
+
+namespace test_capture_array {
+namespace ns1 {
+constexpr auto f(int I) {
+  int arr[] = { I, I *2, I * 3 };
+  auto L1 = [&] (auto a) { return arr[a]; };
+  int r = L1(2);
+  struct X { int x, y; };
+  return [=](auto a) { return X{arr[a],r}; };
+}
+constexpr auto L = f(3);
+static_assert(L(0).x == 3);
+static_assert(L(0).y == 9);
+static_assert(L(1).x == 6);
+static_assert(L(1).y == 9);
+} // end ns1
+
+} // end test_capture_array
+namespace ns1_unimplemented {
 } // end ns1_unimplemented 
 
 } // end ns test_lambda_is_cce
Index: lib/AST/ExprConstant.cpp
===
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -425,6 +425,9 @@
 /// Index - The call index of this call.
 unsigned Index;
 
+llvm::DenseMap LambdaCaptureFields;
+FieldDecl *LambdaThisCaptureField;
+
 CallStackFrame(EvalInfo &Info, SourceLocation CallLoc,
const FunctionDecl *Callee, const LValue *This,
APValue *Arguments);
@@ -2279,6 +2282,10 @@
   return true;
 }
 
+static bool handleLValueToRValueConversion(EvalInfo &Info, const Expr *Conv,
+   QualType Type, const LValue &LVal,
+   APValue &RVal);
+
 /// Try to evaluate the initializer for a variable declaration.
 ///
 /// \param Info   Information about the ongoing evaluation.
@@ -2290,6 +2297,7 @@
 static bool evaluateVarDeclInit(EvalInfo &Info, const Expr *E,
 const VarDecl *VD, CallStackFrame *Frame,
 APValue *&Result) {
+
   // If this is a parameter to an active constexpr function call, perform
   // argument substitution.
   if (const ParmVarDecl *PVD = dyn_cast(VD)) {
@@ -4180,6 +4188,10 @@
   return false;
 This->moveInto(Result);
 return

[PATCH] D29748: [cxx1z-constexpr-lambda] Implement captures - thus completing implementation of constexpr lambdas.

[Faisal Vali via Phabricator via cfe-commits]

faisalv added inline comments.



Comment at: lib/AST/ExprConstant.cpp:5061
+  APValue RVal;
+  // FIXME: We need to make sure we're passing the right type that
+  // maintains cv-qualifiers.

rsmith wrote:
> faisalv wrote:
> > I don't think we need this fixme - the type of the expression should be 
> > correct - all other const checks for mutability have already been 
> > performed, right?
> When using `handleLValueToRValueConversion` to obtain the lvalue denoted by a 
> reference, the type you pass should be the reference type itself 
> (`FD->getType()`). This approach will give the wrong answer when using a 
> captured volatile object:
> ```
> void f() {
>   volatile int n;
>   constexpr volatile int *p = [&]{ return &n; }(); // should be accepted
> }
> ```
OK - but how is the address of a local variable a constant expression?


https://reviews.llvm.org/D29748



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

[PATCH] D31588: Fix PR25627: Certain constant local variables must be usable as template arguments (without being odr-used)

[Faisal Vali via Phabricator via cfe-commits]

faisalv updated this revision to Diff 96858.
faisalv marked 3 inline comments as done.
faisalv added a comment.

Updated the patch following Richard's feedback:

- teach ParseConstantExpression to create its own ExpressionEvaluationContext 
only if asked to.


https://reviews.llvm.org/D31588

Files:
  include/clang/Parse/Parser.h
  lib/Parse/ParseExpr.cpp
  lib/Parse/ParseTemplate.cpp
  test/SemaCXX/lambda-expressions.cpp
  test/SemaCXX/local-classes.cpp

Index: test/SemaCXX/local-classes.cpp
===
--- test/SemaCXX/local-classes.cpp
+++ test/SemaCXX/local-classes.cpp
@@ -40,3 +40,15 @@
 };
   }
 }
+
+namespace PR25627_dont_odr_use_local_consts {
+  template struct X {};
+  
+  void foo() {
+const int N = 10;
+  
+struct Local {
+  void f(X) {} // OK
+}; 
+  }
+}
Index: test/SemaCXX/lambda-expressions.cpp
===
--- test/SemaCXX/lambda-expressions.cpp
+++ test/SemaCXX/lambda-expressions.cpp
@@ -573,3 +573,13 @@
   auto s1 = S1{[name=name]() {}}; // expected-error {{use of undeclared identifier 'name'; did you mean 'name1'?}}
 }
 }
+
+namespace PR25627_dont_odr_use_local_consts {
+  
+  template struct X {};
+  
+  void foo() {
+const int N = 10;
+(void) [] { X x; };
+  }
+}
Index: lib/Parse/ParseTemplate.cpp
===
--- lib/Parse/ParseTemplate.cpp
+++ lib/Parse/ParseTemplate.cpp
@@ -1198,42 +1198,48 @@
   //   expression is resolved to a type-id, regardless of the form of
   //   the corresponding template-parameter.
   //
-  // Therefore, we initially try to parse a type-id.  
+  // Therefore, we initially try to parse a type-id - and isCXXTypeId might look
+  // up and annotate an identifier as an id - expression during disambiguation,
+  // so enter the appropriate context for a constant expression template
+  // argument before trying to disambiguate.
+
+  EnterExpressionEvaluationContext EnterConstantEvaluated(
+  Actions, Sema::ExpressionEvaluationContext::ConstantEvaluated);
   if (isCXXTypeId(TypeIdAsTemplateArgument)) {
 SourceLocation Loc = Tok.getLocation();
-TypeResult TypeArg = ParseTypeName(/*Range=*/nullptr,
-   Declarator::TemplateTypeArgContext);
+TypeResult TypeArg =
+ParseTypeName(/*Range=*/nullptr, Declarator::TemplateTypeArgContext);
 if (TypeArg.isInvalid())
   return ParsedTemplateArgument();
-
+
 return ParsedTemplateArgument(ParsedTemplateArgument::Type,
-  TypeArg.get().getAsOpaquePtr(), 
-  Loc);
+  TypeArg.get().getAsOpaquePtr(), Loc);
   }
-  
   // Try to parse a template template argument.
   {
 TentativeParsingAction TPA(*this);
 
-ParsedTemplateArgument TemplateTemplateArgument
-  = ParseTemplateTemplateArgument();
+ParsedTemplateArgument TemplateTemplateArgument =
+ParseTemplateTemplateArgument();
 if (!TemplateTemplateArgument.isInvalid()) {
   TPA.Commit();
   return TemplateTemplateArgument;
 }
-
+
 // Revert this tentative parse to parse a non-type template argument.
 TPA.Revert();
   }
-  
-  // Parse a non-type template argument. 
+
+  // Parse a non-type template argument.
   SourceLocation Loc = Tok.getLocation();
-  ExprResult ExprArg = ParseConstantExpression(MaybeTypeCast);
+  ExprResult ExprArg = ParseConstantExpression(
+  MaybeTypeCast,
+  true /*Don't create another ConstantExpressionEvaluationContext*/);
   if (ExprArg.isInvalid() || !ExprArg.get())
 return ParsedTemplateArgument();
 
-  return ParsedTemplateArgument(ParsedTemplateArgument::NonType, 
-ExprArg.get(), Loc);
+  return ParsedTemplateArgument(ParsedTemplateArgument::NonType, ExprArg.get(),
+Loc);
 }
 
 /// \brief Determine whether the current tokens can only be parsed as a 
@@ -1274,16 +1280,16 @@
 /// template-argument-list ',' template-argument
 bool
 Parser::ParseTemplateArgumentList(TemplateArgList &TemplateArgs) {
-  // Template argument lists are constant-evaluation contexts.
-  EnterExpressionEvaluationContext EvalContext(
-  Actions, Sema::ExpressionEvaluationContext::ConstantEvaluated);
+  
   ColonProtectionRAIIObject ColonProtection(*this, false);
 
   do {
 ParsedTemplateArgument Arg = ParseTemplateArgument();
 SourceLocation EllipsisLoc;
-if (TryConsumeToken(tok::ellipsis, EllipsisLoc))
+if (TryConsumeToken(tok::ellipsis, EllipsisLoc)) {
+  // FIXME: Do we need need to enter a constant evaluation context here?
   Arg = Actions.ActOnPackExpansion(Arg, EllipsisLoc);
+}
 
 if (Arg.isInvalid()) {
   SkipUntil(tok::comma, tok::greater, StopAtSemi | StopBeforeMatch);
Index: lib/Parse/ParseExpr.cpp

[PATCH] D31588: Fix PR25627: Certain constant local variables must be usable as template arguments (without being odr-used)

[Faisal Vali via Phabricator via cfe-commits]

faisalv updated this revision to Diff 96861.
faisalv added a comment.

Fixed a regression test that should have passed without emitting error 
diagnostics - and now does.


https://reviews.llvm.org/D31588

Files:
  include/clang/Parse/Parser.h
  lib/Parse/ParseExpr.cpp
  lib/Parse/ParseTemplate.cpp
  test/SemaCXX/lambda-expressions.cpp
  test/SemaCXX/local-classes.cpp

Index: test/SemaCXX/local-classes.cpp
===
--- test/SemaCXX/local-classes.cpp
+++ test/SemaCXX/local-classes.cpp
@@ -40,3 +40,15 @@
 };
   }
 }
+
+namespace PR25627_dont_odr_use_local_consts {
+  template struct X {};
+  
+  void foo() {
+const int N = 10;
+  
+struct Local {
+  void f(X) {} // OK
+}; 
+  }
+}
Index: test/SemaCXX/lambda-expressions.cpp
===
--- test/SemaCXX/lambda-expressions.cpp
+++ test/SemaCXX/lambda-expressions.cpp
@@ -525,9 +525,9 @@
 class S {};
 
 void foo() {
-  const int num = 18;  // expected-note {{'num' declared here}}
+  const int num = 18; 
   auto outer = []() {
-auto inner = [](S &X) {};  // expected-error {{variable 'num' cannot be implicitly captured in a lambda with no capture-default specified}}
+auto inner = [](S &X) {};  
   };
 }
 }
@@ -573,3 +573,13 @@
   auto s1 = S1{[name=name]() {}}; // expected-error {{use of undeclared identifier 'name'; did you mean 'name1'?}}
 }
 }
+
+namespace PR25627_dont_odr_use_local_consts {
+  
+  template struct X {};
+  
+  void foo() {
+const int N = 10;
+(void) [] { X x; };
+  }
+}
Index: lib/Parse/ParseTemplate.cpp
===
--- lib/Parse/ParseTemplate.cpp
+++ lib/Parse/ParseTemplate.cpp
@@ -1198,42 +1198,49 @@
   //   expression is resolved to a type-id, regardless of the form of
   //   the corresponding template-parameter.
   //
-  // Therefore, we initially try to parse a type-id.  
+  // Therefore, we initially try to parse a type-id - and isCXXTypeId might look
+  // up and annotate an identifier as an id-expression during disambiguation,
+  // so enter the appropriate context for a constant expression template
+  // argument before trying to disambiguate.
+
+  EnterExpressionEvaluationContext EnterConstantEvaluated(
+  Actions, Sema::ExpressionEvaluationContext::ConstantEvaluated);
   if (isCXXTypeId(TypeIdAsTemplateArgument)) {
 SourceLocation Loc = Tok.getLocation();
-TypeResult TypeArg = ParseTypeName(/*Range=*/nullptr,
-   Declarator::TemplateTypeArgContext);
+TypeResult TypeArg =
+ParseTypeName(/*Range=*/nullptr, Declarator::TemplateTypeArgContext);
 if (TypeArg.isInvalid())
   return ParsedTemplateArgument();
-
+
 return ParsedTemplateArgument(ParsedTemplateArgument::Type,
-  TypeArg.get().getAsOpaquePtr(), 
-  Loc);
+  TypeArg.get().getAsOpaquePtr(), Loc);
   }
-  
   // Try to parse a template template argument.
   {
 TentativeParsingAction TPA(*this);
 
-ParsedTemplateArgument TemplateTemplateArgument
-  = ParseTemplateTemplateArgument();
+ParsedTemplateArgument TemplateTemplateArgument =
+ParseTemplateTemplateArgument();
 if (!TemplateTemplateArgument.isInvalid()) {
   TPA.Commit();
   return TemplateTemplateArgument;
 }
-
+
 // Revert this tentative parse to parse a non-type template argument.
 TPA.Revert();
   }
-  
-  // Parse a non-type template argument. 
+
+  // Parse a non-type template argument.
   SourceLocation Loc = Tok.getLocation();
-  ExprResult ExprArg = ParseConstantExpression(MaybeTypeCast);
+  ExprResult ExprArg = ParseConstantExpression(
+  MaybeTypeCast,
+  false /*don't create another ConstantExpressionEvaluationContext, 
+use ours instead*/);
   if (ExprArg.isInvalid() || !ExprArg.get())
 return ParsedTemplateArgument();
 
-  return ParsedTemplateArgument(ParsedTemplateArgument::NonType, 
-ExprArg.get(), Loc);
+  return ParsedTemplateArgument(ParsedTemplateArgument::NonType, ExprArg.get(),
+Loc);
 }
 
 /// \brief Determine whether the current tokens can only be parsed as a 
@@ -1274,16 +1281,16 @@
 /// template-argument-list ',' template-argument
 bool
 Parser::ParseTemplateArgumentList(TemplateArgList &TemplateArgs) {
-  // Template argument lists are constant-evaluation contexts.
-  EnterExpressionEvaluationContext EvalContext(
-  Actions, Sema::ExpressionEvaluationContext::ConstantEvaluated);
+  
   ColonProtectionRAIIObject ColonProtection(*this, false);
 
   do {
 ParsedTemplateArgument Arg = ParseTemplateArgument();
 SourceLocation EllipsisLoc;
-if (TryConsumeToken(tok::ellipsis, EllipsisLoc))
+if (TryConsumeToken(tok::ellipsis, EllipsisLoc)) {
+  // FIXME: Do we need n

[PATCH] D31588: Fix PR25627: Certain constant local variables must be usable as template arguments (without being odr-used)

[Faisal Vali via Phabricator via cfe-commits]

faisalv added a comment.

*ping*


https://reviews.llvm.org/D31588



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

[PATCH] D31588: Fix PR25627: Certain constant local variables must be usable as template arguments (without being odr-used)

[Faisal Vali via Phabricator via cfe-commits]

faisalv added a comment.

*ping*
*ping*


https://reviews.llvm.org/D31588



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

[PATCH] D31588: Fix PR25627: Certain constant local variables must be usable as template arguments (without being odr-used)

[Faisal Vali via Phabricator via cfe-commits]

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

Modified and committed as: 
http://llvm.org/viewvc/llvm-project?view=revision&revision=303492


https://reviews.llvm.org/D31588



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

[PATCH] D29748: [cxx1z-constexpr-lambda] Implement captures - thus completing implementation of constexpr lambdas.

[Faisal Vali via Phabricator via cfe-commits]

faisalv closed this revision.
faisalv added a comment.

Closed by commit https://reviews.llvm.org/rL295279


https://reviews.llvm.org/D29748



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

[PATCH] D31588: Fix PR25627: Certain constant local variables must be usable as template arguments (without being odr-used)

[Faisal Vali via Phabricator via cfe-commits]

faisalv closed this revision.
faisalv added a comment.

Committed here : https://reviews.llvm.org/rL303492


https://reviews.llvm.org/D31588



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

[PATCH] D31414: [NFC, Refactor] Modernize TemplateIdAnnotation using TrailingObjects

[Faisal Vali via Phabricator via cfe-commits]

faisalv added a comment.

*ping*


https://reviews.llvm.org/D31414



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

[PATCH] D31414: [NFC, Refactor] Modernize TemplateIdAnnotation using TrailingObjects

[Faisal Vali via Phabricator via cfe-commits]

faisalv updated this revision to Diff 99787.
faisalv added a comment.

Incorporated Aaron's feedback - thanks Aaron!


https://reviews.llvm.org/D31414

Files:
  include/clang/Basic/TemplateKinds.h
  include/clang/Sema/ParsedTemplate.h
  lib/Parse/ParseExprCXX.cpp
  lib/Parse/ParseTemplate.cpp

Index: lib/Parse/ParseTemplate.cpp
===
--- lib/Parse/ParseTemplate.cpp
+++ lib/Parse/ParseTemplate.cpp
@@ -1011,25 +1011,21 @@
 // Build a template-id annotation token that can be processed
 // later.
 Tok.setKind(tok::annot_template_id);
-TemplateIdAnnotation *TemplateId
-  = TemplateIdAnnotation::Allocate(TemplateArgs.size(), TemplateIds);
-TemplateId->TemplateNameLoc = TemplateNameLoc;
-if (TemplateName.getKind() == UnqualifiedId::IK_Identifier) {
-  TemplateId->Name = TemplateName.Identifier;
-  TemplateId->Operator = OO_None;
-} else {
-  TemplateId->Name = nullptr;
-  TemplateId->Operator = TemplateName.OperatorFunctionId.Operator;
-}
-TemplateId->SS = SS;
-TemplateId->TemplateKWLoc = TemplateKWLoc;
-TemplateId->Template = Template;
-TemplateId->Kind = TNK;
-TemplateId->LAngleLoc = LAngleLoc;
-TemplateId->RAngleLoc = RAngleLoc;
-ParsedTemplateArgument *Args = TemplateId->getTemplateArgs();
-for (unsigned Arg = 0, ArgEnd = TemplateArgs.size(); Arg != ArgEnd; ++Arg)
-  Args[Arg] = ParsedTemplateArgument(TemplateArgs[Arg]);
+
+IdentifierInfo *TemplateII =
+TemplateName.getKind() == UnqualifiedId::IK_Identifier
+? TemplateName.Identifier
+: nullptr;
+
+OverloadedOperatorKind OpKind =
+TemplateName.getKind() == UnqualifiedId::IK_Identifier
+? OO_None
+: TemplateName.OperatorFunctionId.Operator;
+
+TemplateIdAnnotation *TemplateId = TemplateIdAnnotation::Create(
+  SS, TemplateKWLoc, TemplateNameLoc, TemplateII, OpKind, Template, TNK,
+  LAngleLoc, RAngleLoc, TemplateArgs, TemplateIds);
+
 Tok.setAnnotationValue(TemplateId);
 if (TemplateKWLoc.isValid())
   Tok.setLocation(TemplateKWLoc);
Index: lib/Parse/ParseExprCXX.cpp
===
--- lib/Parse/ParseExprCXX.cpp
+++ lib/Parse/ParseExprCXX.cpp
@@ -2120,31 +2120,18 @@
   Id.getKind() == UnqualifiedId::IK_LiteralOperatorId) {
 // Form a parsed representation of the template-id to be stored in the
 // UnqualifiedId.
-TemplateIdAnnotation *TemplateId
-  = TemplateIdAnnotation::Allocate(TemplateArgs.size(), TemplateIds);
 
 // FIXME: Store name for literal operator too.
-if (Id.getKind() == UnqualifiedId::IK_Identifier) {
-  TemplateId->Name = Id.Identifier;
-  TemplateId->Operator = OO_None;
-  TemplateId->TemplateNameLoc = Id.StartLocation;
-} else {
-  TemplateId->Name = nullptr;
-  TemplateId->Operator = Id.OperatorFunctionId.Operator;
-  TemplateId->TemplateNameLoc = Id.StartLocation;
-}
+IdentifierInfo *TemplateII =
+Id.getKind() == UnqualifiedId::IK_Identifier ? Id.Identifier : nullptr;
+OverloadedOperatorKind OpKind = Id.getKind() == UnqualifiedId::IK_Identifier
+? OO_None
+: Id.OperatorFunctionId.Operator;
 
-TemplateId->SS = SS;
-TemplateId->TemplateKWLoc = TemplateKWLoc;
-TemplateId->Template = Template;
-TemplateId->Kind = TNK;
-TemplateId->LAngleLoc = LAngleLoc;
-TemplateId->RAngleLoc = RAngleLoc;
-ParsedTemplateArgument *Args = TemplateId->getTemplateArgs();
-for (unsigned Arg = 0, ArgEnd = TemplateArgs.size(); 
- Arg != ArgEnd; ++Arg)
-  Args[Arg] = TemplateArgs[Arg];
-
+TemplateIdAnnotation *TemplateId = TemplateIdAnnotation::Create(
+SS, TemplateKWLoc, Id.StartLocation, TemplateII, OpKind, Template, TNK,
+LAngleLoc, RAngleLoc, TemplateArgs, TemplateIds);
+
 Id.setTemplateId(TemplateId);
 return false;
   }
Index: include/clang/Sema/ParsedTemplate.h
===
--- include/clang/Sema/ParsedTemplate.h
+++ include/clang/Sema/ParsedTemplate.h
@@ -145,12 +145,15 @@
   /// expressions, or template names, and the source locations for important 
   /// tokens. All of the information about template arguments is allocated 
   /// directly after this structure.
-  struct TemplateIdAnnotation {
+  struct TemplateIdAnnotation final
+  : private llvm::TrailingObjects {
+friend TrailingObjects;
 /// \brief The nested-name-specifier that precedes the template name.
 CXXScopeSpec SS;
 
-/// TemplateKWLoc - The location of the template keyword within the
-/// source.
+/// TemplateKWLoc - The location of the template keyword.
+/// For e.g. typename T::template Y
 SourceLocation TemplateKWLoc;
 
 /// TemplateNameLoc - The location of the template

[PATCH] D31414: [NFC, Refactor] Modernize TemplateIdAnnotation using TrailingObjects

[Faisal Vali via Phabricator via cfe-commits]

faisalv marked 2 inline comments as done.
faisalv added a comment.

mark aaron's feedback as done.


https://reviews.llvm.org/D31414



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

[PATCH] D31414: [NFC, Refactor] Modernize TemplateIdAnnotation using TrailingObjects

[Faisal Vali via Phabricator via cfe-commits]

faisalv closed this revision.
faisalv added a comment.

Committed as https://reviews.llvm.org/rL303594


https://reviews.llvm.org/D31414



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