[PATCH] D133853: [AST] Add msvc-specific C++11 attributes

[S. B. Tam via Phabricator via cfe-commits]

cpplearner added a comment.

In D133853#3808674 , @aaron.ballman 
wrote:

> Now I'm wondering why the attribute exists at all. If it's functionally 
> equivalent to `constexpr` as a keyword, what are the use cases for the 
> attribute?

It appears that `[[msvc::constexpr]]` does not make a function `constexpr`, but 
if `[[msvc::constexpr]]` is used in a function definition //and// in a call to 
that function, then the annotated function call can be evaluated during 
constant evaluation: https://godbolt.org/z/3MPTsz6Yn

Apparently this is used to implement constexpr `std::construct_at`, which needs 
to call placement `operator new`, but the latter is not `constexpr`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133853

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

[PATCH] D114583: [clang-format] Adjust braced list detection

[S. B. Tam via Phabricator via cfe-commits]

cpplearner added a comment.

Yes, I need someone to commit it on my behalf. My name and email address: `Tan 
S. B. `


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

https://reviews.llvm.org/D114583

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

[PATCH] D114583: [clang-format] Adjust braced list detection

[S. B. Tam via Phabricator via cfe-commits]

cpplearner updated this revision to Diff 390890.

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

https://reviews.llvm.org/D114583

Files:
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -11768,6 +11768,27 @@
"  f(v);\n"
"}");
 
+  verifyFormat("void foo() {\n"
+   "  { // asdf\n"
+   "{ int a; }\n"
+   "  }\n"
+   "  {\n"
+   "{ int b; }\n"
+   "  }\n"
+   "}");
+  verifyFormat("namespace n {\n"
+   "void foo() {\n"
+   "  {\n"
+   "{\n"
+   "  statement();\n"
+   "  if (false) {\n"
+   "  }\n"
+   "}\n"
+   "  }\n"
+   "  {}\n"
+   "}\n"
+   "} // namespace n");
+
   // Long lists should be formatted in columns even if they are nested.
   verifyFormat(
   "vector x = function({1, 22, 333, , 5, 66, 777,\n"
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -578,17 +578,14 @@
   // BlockKind later if we parse a braced list (where all blocks
   // inside are by default braced lists), or when we explicitly detect
   // blocks (for example while parsing lambdas).
-  // FIXME: Some of these do not apply to JS, e.g. "} {" can never be a
-  // braced list in JS.
   ProbablyBracedList =
   (Style.Language == FormatStyle::LK_JavaScript &&
NextTok->isOneOf(Keywords.kw_of, Keywords.kw_in,
 Keywords.kw_as)) ||
   (Style.isCpp() && NextTok->is(tok::l_paren)) ||
   NextTok->isOneOf(tok::comma, tok::period, tok::colon,
-   tok::r_paren, tok::r_square, tok::l_brace,
-   tok::ellipsis) ||
-  (NextTok->is(tok::identifier) &&
+   tok::r_paren, tok::r_square, tok::ellipsis) ||
+  (NextTok->isOneOf(tok::l_brace, tok::identifier) &&
!PrevTok->isOneOf(tok::semi, tok::r_brace, tok::l_brace)) ||
   (NextTok->is(tok::semi) &&
(!ExpectClassBody || LBraceStack.size() != 1)) ||
@@ -2854,7 +2851,7 @@
   // class Foo implements {bar: number} { }
   nextToken();
   if (FormatTok->is(tok::l_brace)) {
-tryToParseBracedList();
+parseBracedList();
 continue;
   }
 }


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -11768,6 +11768,27 @@
"  f(v);\n"
"}");
 
+  verifyFormat("void foo() {\n"
+   "  { // asdf\n"
+   "{ int a; }\n"
+   "  }\n"
+   "  {\n"
+   "{ int b; }\n"
+   "  }\n"
+   "}");
+  verifyFormat("namespace n {\n"
+   "void foo() {\n"
+   "  {\n"
+   "{\n"
+   "  statement();\n"
+   "  if (false) {\n"
+   "  }\n"
+   "}\n"
+   "  }\n"
+   "  {}\n"
+   "}\n"
+   "} // namespace n");
+
   // Long lists should be formatted in columns even if they are nested.
   verifyFormat(
   "vector x = function({1, 22, 333, , 5, 66, 777,\n"
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -578,17 +578,14 @@
   // BlockKind later if we parse a braced list (where all blocks
   // inside are by default braced lists), or when we explicitly detect
   // blocks (for example while parsing lambdas).
-  // FIXME: Some of these do not apply to JS, e.g. "} {" can never be a
-  // braced list in JS.
   ProbablyBracedList =
   (Style.Language == FormatStyle::LK_JavaScript &&
NextTok->isOneOf(Keywords.kw_of, Keywords.kw_in,
 Keywords.kw_as)) ||
   (Style.isCpp() && NextTok->is(tok::l_paren)) ||
   NextTok->isOneOf(tok::comma, tok::period, tok::colon,
-   tok::r_paren, tok::r_square, tok::l_brace,
-   tok::ellipsis) ||
-  

[PATCH] D114583: [clang-format] Adjust braced list detection

[S. B. Tam via Phabricator via cfe-commits]

cpplearner updated this revision to Diff 389822.
cpplearner added a comment.

Fixed tests.

Since in JavaScript, the thing after `extends` must be an expression, and the 
thing after `implements` must be an object type, I decided to parse both as 
braced lists without going through `tryToParseBracedList`.


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

https://reviews.llvm.org/D114583

Files:
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -11731,6 +11731,15 @@
"  f(v);\n"
"}");
 
+  verifyFormat("void foo() {\n"
+   "  { // asdf\n"
+   "{ int a; }\n"
+   "  }\n"
+   "  {\n"
+   "{ int b; }\n"
+   "  }\n"
+   "}");
+
   // Long lists should be formatted in columns even if they are nested.
   verifyFormat(
   "vector x = function({1, 22, 333, , 5, 66, 777,\n"
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -513,17 +513,14 @@
   // BlockKind later if we parse a braced list (where all blocks
   // inside are by default braced lists), or when we explicitly detect
   // blocks (for example while parsing lambdas).
-  // FIXME: Some of these do not apply to JS, e.g. "} {" can never be a
-  // braced list in JS.
   ProbablyBracedList =
   (Style.Language == FormatStyle::LK_JavaScript &&
NextTok->isOneOf(Keywords.kw_of, Keywords.kw_in,
 Keywords.kw_as)) ||
   (Style.isCpp() && NextTok->is(tok::l_paren)) ||
   NextTok->isOneOf(tok::comma, tok::period, tok::colon,
-   tok::r_paren, tok::r_square, tok::l_brace,
-   tok::ellipsis) ||
-  (NextTok->is(tok::identifier) &&
+   tok::r_paren, tok::r_square, tok::ellipsis) ||
+  (NextTok->isOneOf(tok::l_brace, tok::identifier) &&
!PrevTok->isOneOf(tok::semi, tok::r_brace, tok::l_brace)) ||
   (NextTok->is(tok::semi) &&
(!ExpectClassBody || LBraceStack.size() != 1)) ||
@@ -2761,7 +2758,7 @@
   // class Foo implements {bar: number} { }
   nextToken();
   if (FormatTok->is(tok::l_brace)) {
-tryToParseBracedList();
+parseBracedList();
 continue;
   }
 }


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -11731,6 +11731,15 @@
"  f(v);\n"
"}");
 
+  verifyFormat("void foo() {\n"
+   "  { // asdf\n"
+   "{ int a; }\n"
+   "  }\n"
+   "  {\n"
+   "{ int b; }\n"
+   "  }\n"
+   "}");
+
   // Long lists should be formatted in columns even if they are nested.
   verifyFormat(
   "vector x = function({1, 22, 333, , 5, 66, 777,\n"
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -513,17 +513,14 @@
   // BlockKind later if we parse a braced list (where all blocks
   // inside are by default braced lists), or when we explicitly detect
   // blocks (for example while parsing lambdas).
-  // FIXME: Some of these do not apply to JS, e.g. "} {" can never be a
-  // braced list in JS.
   ProbablyBracedList =
   (Style.Language == FormatStyle::LK_JavaScript &&
NextTok->isOneOf(Keywords.kw_of, Keywords.kw_in,
 Keywords.kw_as)) ||
   (Style.isCpp() && NextTok->is(tok::l_paren)) ||
   NextTok->isOneOf(tok::comma, tok::period, tok::colon,
-   tok::r_paren, tok::r_square, tok::l_brace,
-   tok::ellipsis) ||
-  (NextTok->is(tok::identifier) &&
+   tok::r_paren, tok::r_square, tok::ellipsis) ||
+  (NextTok->isOneOf(tok::l_brace, tok::identifier) &&
!PrevTok->isOneOf(tok::semi, tok::r_brace, tok::l_brace)) ||
   (NextTok->is(tok::semi) &&
(!ExpectClassBody || LBraceStack.size() != 1)) ||
@@ -2761,7 +2758,7 @@
   // class Foo implements {bar: number} { }
   nextToken();
   if 

[PATCH] D65050: [SemaTemplate] Mark a function type as dependent when its parameter list contains pack expansion

[S. B. Tam via Phabricator via cfe-commits]

cpplearner updated this revision to Diff 265683.
cpplearner added a comment.

rebase & ping @rsmith


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

https://reviews.llvm.org/D65050

Files:
  clang/lib/AST/Type.cpp
  clang/test/SemaTemplate/alias-templates.cpp


Index: clang/test/SemaTemplate/alias-templates.cpp
===
--- clang/test/SemaTemplate/alias-templates.cpp
+++ clang/test/SemaTemplate/alias-templates.cpp
@@ -265,3 +265,34 @@
 int z = Bar(); // expected-error {{use of template template parameter 
'Bar' requires template arguments}}
   }
 }
+
+namespace PR42654 {
+  template struct function { };
+
+  template
+  struct thing {
+void f(function) { }
+  };
+
+  template
+  struct Environment
+  {
+template
+using Integer = int;
+
+using Function = function...)>;
+using MyTuple = thing...>;
+
+void run(Function func)
+{
+  MyTuple t;
+  t.f(func);
+}
+  };
+
+  void f()
+  {
+Environment env;
+env.run({});
+  }
+}
Index: clang/lib/AST/Type.cpp
===
--- clang/lib/AST/Type.cpp
+++ clang/lib/AST/Type.cpp
@@ -3110,6 +3110,11 @@
   for (unsigned i = 0; i != getNumParams(); ++i) {
 addDependence(params[i]->getDependence() &
   ~TypeDependence::VariablyModified);
+// A pack expansion with a non-dependent pattern affects the number of
+// parameters, thus we mark such function type as dependent, even though
+// this isn't listed in N4861 [temp.dep.type].
+if (params[i]->getAs())
+  addDependence(TypeDependence::Dependent);
 argSlot[i] = params[i];
   }
 


Index: clang/test/SemaTemplate/alias-templates.cpp
===
--- clang/test/SemaTemplate/alias-templates.cpp
+++ clang/test/SemaTemplate/alias-templates.cpp
@@ -265,3 +265,34 @@
 int z = Bar(); // expected-error {{use of template template parameter 'Bar' requires template arguments}}
   }
 }
+
+namespace PR42654 {
+  template struct function { };
+
+  template
+  struct thing {
+void f(function) { }
+  };
+
+  template
+  struct Environment
+  {
+template
+using Integer = int;
+
+using Function = function...)>;
+using MyTuple = thing...>;
+
+void run(Function func)
+{
+  MyTuple t;
+  t.f(func);
+}
+  };
+
+  void f()
+  {
+Environment env;
+env.run({});
+  }
+}
Index: clang/lib/AST/Type.cpp
===
--- clang/lib/AST/Type.cpp
+++ clang/lib/AST/Type.cpp
@@ -3110,6 +3110,11 @@
   for (unsigned i = 0; i != getNumParams(); ++i) {
 addDependence(params[i]->getDependence() &
   ~TypeDependence::VariablyModified);
+// A pack expansion with a non-dependent pattern affects the number of
+// parameters, thus we mark such function type as dependent, even though
+// this isn't listed in N4861 [temp.dep.type].
+if (params[i]->getAs())
+  addDependence(TypeDependence::Dependent);
 argSlot[i] = params[i];
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65050: [SemaTemplate] Mark a function type as dependent when its parameter list contains pack expansion

[S. B. Tam via Phabricator via cfe-commits]

cpplearner added a comment.

ping @rsmith


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

https://reviews.llvm.org/D65050



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

[PATCH] D65050: [SemaTemplate] Mark a function type as dependent when its parameter list contains pack expansion

[S. B. Tam via Phabricator via cfe-commits]

cpplearner added a comment.

In D65050#1608777 , @efriedma wrote:

> > this looks like it could be a Core Issue
>
> I think the issue is clang-specific. clang splits the standard notion of a 
> dependent type into two separate bits, for the sake of diagnostics: 
> `isDependentType()`, and `isInstantiationDependentType()`.  
> `isInstantiationDependentType()` reflects the actual standard definition of a 
> dependent type; `isDependentType()` is a type that can actually vary across 
> instantiations.


According to comment in `include/clang/AST/Type.h` 
(https://github.com/llvm/llvm-project/blob/llvmorg-8.0.1/clang/include/clang/AST/Type.h#L1426),
 `Dependent` reflects the standard definition, while `InstantiationDependent` 
reflects "whether this type somehow involves a template parameter, even if the 
resolution of the type does not depend on a template parameter" (e.g. 
`decltype(sizeof(T))`).

Therefore, I agree with Aaron that this could be a Core issue.


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

https://reviews.llvm.org/D65050



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

[PATCH] D65050: [SemaTemplate] Mark a function type as dependent when its parameter list contains pack expansion

[S. B. Tam via Phabricator via cfe-commits]

cpplearner added a comment.

ping @rsmith


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

https://reviews.llvm.org/D65050



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

[PATCH] D47419: [SemaDeclCXX] Allow inheriting constructor declaration that specify a cv-qualified type

[S. B. Tam via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL369196: [SemaDeclCXX] Allow inheriting constructor 
declaration to specify a cv… (authored by cpplearner, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D47419?vs=148744=215753#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D47419

Files:
  cfe/trunk/lib/Sema/SemaDeclCXX.cpp
  cfe/trunk/test/CXX/special/class.inhctor/elsewhere.cpp


Index: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
===
--- cfe/trunk/lib/Sema/SemaDeclCXX.cpp
+++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp
@@ -9997,7 +9997,8 @@
 QualType DesiredBase,
 bool ) {
   // Check whether the named type is a direct base class.
-  CanQualType CanonicalDesiredBase = 
DesiredBase->getCanonicalTypeUnqualified();
+  CanQualType CanonicalDesiredBase = DesiredBase->getCanonicalTypeUnqualified()
+.getUnqualifiedType();
   for (auto  : Derived->bases()) {
 CanQualType BaseType = Base.getType()->getCanonicalTypeUnqualified();
 if (CanonicalDesiredBase == BaseType)
Index: cfe/trunk/test/CXX/special/class.inhctor/elsewhere.cpp
===
--- cfe/trunk/test/CXX/special/class.inhctor/elsewhere.cpp
+++ cfe/trunk/test/CXX/special/class.inhctor/elsewhere.cpp
@@ -62,3 +62,4 @@
   G(int &) : G(0) {}
 };
 G g(123);
+G g2(123);


Index: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
===
--- cfe/trunk/lib/Sema/SemaDeclCXX.cpp
+++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp
@@ -9997,7 +9997,8 @@
 QualType DesiredBase,
 bool ) {
   // Check whether the named type is a direct base class.
-  CanQualType CanonicalDesiredBase = DesiredBase->getCanonicalTypeUnqualified();
+  CanQualType CanonicalDesiredBase = DesiredBase->getCanonicalTypeUnqualified()
+.getUnqualifiedType();
   for (auto  : Derived->bases()) {
 CanQualType BaseType = Base.getType()->getCanonicalTypeUnqualified();
 if (CanonicalDesiredBase == BaseType)
Index: cfe/trunk/test/CXX/special/class.inhctor/elsewhere.cpp
===
--- cfe/trunk/test/CXX/special/class.inhctor/elsewhere.cpp
+++ cfe/trunk/test/CXX/special/class.inhctor/elsewhere.cpp
@@ -62,3 +62,4 @@
   G(int &) : G(0) {}
 };
 G g(123);
+G g2(123);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47419: [SemaDeclCXX] Allow inheriting constructor declaration that specify a cv-qualified type

[S. B. Tam via Phabricator via cfe-commits]

cpplearner added a comment.
Herald added a project: clang.

ping


Repository:
  rC Clang

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

https://reviews.llvm.org/D47419



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

[PATCH] D65050: [SemaTemplate] Mark a function type as dependent when its parameter list contains pack expansion

[S. B. Tam via Phabricator via cfe-commits]

cpplearner added a comment.

ping


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

https://reviews.llvm.org/D65050



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

[PATCH] D65050: [SemaTemplate] Mark a function type as dependent when its parameter list contains pack expansion

[S. B. Tam via Phabricator via cfe-commits]

cpplearner added a comment.

In D65050#1606022 , @aaron.ballman 
wrote:

> The noexcept specifier is part of the type these days, is that also handled 
> properly?


I believe that it's properly handled in this section of 
`FunctionProtoType::FunctionProtoType`:

  // If this is a canonical type, and its exception specification is dependent,
  // then it's a dependent type. This only happens in C++17 onwards.
  if (isCanonicalUnqualified()) {
if (getExceptionSpecType() == EST_Dynamic ||
getExceptionSpecType() == EST_DependentNoexcept) {
  assert(hasDependentExceptionSpec() && "type should not be canonical");
  setDependent();
}
  } else if (getCanonicalTypeInternal()->isDependentType()) {
// Ask our canonical type whether our exception specification was dependent.
setDependent();
  }


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

https://reviews.llvm.org/D65050



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

[PATCH] D65050: [SemaTemplate] Mark a function type as dependent when its parameter list contains pack expansion

[S. B. Tam via Phabricator via cfe-commits]

cpplearner added a comment.

ping


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

https://reviews.llvm.org/D65050



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

[PATCH] D65050: [SemaTemplate] Mark a function type as dependent when its parameter list contains pack expansion

[S. B. Tam via Phabricator via cfe-commits]

cpplearner added a comment.

In D65050#1596514 , @efriedma wrote:

> Is this the only place where a compound type can contain a PackExpansionType?


Yes AFAIK. No other place can contain a list or a pack expansion. The closest 
thing is dynamic exception specification (which isn't part of the type), and it 
seems to be handled properly.

> The code could probably use a brief comment explaining why we need to check 
> this explicitly, even though it isn't listed in [temp.dep.type].

Done.


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

https://reviews.llvm.org/D65050



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

[PATCH] D65050: [SemaTemplate] Mark a function type as dependent when its parameter list contains pack expansion

[S. B. Tam via Phabricator via cfe-commits]

cpplearner updated this revision to Diff 211358.

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

https://reviews.llvm.org/D65050

Files:
  clang/lib/AST/Type.cpp
  clang/test/SemaTemplate/alias-templates.cpp


Index: clang/test/SemaTemplate/alias-templates.cpp
===
--- clang/test/SemaTemplate/alias-templates.cpp
+++ clang/test/SemaTemplate/alias-templates.cpp
@@ -267,3 +267,34 @@
 int z = Bar(); // expected-error {{use of template template parameter 
'Bar' requires template arguments}}
   }
 }
+
+namespace PR42654 {
+  template struct function { };
+
+  template
+  struct thing {
+void f(function) { }
+  };
+
+  template
+  struct Environment
+  {
+template
+using Integer = int;
+
+using Function = function...)>;
+using MyTuple = thing...>;
+
+void run(Function func)
+{
+  MyTuple t;
+  t.f(func);
+}
+  };
+
+  void f()
+  {
+Environment env;
+env.run({});
+  }
+}
Index: clang/lib/AST/Type.cpp
===
--- clang/lib/AST/Type.cpp
+++ clang/lib/AST/Type.cpp
@@ -2912,7 +2912,10 @@
   // Fill in the trailing argument array.
   auto *argSlot = getTrailingObjects();
   for (unsigned i = 0; i != getNumParams(); ++i) {
-if (params[i]->isDependentType())
+// A pack expansion with a non-dependent pattern still affects the number 
of
+// parameters, thus we mark such function type as dependent, even though
+// this isn't listed in N4820 [temp.dep.type].
+if (params[i]->isDependentType() || params[i]->getAs())
   setDependent();
 else if (params[i]->isInstantiationDependentType())
   setInstantiationDependent();


Index: clang/test/SemaTemplate/alias-templates.cpp
===
--- clang/test/SemaTemplate/alias-templates.cpp
+++ clang/test/SemaTemplate/alias-templates.cpp
@@ -267,3 +267,34 @@
 int z = Bar(); // expected-error {{use of template template parameter 'Bar' requires template arguments}}
   }
 }
+
+namespace PR42654 {
+  template struct function { };
+
+  template
+  struct thing {
+void f(function) { }
+  };
+
+  template
+  struct Environment
+  {
+template
+using Integer = int;
+
+using Function = function...)>;
+using MyTuple = thing...>;
+
+void run(Function func)
+{
+  MyTuple t;
+  t.f(func);
+}
+  };
+
+  void f()
+  {
+Environment env;
+env.run({});
+  }
+}
Index: clang/lib/AST/Type.cpp
===
--- clang/lib/AST/Type.cpp
+++ clang/lib/AST/Type.cpp
@@ -2912,7 +2912,10 @@
   // Fill in the trailing argument array.
   auto *argSlot = getTrailingObjects();
   for (unsigned i = 0; i != getNumParams(); ++i) {
-if (params[i]->isDependentType())
+// A pack expansion with a non-dependent pattern still affects the number of
+// parameters, thus we mark such function type as dependent, even though
+// this isn't listed in N4820 [temp.dep.type].
+if (params[i]->isDependentType() || params[i]->getAs())
   setDependent();
 else if (params[i]->isInstantiationDependentType())
   setInstantiationDependent();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65050: [SemaTemplate] Mark a function type as dependent when its parameter list contains pack expansion

[S. B. Tam via Phabricator via cfe-commits]

cpplearner created this revision.
cpplearner added reviewers: doug.gregor, eli.friedman, lvoufo, rsmith.
Herald added subscribers: cfe-commits, dexonsmith.
Herald added a project: clang.

Given `template using Int = int;`, the type `void(Int...)` should be 
treated as a dependent type, even though `Int` is not dependent.

This fixes https://bugs.llvm.org/show_bug.cgi?id=42654


Repository:
  rC Clang

https://reviews.llvm.org/D65050

Files:
  clang/lib/AST/Type.cpp
  clang/test/SemaTemplate/alias-templates.cpp


Index: clang/test/SemaTemplate/alias-templates.cpp
===
--- clang/test/SemaTemplate/alias-templates.cpp
+++ clang/test/SemaTemplate/alias-templates.cpp
@@ -267,3 +267,34 @@
 int z = Bar(); // expected-error {{use of template template parameter 
'Bar' requires template arguments}}
   }
 }
+
+namespace PR42654 {
+  template struct function { };
+
+  template
+  struct thing {
+void f(function) { }
+  };
+
+  template
+  struct Environment
+  {
+template
+using Integer = int;
+
+using Function = function...)>;
+using MyTuple = thing...>;
+
+void run(Function func)
+{
+  MyTuple t;
+  t.f(func);
+}
+  };
+
+  void f()
+  {
+Environment env;
+env.run({});
+  }
+}
Index: clang/lib/AST/Type.cpp
===
--- clang/lib/AST/Type.cpp
+++ clang/lib/AST/Type.cpp
@@ -2912,7 +2912,7 @@
   // Fill in the trailing argument array.
   auto *argSlot = getTrailingObjects();
   for (unsigned i = 0; i != getNumParams(); ++i) {
-if (params[i]->isDependentType())
+if (params[i]->isDependentType() || params[i]->getAs())
   setDependent();
 else if (params[i]->isInstantiationDependentType())
   setInstantiationDependent();


Index: clang/test/SemaTemplate/alias-templates.cpp
===
--- clang/test/SemaTemplate/alias-templates.cpp
+++ clang/test/SemaTemplate/alias-templates.cpp
@@ -267,3 +267,34 @@
 int z = Bar(); // expected-error {{use of template template parameter 'Bar' requires template arguments}}
   }
 }
+
+namespace PR42654 {
+  template struct function { };
+
+  template
+  struct thing {
+void f(function) { }
+  };
+
+  template
+  struct Environment
+  {
+template
+using Integer = int;
+
+using Function = function...)>;
+using MyTuple = thing...>;
+
+void run(Function func)
+{
+  MyTuple t;
+  t.f(func);
+}
+  };
+
+  void f()
+  {
+Environment env;
+env.run({});
+  }
+}
Index: clang/lib/AST/Type.cpp
===
--- clang/lib/AST/Type.cpp
+++ clang/lib/AST/Type.cpp
@@ -2912,7 +2912,7 @@
   // Fill in the trailing argument array.
   auto *argSlot = getTrailingObjects();
   for (unsigned i = 0; i != getNumParams(); ++i) {
-if (params[i]->isDependentType())
+if (params[i]->isDependentType() || params[i]->getAs())
   setDependent();
 else if (params[i]->isInstantiationDependentType())
   setInstantiationDependent();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61027: Fix crash on switch conditions of non-integer types in templates

[S. B. Tam via Phabricator via cfe-commits]

cpplearner added a comment.

(I am ensadc at bugzilla)

Does this cause regression in the following case?

  template
  void f() {
  switch (N) case 0:; // should be diagnosed
  switch (0) case N:; // should be diagnosed
  }

Currently clang diagnoses these `switch` statements even if the template is not 
instantiated, GCC does not.

AFAIK both compilers are correct under [temp.res]/8, but clang's current 
behavior seems more helpful.


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

https://reviews.llvm.org/D61027



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

[PATCH] D59467: [clang] Adding the Likely Attribute from C++2a to AST

[S. B. Tam via Phabricator via cfe-commits]

cpplearner added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:8166
+def err_multiple_likelihood : Error<
+  "there can only be one %0 attribue in any attribute list">;
+def err_mutuably_exclusive_likelihood : Error<

attribue => attribute



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:8172
+def note_previous_likelihood : Note<
+  "previously used %0 attribue">;
+

attribue => attribute



Comment at: clang/lib/Parse/ParseStmt.cpp:1329
+  } else if (ElseBranchAttr) {
+if (ThenBranchAttr->getSpelling()[0] == 'l')
+  WhichBranch = IfStmt::ElseBranch;

Should this use `ElseBranchAttr` instead of `ThenBranchAttr`?


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

https://reviews.llvm.org/D59467



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

[PATCH] D57032: [SemaCXX] Param diagnostic matches overload logic

[S. B. Tam via Phabricator via cfe-commits]

cpplearner added inline comments.



Comment at: lib/Sema/SemaDecl.cpp:5092
 // The parameter types are identical
-if (Context.hasSameType(DefParamTy, DeclParamTy))
+if (Context.hasSameType(DefParamTy.getUnqualifiedType(),
+DeclParamTy.getUnqualifiedType()))

I see this is what `Sema::FunctionParamTypesAreEqual` does, but maybe both 
places should use `hasSameUnqualifiedType`?


Repository:
  rC Clang

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

https://reviews.llvm.org/D57032



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

[PATCH] D55413: [ExprConstant] Handle compound assignment when LHS has integral type and RHS has floating point type

[S. B. Tam via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC349444: [ExprConstant] Handle compound assignment when LHS 
has integral type and RHS… (authored by cpplearner, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D55413?vs=178242=178597#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D55413

Files:
  lib/AST/ExprConstant.cpp
  test/SemaCXX/constant-expression-cxx1y.cpp


Index: test/SemaCXX/constant-expression-cxx1y.cpp
===
--- test/SemaCXX/constant-expression-cxx1y.cpp
+++ test/SemaCXX/constant-expression-cxx1y.cpp
@@ -356,6 +356,14 @@
 if (a != 13) return false;
 a &= 14;
 if (a != 12) return false;
+a += -1.2;
+if (a != 10) return false;
+a -= 3.1;
+if (a != 6) return false;
+a *= 2.2;
+if (a != 13) return false;
+if (&(a /= 1.5) != ) return false;
+if (a != 8) return false;
 return true;
   }
   static_assert(test_int(), "");
Index: lib/AST/ExprConstant.cpp
===
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -3439,19 +3439,31 @@
 if (!checkConst(SubobjType))
   return false;
 
-if (!SubobjType->isIntegerType() || !RHS.isInt()) {
+if (!SubobjType->isIntegerType()) {
   // We don't support compound assignment on integer-cast-to-pointer
   // values.
   Info.FFDiag(E);
   return false;
 }
 
-APSInt LHS = HandleIntToIntCast(Info, E, PromotedLHSType,
-SubobjType, Value);
-if (!handleIntIntBinOp(Info, E, LHS, Opcode, RHS.getInt(), LHS))
-  return false;
-Value = HandleIntToIntCast(Info, E, SubobjType, PromotedLHSType, LHS);
-return true;
+if (RHS.isInt()) {
+  APSInt LHS =
+  HandleIntToIntCast(Info, E, PromotedLHSType, SubobjType, Value);
+  if (!handleIntIntBinOp(Info, E, LHS, Opcode, RHS.getInt(), LHS))
+return false;
+  Value = HandleIntToIntCast(Info, E, SubobjType, PromotedLHSType, LHS);
+  return true;
+} else if (RHS.isFloat()) {
+  APFloat FValue(0.0);
+  return HandleIntToFloatCast(Info, E, SubobjType, Value, PromotedLHSType,
+  FValue) &&
+ handleFloatFloatBinOp(Info, E, FValue, Opcode, RHS.getFloat()) &&
+ HandleFloatToIntCast(Info, E, PromotedLHSType, FValue, SubobjType,
+  Value);
+}
+
+Info.FFDiag(E);
+return false;
   }
   bool found(APFloat , QualType SubobjType) {
 return checkConst(SubobjType) &&


Index: test/SemaCXX/constant-expression-cxx1y.cpp
===
--- test/SemaCXX/constant-expression-cxx1y.cpp
+++ test/SemaCXX/constant-expression-cxx1y.cpp
@@ -356,6 +356,14 @@
 if (a != 13) return false;
 a &= 14;
 if (a != 12) return false;
+a += -1.2;
+if (a != 10) return false;
+a -= 3.1;
+if (a != 6) return false;
+a *= 2.2;
+if (a != 13) return false;
+if (&(a /= 1.5) != ) return false;
+if (a != 8) return false;
 return true;
   }
   static_assert(test_int(), "");
Index: lib/AST/ExprConstant.cpp
===
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -3439,19 +3439,31 @@
 if (!checkConst(SubobjType))
   return false;
 
-if (!SubobjType->isIntegerType() || !RHS.isInt()) {
+if (!SubobjType->isIntegerType()) {
   // We don't support compound assignment on integer-cast-to-pointer
   // values.
   Info.FFDiag(E);
   return false;
 }
 
-APSInt LHS = HandleIntToIntCast(Info, E, PromotedLHSType,
-SubobjType, Value);
-if (!handleIntIntBinOp(Info, E, LHS, Opcode, RHS.getInt(), LHS))
-  return false;
-Value = HandleIntToIntCast(Info, E, SubobjType, PromotedLHSType, LHS);
-return true;
+if (RHS.isInt()) {
+  APSInt LHS =
+  HandleIntToIntCast(Info, E, PromotedLHSType, SubobjType, Value);
+  if (!handleIntIntBinOp(Info, E, LHS, Opcode, RHS.getInt(), LHS))
+return false;
+  Value = HandleIntToIntCast(Info, E, SubobjType, PromotedLHSType, LHS);
+  return true;
+} else if (RHS.isFloat()) {
+  APFloat FValue(0.0);
+  return HandleIntToFloatCast(Info, E, SubobjType, Value, PromotedLHSType,
+  FValue) &&
+ handleFloatFloatBinOp(Info, E, FValue, Opcode, RHS.getFloat()) &&
+ HandleFloatToIntCast(Info, E, PromotedLHSType, FValue, SubobjType,
+  Value);
+}
+
+Info.FFDiag(E);
+return false;
   }
   bool found(APFloat , QualType SubobjType) {
 return checkConst(SubobjType) &&
___
cfe-commits mailing 

[PATCH] D55413: [ExprConstant] Handle compound assignment when LHS has integral type and RHS has floating point type

[S. B. Tam via Phabricator via cfe-commits]

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



Comment at: clang/lib/AST/ExprConstant.cpp:3453
   Value) &&
handleFloatFloatBinOp(Info, E, Value, Opcode, RHS.getFloat()) &&
HandleFloatToFloatCast(Info, E, PromotedLHSType, SubobjType, Value);

rjmccall wrote:
> rsmith wrote:
> > Does this work for the float += int case?
> IIRC, the RHS gets promoted to the computation type in Sema.
Yes, in the float += int case, the RHS is the result of ImplicitCastExpr of 
kind IntegralToFloating. And `test_float()` contains test for that case.


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

https://reviews.llvm.org/D55413



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

[PATCH] D55413: [ExprConstant] Handle compound assignment when LHS has integral type and RHS has floating point type

[S. B. Tam via Phabricator via cfe-commits]

cpplearner updated this revision to Diff 178242.
cpplearner marked an inline comment as done.

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

https://reviews.llvm.org/D55413

Files:
  clang/lib/AST/ExprConstant.cpp
  clang/test/SemaCXX/constant-expression-cxx1y.cpp


Index: clang/test/SemaCXX/constant-expression-cxx1y.cpp
===
--- clang/test/SemaCXX/constant-expression-cxx1y.cpp
+++ clang/test/SemaCXX/constant-expression-cxx1y.cpp
@@ -356,6 +356,14 @@
 if (a != 13) return false;
 a &= 14;
 if (a != 12) return false;
+a += -1.2;
+if (a != 10) return false;
+a -= 3.1;
+if (a != 6) return false;
+a *= 2.2;
+if (a != 13) return false;
+if (&(a /= 1.5) != ) return false;
+if (a != 8) return false;
 return true;
   }
   static_assert(test_int(), "");
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -3424,19 +3424,31 @@
 if (!checkConst(SubobjType))
   return false;
 
-if (!SubobjType->isIntegerType() || !RHS.isInt()) {
+if (!SubobjType->isIntegerType()) {
   // We don't support compound assignment on integer-cast-to-pointer
   // values.
   Info.FFDiag(E);
   return false;
 }
 
-APSInt LHS = HandleIntToIntCast(Info, E, PromotedLHSType,
-SubobjType, Value);
-if (!handleIntIntBinOp(Info, E, LHS, Opcode, RHS.getInt(), LHS))
-  return false;
-Value = HandleIntToIntCast(Info, E, SubobjType, PromotedLHSType, LHS);
-return true;
+if (RHS.isInt()) {
+  APSInt LHS =
+  HandleIntToIntCast(Info, E, PromotedLHSType, SubobjType, Value);
+  if (!handleIntIntBinOp(Info, E, LHS, Opcode, RHS.getInt(), LHS))
+return false;
+  Value = HandleIntToIntCast(Info, E, SubobjType, PromotedLHSType, LHS);
+  return true;
+} else if (RHS.isFloat()) {
+  APFloat FValue(0.0);
+  return HandleIntToFloatCast(Info, E, SubobjType, Value, PromotedLHSType,
+  FValue) &&
+ handleFloatFloatBinOp(Info, E, FValue, Opcode, RHS.getFloat()) &&
+ HandleFloatToIntCast(Info, E, PromotedLHSType, FValue, SubobjType,
+  Value);
+}
+
+Info.FFDiag(E);
+return false;
   }
   bool found(APFloat , QualType SubobjType) {
 return checkConst(SubobjType) &&


Index: clang/test/SemaCXX/constant-expression-cxx1y.cpp
===
--- clang/test/SemaCXX/constant-expression-cxx1y.cpp
+++ clang/test/SemaCXX/constant-expression-cxx1y.cpp
@@ -356,6 +356,14 @@
 if (a != 13) return false;
 a &= 14;
 if (a != 12) return false;
+a += -1.2;
+if (a != 10) return false;
+a -= 3.1;
+if (a != 6) return false;
+a *= 2.2;
+if (a != 13) return false;
+if (&(a /= 1.5) != ) return false;
+if (a != 8) return false;
 return true;
   }
   static_assert(test_int(), "");
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -3424,19 +3424,31 @@
 if (!checkConst(SubobjType))
   return false;
 
-if (!SubobjType->isIntegerType() || !RHS.isInt()) {
+if (!SubobjType->isIntegerType()) {
   // We don't support compound assignment on integer-cast-to-pointer
   // values.
   Info.FFDiag(E);
   return false;
 }
 
-APSInt LHS = HandleIntToIntCast(Info, E, PromotedLHSType,
-SubobjType, Value);
-if (!handleIntIntBinOp(Info, E, LHS, Opcode, RHS.getInt(), LHS))
-  return false;
-Value = HandleIntToIntCast(Info, E, SubobjType, PromotedLHSType, LHS);
-return true;
+if (RHS.isInt()) {
+  APSInt LHS =
+  HandleIntToIntCast(Info, E, PromotedLHSType, SubobjType, Value);
+  if (!handleIntIntBinOp(Info, E, LHS, Opcode, RHS.getInt(), LHS))
+return false;
+  Value = HandleIntToIntCast(Info, E, SubobjType, PromotedLHSType, LHS);
+  return true;
+} else if (RHS.isFloat()) {
+  APFloat FValue(0.0);
+  return HandleIntToFloatCast(Info, E, SubobjType, Value, PromotedLHSType,
+  FValue) &&
+ handleFloatFloatBinOp(Info, E, FValue, Opcode, RHS.getFloat()) &&
+ HandleFloatToIntCast(Info, E, PromotedLHSType, FValue, SubobjType,
+  Value);
+}
+
+Info.FFDiag(E);
+return false;
   }
   bool found(APFloat , QualType SubobjType) {
 return checkConst(SubobjType) &&
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55413: [ExprConstant] Handle compound assignment when LHS has integral type and RHS has floating point type

[S. B. Tam via Phabricator via cfe-commits]

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



Comment at: clang/lib/AST/ExprConstant.cpp:3441
+  Value);
+}
 APSInt LHS = HandleIntToIntCast(Info, E, PromotedLHSType,

rjmccall wrote:
> cpplearner wrote:
> > rjmccall wrote:
> > > Can we more fundamentally restructure this entire handler so that, if the 
> > > compound assignment's computation result type is an arithmetic type, we 
> > > just promote both operands to that type, do the arithmetic there, and 
> > > then coerce back down?  This is C++, so the LHS type imposes almost no 
> > > restrictions on the RHS type; also, this is Clang, where we support way 
> > > too many arithmetic types for our own good.
> > It seems the conditional statement is unavoidable, because `APSInt` and 
> > `APFloat` can't be handled at the same time (i.e. you need to choose among 
> > `Handle{Int,Float}To{Int,Float}Cast`, and between 
> > `handleIntIntBinOp`/`handleFloatFloatBinOp`). Maybe it's possible to add a 
> > layer that can accept both `APSInt` and `APFloat`, but it seems like an 
> > overkill if it's only used in the compound assignment case.
> But we can have `HandleValueTo{Int,Float}Cast` functions that start with an 
> arbitrary `APValue` and do the switch on that type, and we can have a 
> `HandleValueValueBinOp` function that asserts that its operands have the same 
> type and does the switch, and those two together should be good enough for 
> what we need here.
That's what I mean by "add a layer", and I think it's unworthy.

Maybe it makes more sense as part of a larger scale rewrite, but that's more 
than I can deal with.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55413



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

[PATCH] D55413: [ExprConstant] Handle compound assignment when LHS has integral type and RHS has floating point type

[S. B. Tam via Phabricator via cfe-commits]

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



Comment at: clang/lib/AST/ExprConstant.cpp:3441
+  Value);
+}
 APSInt LHS = HandleIntToIntCast(Info, E, PromotedLHSType,

rjmccall wrote:
> Can we more fundamentally restructure this entire handler so that, if the 
> compound assignment's computation result type is an arithmetic type, we just 
> promote both operands to that type, do the arithmetic there, and then coerce 
> back down?  This is C++, so the LHS type imposes almost no restrictions on 
> the RHS type; also, this is Clang, where we support way too many arithmetic 
> types for our own good.
It seems the conditional statement is unavoidable, because `APSInt` and 
`APFloat` can't be handled at the same time (i.e. you need to choose among 
`Handle{Int,Float}To{Int,Float}Cast`, and between 
`handleIntIntBinOp`/`handleFloatFloatBinOp`). Maybe it's possible to add a 
layer that can accept both `APSInt` and `APFloat`, but it seems like an 
overkill if it's only used in the compound assignment case.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55413



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

[PATCH] D55413: [ExprConstant] Handle compound assignment when LHS has integral type and RHS has floating point type

[S. B. Tam via Phabricator via cfe-commits]

cpplearner updated this revision to Diff 177719.
cpplearner added a comment.

Added parentheses. Restored the original tests and add more tests to the end of 
this function.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55413

Files:
  clang/lib/AST/ExprConstant.cpp
  clang/test/SemaCXX/constant-expression-cxx1y.cpp


Index: clang/test/SemaCXX/constant-expression-cxx1y.cpp
===
--- clang/test/SemaCXX/constant-expression-cxx1y.cpp
+++ clang/test/SemaCXX/constant-expression-cxx1y.cpp
@@ -356,6 +356,14 @@
 if (a != 13) return false;
 a &= 14;
 if (a != 12) return false;
+a += -1.2;
+if (a != 10) return false;
+a -= 3.1;
+if (a != 6) return false;
+a *= 2.2;
+if (a != 13) return false;
+if (&(a /= 1.5) != ) return false;
+if (a != 8) return false;
 return true;
   }
   static_assert(test_int(), "");
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -3424,13 +3424,21 @@
 if (!checkConst(SubobjType))
   return false;
 
-if (!SubobjType->isIntegerType() || !RHS.isInt()) {
+if (!SubobjType->isIntegerType() || (!RHS.isInt() && !RHS.isFloat())) {
   // We don't support compound assignment on integer-cast-to-pointer
   // values.
   Info.FFDiag(E);
   return false;
 }
 
+if (RHS.isFloat()) {
+  APFloat FValue(0.0);
+  return HandleIntToFloatCast(Info, E, SubobjType, Value, PromotedLHSType,
+  FValue) &&
+ handleFloatFloatBinOp(Info, E, FValue, Opcode, RHS.getFloat()) &&
+ HandleFloatToIntCast(Info, E, PromotedLHSType, FValue, SubobjType,
+  Value);
+}
 APSInt LHS = HandleIntToIntCast(Info, E, PromotedLHSType,
 SubobjType, Value);
 if (!handleIntIntBinOp(Info, E, LHS, Opcode, RHS.getInt(), LHS))


Index: clang/test/SemaCXX/constant-expression-cxx1y.cpp
===
--- clang/test/SemaCXX/constant-expression-cxx1y.cpp
+++ clang/test/SemaCXX/constant-expression-cxx1y.cpp
@@ -356,6 +356,14 @@
 if (a != 13) return false;
 a &= 14;
 if (a != 12) return false;
+a += -1.2;
+if (a != 10) return false;
+a -= 3.1;
+if (a != 6) return false;
+a *= 2.2;
+if (a != 13) return false;
+if (&(a /= 1.5) != ) return false;
+if (a != 8) return false;
 return true;
   }
   static_assert(test_int(), "");
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -3424,13 +3424,21 @@
 if (!checkConst(SubobjType))
   return false;
 
-if (!SubobjType->isIntegerType() || !RHS.isInt()) {
+if (!SubobjType->isIntegerType() || (!RHS.isInt() && !RHS.isFloat())) {
   // We don't support compound assignment on integer-cast-to-pointer
   // values.
   Info.FFDiag(E);
   return false;
 }
 
+if (RHS.isFloat()) {
+  APFloat FValue(0.0);
+  return HandleIntToFloatCast(Info, E, SubobjType, Value, PromotedLHSType,
+  FValue) &&
+ handleFloatFloatBinOp(Info, E, FValue, Opcode, RHS.getFloat()) &&
+ HandleFloatToIntCast(Info, E, PromotedLHSType, FValue, SubobjType,
+  Value);
+}
 APSInt LHS = HandleIntToIntCast(Info, E, PromotedLHSType,
 SubobjType, Value);
 if (!handleIntIntBinOp(Info, E, LHS, Opcode, RHS.getInt(), LHS))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55413: [ExprConstant] Handle compound assignment when LHS has integral type and RHS has floating point type

[S. B. Tam via Phabricator via cfe-commits]

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



Comment at: clang/test/SemaCXX/constant-expression-cxx1y.cpp:343
 if (a != 7) return false;
-a *= 3;
 if (a != 21) return false;

riccibruno wrote:
> Why remove `a *= 3` instead of just adding `a *= 3.1`.
Because adding `a *= 3.1` here will change the result of the whole calculation?


Repository:
  rC Clang

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

https://reviews.llvm.org/D55413



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

[PATCH] D55413: [ExprConstant] Handle compound assignment when LHS has integral type and RHS has floating point type

[S. B. Tam via Phabricator via cfe-commits]

cpplearner updated this revision to Diff 177708.

Repository:
  rC Clang

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

https://reviews.llvm.org/D55413

Files:
  clang/lib/AST/ExprConstant.cpp
  clang/test/SemaCXX/constant-expression-cxx1y.cpp


Index: clang/test/SemaCXX/constant-expression-cxx1y.cpp
===
--- clang/test/SemaCXX/constant-expression-cxx1y.cpp
+++ clang/test/SemaCXX/constant-expression-cxx1y.cpp
@@ -340,9 +340,9 @@
 if (a != 9) return false;
 a -= 2;
 if (a != 7) return false;
-a *= 3;
+a *= 3.1;
 if (a != 21) return false;
-if (&(a /= 10) != ) return false;
+if (&(a /= 7.9) != ) return false;
 if (a != 2) return false;
 a <<= 3;
 if (a != 16) return false;
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -3424,13 +3424,21 @@
 if (!checkConst(SubobjType))
   return false;
 
-if (!SubobjType->isIntegerType() || !RHS.isInt()) {
+if (!SubobjType->isIntegerType() || !RHS.isInt() && !RHS.isFloat()) {
   // We don't support compound assignment on integer-cast-to-pointer
   // values.
   Info.FFDiag(E);
   return false;
 }
 
+if (RHS.isFloat()) {
+  APFloat FValue(0.0);
+  return HandleIntToFloatCast(Info, E, SubobjType, Value, PromotedLHSType,
+  FValue) &&
+ handleFloatFloatBinOp(Info, E, FValue, Opcode, RHS.getFloat()) &&
+ HandleFloatToIntCast(Info, E, PromotedLHSType, FValue, SubobjType,
+  Value);
+}
 APSInt LHS = HandleIntToIntCast(Info, E, PromotedLHSType,
 SubobjType, Value);
 if (!handleIntIntBinOp(Info, E, LHS, Opcode, RHS.getInt(), LHS))


Index: clang/test/SemaCXX/constant-expression-cxx1y.cpp
===
--- clang/test/SemaCXX/constant-expression-cxx1y.cpp
+++ clang/test/SemaCXX/constant-expression-cxx1y.cpp
@@ -340,9 +340,9 @@
 if (a != 9) return false;
 a -= 2;
 if (a != 7) return false;
-a *= 3;
+a *= 3.1;
 if (a != 21) return false;
-if (&(a /= 10) != ) return false;
+if (&(a /= 7.9) != ) return false;
 if (a != 2) return false;
 a <<= 3;
 if (a != 16) return false;
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -3424,13 +3424,21 @@
 if (!checkConst(SubobjType))
   return false;
 
-if (!SubobjType->isIntegerType() || !RHS.isInt()) {
+if (!SubobjType->isIntegerType() || !RHS.isInt() && !RHS.isFloat()) {
   // We don't support compound assignment on integer-cast-to-pointer
   // values.
   Info.FFDiag(E);
   return false;
 }
 
+if (RHS.isFloat()) {
+  APFloat FValue(0.0);
+  return HandleIntToFloatCast(Info, E, SubobjType, Value, PromotedLHSType,
+  FValue) &&
+ handleFloatFloatBinOp(Info, E, FValue, Opcode, RHS.getFloat()) &&
+ HandleFloatToIntCast(Info, E, PromotedLHSType, FValue, SubobjType,
+  Value);
+}
 APSInt LHS = HandleIntToIntCast(Info, E, PromotedLHSType,
 SubobjType, Value);
 if (!handleIntIntBinOp(Info, E, LHS, Opcode, RHS.getInt(), LHS))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55413: [ExprConstant] Handle compound assignment when LHS has integral type and RHS has floating point type

[S. B. Tam via Phabricator via cfe-commits]

cpplearner created this revision.
cpplearner added a reviewer: rsmith.
Herald added a subscriber: cfe-commits.

Fixes PR39858


Repository:
  rC Clang

https://reviews.llvm.org/D55413

Files:
  clang/lib/AST/ExprConstant.cpp
  clang/test/SemaCXX/constant-expression-cxx1y.cpp


Index: clang/test/SemaCXX/constant-expression-cxx1y.cpp
===
--- clang/test/SemaCXX/constant-expression-cxx1y.cpp
+++ clang/test/SemaCXX/constant-expression-cxx1y.cpp
@@ -338,11 +338,11 @@
 int a = 3;
 a += 6;
 if (a != 9) return false;
-a -= 2;
+a -= 1.8;
 if (a != 7) return false;
 a *= 3;
 if (a != 21) return false;
-if (&(a /= 10) != ) return false;
+if (&(a /= 7.9) != ) return false;
 if (a != 2) return false;
 a <<= 3;
 if (a != 16) return false;
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -3424,13 +3424,21 @@
 if (!checkConst(SubobjType))
   return false;

-if (!SubobjType->isIntegerType() || !RHS.isInt()) {
+if (!SubobjType->isIntegerType() || !RHS.isInt() && !RHS.isFloat()) {
   // We don't support compound assignment on integer-cast-to-pointer
   // values.
   Info.FFDiag(E);
   return false;
 }

+if (RHS.isFloat()) {
+  APFloat FValue(0.0);
+  return HandleIntToFloatCast(Info, E, SubobjType, Value, PromotedLHSType,
+  FValue) &&
+ handleFloatFloatBinOp(Info, E, FValue, Opcode, RHS.getFloat()) &&
+ HandleFloatToIntCast(Info, E, PromotedLHSType, FValue, SubobjType,
+  Value);
+}
 APSInt LHS = HandleIntToIntCast(Info, E, PromotedLHSType,
 SubobjType, Value);
 if (!handleIntIntBinOp(Info, E, LHS, Opcode, RHS.getInt(), LHS))


Index: clang/test/SemaCXX/constant-expression-cxx1y.cpp
===
--- clang/test/SemaCXX/constant-expression-cxx1y.cpp
+++ clang/test/SemaCXX/constant-expression-cxx1y.cpp
@@ -338,11 +338,11 @@
 int a = 3;
 a += 6;
 if (a != 9) return false;
-a -= 2;
+a -= 1.8;
 if (a != 7) return false;
 a *= 3;
 if (a != 21) return false;
-if (&(a /= 10) != ) return false;
+if (&(a /= 7.9) != ) return false;
 if (a != 2) return false;
 a <<= 3;
 if (a != 16) return false;
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -3424,13 +3424,21 @@
 if (!checkConst(SubobjType))
   return false;

-if (!SubobjType->isIntegerType() || !RHS.isInt()) {
+if (!SubobjType->isIntegerType() || !RHS.isInt() && !RHS.isFloat()) {
   // We don't support compound assignment on integer-cast-to-pointer
   // values.
   Info.FFDiag(E);
   return false;
 }

+if (RHS.isFloat()) {
+  APFloat FValue(0.0);
+  return HandleIntToFloatCast(Info, E, SubobjType, Value, PromotedLHSType,
+  FValue) &&
+ handleFloatFloatBinOp(Info, E, FValue, Opcode, RHS.getFloat()) &&
+ HandleFloatToIntCast(Info, E, PromotedLHSType, FValue, SubobjType,
+  Value);
+}
 APSInt LHS = HandleIntToIntCast(Info, E, PromotedLHSType,
 SubobjType, Value);
 if (!handleIntIntBinOp(Info, E, LHS, Opcode, RHS.getInt(), LHS))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44480: [Sema] Don't skip function bodies with 'auto' without trailing return type

[S. B. Tam via Phabricator via cfe-commits]

cpplearner added a comment.

Does `getAs()` work correctly with function returning `auto&`?


Repository:
  rL LLVM

https://reviews.llvm.org/D44480



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

[PATCH] D47419: [SemaDeclCXX] Allow inheriting constructor declaration that specify a cv-qualified type

[S. B. Tam via Phabricator via cfe-commits]

cpplearner added inline comments.



Comment at: lib/Sema/SemaDeclCXX.cpp:9690
+  CanQualType CanonicalDesiredBase = DesiredBase->getCanonicalTypeUnqualified()
+.getUnqualifiedType();
   for (auto  : Derived->bases()) {

rsmith wrote:
> How are we getting a qualified type here? Is this actually a bug in 
> `getCanonicalTypeUnqualified`?
It seems that `getCanonicalTypeUnqualified` does not strip qualifiers from the 
canonical type. I guess "Unqualified" here just means the method does not 
include local qualifiers, unlike QualType::getCanonicalType.

Thus, in the case of `using cbase = const base;`, `getCanonicalTypeUnqualified` 
will return the canonical type of `cbase` as is, which is `const base`, a 
const-qualified type.


Repository:
  rC Clang

https://reviews.llvm.org/D47419



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

[PATCH] D47419: [SemaDeclCXX] Allow inheriting constructor declaration that specify a cv-qualified type

[S. B. Tam via Phabricator via cfe-commits]

cpplearner created this revision.
cpplearner added a reviewer: rsmith.
Herald added a subscriber: cfe-commits.

This will allow the following code:

  struct base {};
  using cbase = const base;
  struct inherit : cbase {
  using cbase::cbase; // previously error: 'cbase' (aka 'const base') is 
not a direct base of 'inherit', cannot inherit constructors
  };

See 
https://stackoverflow.com/questions/50534219/inherit-from-const-type-passed-as-template-parameter


Repository:
  rC Clang

https://reviews.llvm.org/D47419

Files:
  lib/Sema/SemaDeclCXX.cpp
  test/CXX/special/class.inhctor/elsewhere.cpp


Index: test/CXX/special/class.inhctor/elsewhere.cpp
===
--- test/CXX/special/class.inhctor/elsewhere.cpp
+++ test/CXX/special/class.inhctor/elsewhere.cpp
@@ -62,3 +62,4 @@
   G(int &) : G(0) {}
 };
 G g(123);
+G g2(123);
Index: lib/Sema/SemaDeclCXX.cpp
===
--- lib/Sema/SemaDeclCXX.cpp
+++ lib/Sema/SemaDeclCXX.cpp
@@ -9686,7 +9686,8 @@
 QualType DesiredBase,
 bool ) {
   // Check whether the named type is a direct base class.
-  CanQualType CanonicalDesiredBase = 
DesiredBase->getCanonicalTypeUnqualified();
+  CanQualType CanonicalDesiredBase = DesiredBase->getCanonicalTypeUnqualified()
+.getUnqualifiedType();
   for (auto  : Derived->bases()) {
 CanQualType BaseType = Base.getType()->getCanonicalTypeUnqualified();
 if (CanonicalDesiredBase == BaseType)


Index: test/CXX/special/class.inhctor/elsewhere.cpp
===
--- test/CXX/special/class.inhctor/elsewhere.cpp
+++ test/CXX/special/class.inhctor/elsewhere.cpp
@@ -62,3 +62,4 @@
   G(int &) : G(0) {}
 };
 G g(123);
+G g2(123);
Index: lib/Sema/SemaDeclCXX.cpp
===
--- lib/Sema/SemaDeclCXX.cpp
+++ lib/Sema/SemaDeclCXX.cpp
@@ -9686,7 +9686,8 @@
 QualType DesiredBase,
 bool ) {
   // Check whether the named type is a direct base class.
-  CanQualType CanonicalDesiredBase = DesiredBase->getCanonicalTypeUnqualified();
+  CanQualType CanonicalDesiredBase = DesiredBase->getCanonicalTypeUnqualified()
+.getUnqualifiedType();
   for (auto  : Derived->bases()) {
 CanQualType BaseType = Base.getType()->getCanonicalTypeUnqualified();
 if (CanonicalDesiredBase == BaseType)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45403: Make [[maybe_unused]] work with static data members

[S. B. Tam via Phabricator via cfe-commits]

cpplearner added a comment.

I do not have commit access. Could someone commit the change for me?


https://reviews.llvm.org/D45403



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

[PATCH] D45403: Make [[maybe_unused]] work with static data members

[S. B. Tam via Phabricator via cfe-commits]

cpplearner updated this revision to Diff 141880.
cpplearner added a comment.

Removed `ExpectedForMaybeUnused` from AttributeList.h, and removed the entry 
for `ExpectedForMaybeUnused` from the `warn_attribute_wrong_decl_type` table 
definition.


https://reviews.llvm.org/D45403

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Sema/AttributeList.h
  lib/Sema/SemaDeclAttr.cpp
  test/CXX/dcl.dcl/dcl.attr/dcl.attr.unused/p2.cpp


Index: test/CXX/dcl.dcl/dcl.attr/dcl.attr.unused/p2.cpp
===
--- test/CXX/dcl.dcl/dcl.attr/dcl.attr.unused/p2.cpp
+++ test/CXX/dcl.dcl/dcl.attr/dcl.attr.unused/p2.cpp
@@ -2,7 +2,7 @@
 
 struct [[maybe_unused]] S {
   int I [[maybe_unused]];
-  static int SI [[maybe_unused]]; // expected-warning {{'maybe_unused' 
attribute only applies to variables, functions, methods, types, enumerations, 
enumerators, labels, and non-static data members}}
+  static int SI [[maybe_unused]];
 };
 
 enum [[maybe_unused]] E1 {
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -2042,16 +2042,6 @@
 static void handleUnusedAttr(Sema , Decl *D, const AttributeList ) {
   bool IsCXX17Attr = AL.isCXX11Attribute() && !AL.getScopeName();
 
-  if (IsCXX17Attr && isa(D)) {
-// The C++17 spelling of this attribute cannot be applied to a static data
-// member per [dcl.attr.unused]p2.
-if (cast(D)->isStaticDataMember()) {
-  S.Diag(AL.getLoc(), diag::warn_attribute_wrong_decl_type)
-  << AL.getName() << ExpectedForMaybeUnused;
-  return;
-}
-  }
-
   // If this is spelled as the standard C++17 attribute, but not in C++17, warn
   // about using it as an extension.
   if (!S.getLangOpts().CPlusPlus17 && IsCXX17Attr)
Index: include/clang/Sema/AttributeList.h
===
--- include/clang/Sema/AttributeList.h
+++ include/clang/Sema/AttributeList.h
@@ -928,7 +928,6 @@
   ExpectedFunctionVariableOrClass,
   ExpectedKernelFunction,
   ExpectedFunctionWithProtoType,
-  ExpectedForMaybeUnused,
 };
 
 } // namespace clang
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -2834,8 +2834,7 @@
   "|types and namespaces"
   "|variables, functions and classes"
   "|kernel functions"
-  "|non-K functions"
-  "|variables, functions, methods, types, enumerations, enumerators, labels, 
and non-static data members}1">,
+  "|non-K functions}1">,
   InGroup;
 def err_attribute_wrong_decl_type : Error;
 def warn_type_attribute_wrong_type : Warning<


Index: test/CXX/dcl.dcl/dcl.attr/dcl.attr.unused/p2.cpp
===
--- test/CXX/dcl.dcl/dcl.attr/dcl.attr.unused/p2.cpp
+++ test/CXX/dcl.dcl/dcl.attr/dcl.attr.unused/p2.cpp
@@ -2,7 +2,7 @@
 
 struct [[maybe_unused]] S {
   int I [[maybe_unused]];
-  static int SI [[maybe_unused]]; // expected-warning {{'maybe_unused' attribute only applies to variables, functions, methods, types, enumerations, enumerators, labels, and non-static data members}}
+  static int SI [[maybe_unused]];
 };
 
 enum [[maybe_unused]] E1 {
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -2042,16 +2042,6 @@
 static void handleUnusedAttr(Sema , Decl *D, const AttributeList ) {
   bool IsCXX17Attr = AL.isCXX11Attribute() && !AL.getScopeName();
 
-  if (IsCXX17Attr && isa(D)) {
-// The C++17 spelling of this attribute cannot be applied to a static data
-// member per [dcl.attr.unused]p2.
-if (cast(D)->isStaticDataMember()) {
-  S.Diag(AL.getLoc(), diag::warn_attribute_wrong_decl_type)
-  << AL.getName() << ExpectedForMaybeUnused;
-  return;
-}
-  }
-
   // If this is spelled as the standard C++17 attribute, but not in C++17, warn
   // about using it as an extension.
   if (!S.getLangOpts().CPlusPlus17 && IsCXX17Attr)
Index: include/clang/Sema/AttributeList.h
===
--- include/clang/Sema/AttributeList.h
+++ include/clang/Sema/AttributeList.h
@@ -928,7 +928,6 @@
   ExpectedFunctionVariableOrClass,
   ExpectedKernelFunction,
   ExpectedFunctionWithProtoType,
-  ExpectedForMaybeUnused,
 };
 
 } // namespace clang
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -2834,8 +2834,7 @@
   "|types and namespaces"
   "|variables, functions and classes"
   "|kernel functions"
-  "|non-K functions"
-  "|variables, functions, methods, types, enumerations, enumerators, 

[PATCH] D45403: Make [[maybe_unused]] work with static data members

[S. B. Tam via Phabricator via cfe-commits]

cpplearner added a comment.

In https://reviews.llvm.org/D45403#1062276, @aaron.ballman wrote:

> I don't think we're currently diagnosing static data members of classes as 
> being unused in the first place; are there plans to implement that 
> functionality so that someone might want to write the attribute there?


Currently, unused static data members are diagnosed when the enclosing class is 
in an anonymous namespace: https://wandbox.org/permlink/50nTcaESHdjK8pkd 
(ironically, the message is "unused variable").


Repository:
  rC Clang

https://reviews.llvm.org/D45403



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

[PATCH] D45403: Make [[maybe_unused]] work with static data members

[S. B. Tam via Phabricator via cfe-commits]

cpplearner added inline comments.



Comment at: test/CXX/dcl.dcl/dcl.attr/dcl.attr.unused/p2.cpp:5
   int I [[maybe_unused]];
-  static int SI [[maybe_unused]]; // expected-warning {{'maybe_unused' 
attribute only applies to variables, functions, methods, types, enumerations, 
enumerators, labels, and non-static data members}}
+  static int SI [[maybe_unused]];
 };

lebedev.ri wrote:
> As the code comment noted, 
> http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2017/n4659.pdf, page 199:
> ```
> 2 The attribute may be applied to the declaration of a class, a typedef-name, 
> a variable, a **non-static** data
> member, a function, an enumeration, or an enumerator.
> ```
That section says that [[maybe_unused]] can be applied to a non-static data 
member, which doesn't mean it can't be applied to a static data member.

And I'm arguing that since a static data member is a variable, 
[dcl.attr.unused]p2 actually allows [[maybe_unused]] to be applied to a static 
data member.


Repository:
  rC Clang

https://reviews.llvm.org/D45403



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

[PATCH] D45403: Make [[maybe_unused]] work with static data members

[S. B. Tam via Phabricator via cfe-commits]

cpplearner created this revision.
cpplearner added reviewers: aaron.ballman, rsmith.
Herald added a subscriber: cfe-commits.

IIUC a static data member is a variable, so [[maybe_unused]] should be allowed 
to apply to a static data member.


Repository:
  rC Clang

https://reviews.llvm.org/D45403

Files:
  lib/Sema/SemaDeclAttr.cpp
  test/CXX/dcl.dcl/dcl.attr/dcl.attr.unused/p2.cpp


Index: test/CXX/dcl.dcl/dcl.attr/dcl.attr.unused/p2.cpp
===
--- test/CXX/dcl.dcl/dcl.attr/dcl.attr.unused/p2.cpp
+++ test/CXX/dcl.dcl/dcl.attr/dcl.attr.unused/p2.cpp
@@ -2,7 +2,7 @@
 
 struct [[maybe_unused]] S {
   int I [[maybe_unused]];
-  static int SI [[maybe_unused]]; // expected-warning {{'maybe_unused' 
attribute only applies to variables, functions, methods, types, enumerations, 
enumerators, labels, and non-static data members}}
+  static int SI [[maybe_unused]];
 };
 
 enum [[maybe_unused]] E1 {
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -2042,16 +2042,6 @@
 static void handleUnusedAttr(Sema , Decl *D, const AttributeList ) {
   bool IsCXX17Attr = AL.isCXX11Attribute() && !AL.getScopeName();
 
-  if (IsCXX17Attr && isa(D)) {
-// The C++17 spelling of this attribute cannot be applied to a static data
-// member per [dcl.attr.unused]p2.
-if (cast(D)->isStaticDataMember()) {
-  S.Diag(AL.getLoc(), diag::warn_attribute_wrong_decl_type)
-  << AL.getName() << ExpectedForMaybeUnused;
-  return;
-}
-  }
-
   // If this is spelled as the standard C++17 attribute, but not in C++17, warn
   // about using it as an extension.
   if (!S.getLangOpts().CPlusPlus17 && IsCXX17Attr)


Index: test/CXX/dcl.dcl/dcl.attr/dcl.attr.unused/p2.cpp
===
--- test/CXX/dcl.dcl/dcl.attr/dcl.attr.unused/p2.cpp
+++ test/CXX/dcl.dcl/dcl.attr/dcl.attr.unused/p2.cpp
@@ -2,7 +2,7 @@
 
 struct [[maybe_unused]] S {
   int I [[maybe_unused]];
-  static int SI [[maybe_unused]]; // expected-warning {{'maybe_unused' attribute only applies to variables, functions, methods, types, enumerations, enumerators, labels, and non-static data members}}
+  static int SI [[maybe_unused]];
 };
 
 enum [[maybe_unused]] E1 {
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -2042,16 +2042,6 @@
 static void handleUnusedAttr(Sema , Decl *D, const AttributeList ) {
   bool IsCXX17Attr = AL.isCXX11Attribute() && !AL.getScopeName();
 
-  if (IsCXX17Attr && isa(D)) {
-// The C++17 spelling of this attribute cannot be applied to a static data
-// member per [dcl.attr.unused]p2.
-if (cast(D)->isStaticDataMember()) {
-  S.Diag(AL.getLoc(), diag::warn_attribute_wrong_decl_type)
-  << AL.getName() << ExpectedForMaybeUnused;
-  return;
-}
-  }
-
   // If this is spelled as the standard C++17 attribute, but not in C++17, warn
   // about using it as an extension.
   if (!S.getLangOpts().CPlusPlus17 && IsCXX17Attr)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D27850: [libcxx] add missing constexpr to optional::value_or

[S. B. Tam via Phabricator via cfe-commits]

cpplearner updated this revision to Diff 82653.
cpplearner added a comment.

backed out the changes to ``


https://reviews.llvm.org/D27850

Files:
  include/optional
  
test/std/utilities/optional/optional.object/optional.object.observe/value_or.pass.cpp


Index: 
test/std/utilities/optional/optional.object/optional.object.observe/value_or.pass.cpp
===
--- 
test/std/utilities/optional/optional.object/optional.object.observe/value_or.pass.cpp
+++ 
test/std/utilities/optional/optional.object/optional.object.observe/value_or.pass.cpp
@@ -10,7 +10,7 @@
 // UNSUPPORTED: c++98, c++03, c++11, c++14
 // 
 
-// template  T optional::value_or(U&& v) &&;
+// template  constexpr T optional::value_or(U&& v) &&;
 
 #include 
 #include 
@@ -26,22 +26,22 @@
 {
 int i_;
 
-Y(int i) : i_(i) {}
+constexpr Y(int i) : i_(i) {}
 };
 
 struct X
 {
 int i_;
 
-X(int i) : i_(i) {}
-X(X&& x) : i_(x.i_) {x.i_ = 0;}
-X(const Y& y) : i_(y.i_) {}
-X(Y&& y) : i_(y.i_+1) {}
+constexpr X(int i) : i_(i) {}
+constexpr X(X&& x) : i_(x.i_) {x.i_ = 0;}
+constexpr X(const Y& y) : i_(y.i_) {}
+constexpr X(Y&& y) : i_(y.i_+1) {}
 friend constexpr bool operator==(const X& x, const X& y)
 {return x.i_ == y.i_;}
 };
 
-int main()
+constexpr int test()
 {
 {
 optional opt(in_place, 2);
@@ -65,4 +65,10 @@
 assert(std::move(opt).value_or(Y(3)) == 4);
 assert(!opt);
 }
+return 0;
+}
+
+int main()
+{
+static_assert(test() == 0);
 }
Index: include/optional
===
--- include/optional
+++ include/optional
@@ -893,7 +893,7 @@
 
 template 
 _LIBCPP_INLINE_VISIBILITY
-value_type value_or(_Up&& __v) &&
+constexpr value_type value_or(_Up&& __v) &&
 {
 static_assert(is_move_constructible_v,
   "optional::value_or: T must be move constructible");


Index: test/std/utilities/optional/optional.object/optional.object.observe/value_or.pass.cpp
===
--- test/std/utilities/optional/optional.object/optional.object.observe/value_or.pass.cpp
+++ test/std/utilities/optional/optional.object/optional.object.observe/value_or.pass.cpp
@@ -10,7 +10,7 @@
 // UNSUPPORTED: c++98, c++03, c++11, c++14
 // 
 
-// template  T optional::value_or(U&& v) &&;
+// template  constexpr T optional::value_or(U&& v) &&;
 
 #include 
 #include 
@@ -26,22 +26,22 @@
 {
 int i_;
 
-Y(int i) : i_(i) {}
+constexpr Y(int i) : i_(i) {}
 };
 
 struct X
 {
 int i_;
 
-X(int i) : i_(i) {}
-X(X&& x) : i_(x.i_) {x.i_ = 0;}
-X(const Y& y) : i_(y.i_) {}
-X(Y&& y) : i_(y.i_+1) {}
+constexpr X(int i) : i_(i) {}
+constexpr X(X&& x) : i_(x.i_) {x.i_ = 0;}
+constexpr X(const Y& y) : i_(y.i_) {}
+constexpr X(Y&& y) : i_(y.i_+1) {}
 friend constexpr bool operator==(const X& x, const X& y)
 {return x.i_ == y.i_;}
 };
 
-int main()
+constexpr int test()
 {
 {
 optional opt(in_place, 2);
@@ -65,4 +65,10 @@
 assert(std::move(opt).value_or(Y(3)) == 4);
 assert(!opt);
 }
+return 0;
+}
+
+int main()
+{
+static_assert(test() == 0);
 }
Index: include/optional
===
--- include/optional
+++ include/optional
@@ -893,7 +893,7 @@
 
 template 
 _LIBCPP_INLINE_VISIBILITY
-value_type value_or(_Up&& __v) &&
+constexpr value_type value_or(_Up&& __v) &&
 {
 static_assert(is_move_constructible_v,
   "optional::value_or: T must be move constructible");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D27850: [libcxx] add missing constexpr to optional::value_or

[S. B. Tam via Phabricator via cfe-commits]

cpplearner updated this revision to Diff 82551.

https://reviews.llvm.org/D27850

Files:
  include/experimental/optional
  include/optional
  
test/std/experimental/optional/optional.object/optional.object.observe/value_or.pass.cpp
  
test/std/utilities/optional/optional.object/optional.object.observe/value_or.pass.cpp

Index: test/std/utilities/optional/optional.object/optional.object.observe/value_or.pass.cpp
===
--- test/std/utilities/optional/optional.object/optional.object.observe/value_or.pass.cpp
+++ test/std/utilities/optional/optional.object/optional.object.observe/value_or.pass.cpp
@@ -10,7 +10,7 @@
 // UNSUPPORTED: c++98, c++03, c++11, c++14
 // 
 
-// template  T optional::value_or(U&& v) &&;
+// template  constexpr T optional::value_or(U&& v) &&;
 
 #include 
 #include 
@@ -26,22 +26,22 @@
 {
 int i_;
 
-Y(int i) : i_(i) {}
+constexpr Y(int i) : i_(i) {}
 };
 
 struct X
 {
 int i_;
 
-X(int i) : i_(i) {}
-X(X&& x) : i_(x.i_) {x.i_ = 0;}
-X(const Y& y) : i_(y.i_) {}
-X(Y&& y) : i_(y.i_+1) {}
+constexpr X(int i) : i_(i) {}
+constexpr X(X&& x) : i_(x.i_) {x.i_ = 0;}
+constexpr X(const Y& y) : i_(y.i_) {}
+constexpr X(Y&& y) : i_(y.i_+1) {}
 friend constexpr bool operator==(const X& x, const X& y)
 {return x.i_ == y.i_;}
 };
 
-int main()
+constexpr int test()
 {
 {
 optional opt(in_place, 2);
@@ -65,4 +65,10 @@
 assert(std::move(opt).value_or(Y(3)) == 4);
 assert(!opt);
 }
+return 0;
+}
+
+int main()
+{
+static_assert(test() == 0);
 }
Index: test/std/experimental/optional/optional.object/optional.object.observe/value_or.pass.cpp
===
--- test/std/experimental/optional/optional.object/optional.object.observe/value_or.pass.cpp
+++ test/std/experimental/optional/optional.object/optional.object.observe/value_or.pass.cpp
@@ -10,7 +10,7 @@
 // UNSUPPORTED: c++98, c++03, c++11
 // 
 
-// template  T optional::value_or(U&& v) &&;
+// template  constexpr T optional::value_or(U&& v) &&;
 
 #include 
 #include 
@@ -24,43 +24,49 @@
 {
 int i_;
 
-Y(int i) : i_(i) {}
+constexpr Y(int i) : i_(i) {}
 };
 
 struct X
 {
 int i_;
 
-X(int i) : i_(i) {}
-X(X&& x) : i_(x.i_) {x.i_ = 0;}
-X(const Y& y) : i_(y.i_) {}
-X(Y&& y) : i_(y.i_+1) {}
+constexpr X(int i) : i_(i) {}
+constexpr X(X&& x) : i_(x.i_) {x.i_ = 0;}
+constexpr X(const Y& y) : i_(y.i_) {}
+constexpr X(Y&& y) : i_(y.i_+1) {}
 friend constexpr bool operator==(const X& x, const X& y)
 {return x.i_ == y.i_;}
 };
 
-int main()
+constexpr int test()
 {
 {
 optional opt(in_place, 2);
 Y y(3);
-assert(std::move(opt).value_or(y) == 2);
-assert(*opt == 0);
+if (!(std::move(opt).value_or(y) == 2)) std::abort();
+if (!(*opt == 0)) std::abort();
 }
 {
 optional opt(in_place, 2);
-assert(std::move(opt).value_or(Y(3)) == 2);
-assert(*opt == 0);
+if (!(std::move(opt).value_or(Y(3)) == 2)) std::abort();
+if (!(*opt == 0)) std::abort();
 }
 {
 optional opt;
 Y y(3);
-assert(std::move(opt).value_or(y) == 3);
-assert(!opt);
+if (!(std::move(opt).value_or(y) == 3)) std::abort();
+if (!(!opt)) std::abort();
 }
 {
 optional opt;
-assert(std::move(opt).value_or(Y(3)) == 4);
-assert(!opt);
+if (!(std::move(opt).value_or(Y(3)) == 4)) std::abort();
+if (!(!opt)) std::abort();
 }
+return 0;
+}
+
+int main()
+{
+static_assert(test() == 0, "");
 }
Index: include/optional
===
--- include/optional
+++ include/optional
@@ -893,7 +893,7 @@
 
 template 
 _LIBCPP_INLINE_VISIBILITY
-value_type value_or(_Up&& __v) &&
+constexpr value_type value_or(_Up&& __v) &&
 {
 static_assert(is_move_constructible_v,
   "optional::value_or: T must be move constructible");
Index: include/experimental/optional
===
--- include/experimental/optional
+++ include/experimental/optional
@@ -562,7 +562,7 @@
 
 template 
 _LIBCPP_INLINE_VISIBILITY
-value_type value_or(_Up&& __v) &&
+constexpr value_type value_or(_Up&& __v) &&
 {
 static_assert(is_move_constructible::value,
   "optional::value_or: T must be move constructible");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D27850: [libcxx] add missing constexpr to optional::value_or

[S. B. Tam via Phabricator via cfe-commits]

cpplearner updated this revision to Diff 82549.
cpplearner added a comment.

test updated


https://reviews.llvm.org/D27850

Files:
  
test/std/utilities/optional/optional.object/optional.object.observe/value_or.pass.cpp


Index: 
test/std/utilities/optional/optional.object/optional.object.observe/value_or.pass.cpp
===
--- 
test/std/utilities/optional/optional.object/optional.object.observe/value_or.pass.cpp
+++ 
test/std/utilities/optional/optional.object/optional.object.observe/value_or.pass.cpp
@@ -26,22 +26,22 @@
 {
 int i_;
 
-Y(int i) : i_(i) {}
+constexpr Y(int i) : i_(i) {}
 };
 
 struct X
 {
 int i_;
 
-X(int i) : i_(i) {}
-X(X&& x) : i_(x.i_) {x.i_ = 0;}
-X(const Y& y) : i_(y.i_) {}
-X(Y&& y) : i_(y.i_+1) {}
+constexpr X(int i) : i_(i) {}
+constexpr X(X&& x) : i_(x.i_) {x.i_ = 0;}
+constexpr X(const Y& y) : i_(y.i_) {}
+constexpr X(Y&& y) : i_(y.i_+1) {}
 friend constexpr bool operator==(const X& x, const X& y)
 {return x.i_ == y.i_;}
 };
 
-int main()
+constexpr int test()
 {
 {
 optional opt(in_place, 2);
@@ -65,4 +65,10 @@
 assert(std::move(opt).value_or(Y(3)) == 4);
 assert(!opt);
 }
+return 0;
+}
+
+int main()
+{
+static_assert(test() == 0);
 }


Index: test/std/utilities/optional/optional.object/optional.object.observe/value_or.pass.cpp
===
--- test/std/utilities/optional/optional.object/optional.object.observe/value_or.pass.cpp
+++ test/std/utilities/optional/optional.object/optional.object.observe/value_or.pass.cpp
@@ -26,22 +26,22 @@
 {
 int i_;
 
-Y(int i) : i_(i) {}
+constexpr Y(int i) : i_(i) {}
 };
 
 struct X
 {
 int i_;
 
-X(int i) : i_(i) {}
-X(X&& x) : i_(x.i_) {x.i_ = 0;}
-X(const Y& y) : i_(y.i_) {}
-X(Y&& y) : i_(y.i_+1) {}
+constexpr X(int i) : i_(i) {}
+constexpr X(X&& x) : i_(x.i_) {x.i_ = 0;}
+constexpr X(const Y& y) : i_(y.i_) {}
+constexpr X(Y&& y) : i_(y.i_+1) {}
 friend constexpr bool operator==(const X& x, const X& y)
 {return x.i_ == y.i_;}
 };
 
-int main()
+constexpr int test()
 {
 {
 optional opt(in_place, 2);
@@ -65,4 +65,10 @@
 assert(std::move(opt).value_or(Y(3)) == 4);
 assert(!opt);
 }
+return 0;
+}
+
+int main()
+{
+static_assert(test() == 0);
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D27850: [libcxx] add missing constexpr to optional::value_or

[S. B. Tam via Phabricator via cfe-commits]

cpplearner created this revision.
cpplearner added reviewers: mclow.lists, EricWF, howard.hinnant.
cpplearner added a subscriber: cfe-commits.

See https://github.com/cplusplus/draft/pull/839 and 
https://github.com/cplusplus/fundamentals-ts/pull/73.


https://reviews.llvm.org/D27850

Files:
  include/experimental/optional
  include/optional


Index: include/optional
===
--- include/optional
+++ include/optional
@@ -893,7 +893,7 @@
 
 template 
 _LIBCPP_INLINE_VISIBILITY
-value_type value_or(_Up&& __v) &&
+constexpr value_type value_or(_Up&& __v) &&
 {
 static_assert(is_move_constructible_v,
   "optional::value_or: T must be move constructible");
Index: include/experimental/optional
===
--- include/experimental/optional
+++ include/experimental/optional
@@ -562,7 +562,7 @@
 
 template 
 _LIBCPP_INLINE_VISIBILITY
-value_type value_or(_Up&& __v) &&
+constexpr value_type value_or(_Up&& __v) &&
 {
 static_assert(is_move_constructible::value,
   "optional::value_or: T must be move constructible");


Index: include/optional
===
--- include/optional
+++ include/optional
@@ -893,7 +893,7 @@
 
 template 
 _LIBCPP_INLINE_VISIBILITY
-value_type value_or(_Up&& __v) &&
+constexpr value_type value_or(_Up&& __v) &&
 {
 static_assert(is_move_constructible_v,
   "optional::value_or: T must be move constructible");
Index: include/experimental/optional
===
--- include/experimental/optional
+++ include/experimental/optional
@@ -562,7 +562,7 @@
 
 template 
 _LIBCPP_INLINE_VISIBILITY
-value_type value_or(_Up&& __v) &&
+constexpr value_type value_or(_Up&& __v) &&
 {
 static_assert(is_move_constructible::value,
   "optional::value_or: T must be move constructible");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits