Re: [PATCH] D24193: Allow variables with asm labels in naked functions

[Nikola Smiljanić via cfe-commits]

nikola closed this revision.
nikola added a comment.

r281298


https://reviews.llvm.org/D24193



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

Re: [PATCH] D24193: Allow variables with asm labels in naked functions

[Nikola Smiljanić via cfe-commits]

nikola updated this revision to Diff 70985.
nikola added a comment.

This should address Hans' comments, as for the code get I have no idea. I was 
hoping someone more knowledgeable would tell me if this made sense or not?


https://reviews.llvm.org/D24193

Files:
  lib/Sema/SemaDecl.cpp
  test/Sema/attr-naked.c

Index: test/Sema/attr-naked.c
===
--- test/Sema/attr-naked.c
+++ test/Sema/attr-naked.c
@@ -48,3 +48,21 @@
"r"(z) // expected-error{{parameter references not allowed in 
naked functions}}
);
 }
+
+__attribute__((naked)) void t10() {  // expected-note{{attribute is here}}
+  int a; // expected-error{{non-ASM statement in naked function is not 
supported}}
+}
+
+__attribute__((naked)) void t11() {  // expected-note{{attribute is here}}
+  register int a asm("eax") = x; // expected-error{{non-ASM statement in naked 
function is not supported}}
+}
+
+__attribute__((naked)) void t12() {  // expected-note{{attribute is here}}
+  register int a asm("eax"), b asm("ebx") = x; // expected-error{{non-ASM 
statement in naked function is not supported}}
+}
+
+__attribute__((naked)) void t13() {
+  register int a asm("eax");
+  register int b asm("ebx"), c asm("ecx");
+}
+
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -11792,6 +11792,21 @@
 
 if (FD && FD->hasAttr()) {
   for (const Stmt *S : Body->children()) {
+// Allow local register variables without initializer as they don't
+// require prologue.
+bool RegisterVariables = false;
+if (auto *DS = dyn_cast(S)) {
+  for (const auto *Decl : DS->decls()) {
+if (const auto *Var = dyn_cast(Decl)) {
+  RegisterVariables =
+  Var->hasAttr() && !Var->hasInit();
+  if (!RegisterVariables)
+break;
+}
+  }
+}
+if (RegisterVariables)
+  continue;
 if (!isa(S) && !isa(S)) {
   Diag(S->getLocStart(), diag::err_non_asm_stmt_in_naked_function);
   Diag(FD->getAttr()->getLocation(), diag::note_attribute);


Index: test/Sema/attr-naked.c
===
--- test/Sema/attr-naked.c
+++ test/Sema/attr-naked.c
@@ -48,3 +48,21 @@
"r"(z) // expected-error{{parameter references not allowed in naked functions}}
);
 }
+
+__attribute__((naked)) void t10() {  // expected-note{{attribute is here}}
+  int a; // expected-error{{non-ASM statement in naked function is not supported}}
+}
+
+__attribute__((naked)) void t11() {  // expected-note{{attribute is here}}
+  register int a asm("eax") = x; // expected-error{{non-ASM statement in naked function is not supported}}
+}
+
+__attribute__((naked)) void t12() {  // expected-note{{attribute is here}}
+  register int a asm("eax"), b asm("ebx") = x; // expected-error{{non-ASM statement in naked function is not supported}}
+}
+
+__attribute__((naked)) void t13() {
+  register int a asm("eax");
+  register int b asm("ebx"), c asm("ecx");
+}
+
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -11792,6 +11792,21 @@
 
 if (FD && FD->hasAttr()) {
   for (const Stmt *S : Body->children()) {
+// Allow local register variables without initializer as they don't
+// require prologue.
+bool RegisterVariables = false;
+if (auto *DS = dyn_cast(S)) {
+  for (const auto *Decl : DS->decls()) {
+if (const auto *Var = dyn_cast(Decl)) {
+  RegisterVariables =
+  Var->hasAttr() && !Var->hasInit();
+  if (!RegisterVariables)
+break;
+}
+  }
+}
+if (RegisterVariables)
+  continue;
 if (!isa(S) && !isa(S)) {
   Diag(S->getLocStart(), diag::err_non_asm_stmt_in_naked_function);
   Diag(FD->getAttr()->getLocation(), diag::note_attribute);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D24395: Align declarations that are preceded by different number of commas.

[Nikola Smiljanić via cfe-commits]

nikola abandoned this revision.
nikola added a comment.

Thanks for letting me know, that patch looks more complete so I'll abandon 
this. I hope it lands soon!


https://reviews.llvm.org/D24395



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

[PATCH] D24395: Align declarations that are preceded by different number of commas.

[Nikola Smiljanić via cfe-commits]

nikola created this revision.
nikola added a reviewer: djasper.
nikola added a subscriber: cfe-commits.
Herald added a subscriber: klimek.

Having a template with more than one template argument breaks alignment of 
consecutive declarations. Something like this won't be correctly aligned:

int x;
std::pair y;

https://reviews.llvm.org/D24395

Files:
  lib/Format/WhitespaceManager.cpp
  unittests/Format/FormatTest.cpp

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -9310,6 +9310,10 @@
   verifyFormat("int  oneTwoThree{0}; // comment\n"
"unsigned oneTwo; // comment",
Alignment);
+  verifyFormat("template  struct pair {};\n"
+   "inta;\n"
+   "pair b;",
+   Alignment);
   EXPECT_EQ("float const a = 5;\n"
 "\n"
 "int oneTwoThree = 123;",
Index: lib/Format/WhitespaceManager.cpp
===
--- lib/Format/WhitespaceManager.cpp
+++ lib/Format/WhitespaceManager.cpp
@@ -197,7 +197,8 @@
 // finalize the previous sequence.
 template 
 static void AlignTokens(const FormatStyle , F &,
-SmallVector ) {
+SmallVector ,
+bool AligningAssignments) {
   unsigned MinColumn = 0;
   unsigned MaxColumn = UINT_MAX;
 
@@ -214,9 +215,9 @@
   unsigned NestingLevelOfLastMatch = 0;
   unsigned NestingLevel = 0;
 
-  // Keep track of the number of commas before the matching tokens, we will 
only
-  // align a sequence of matching tokens if they are preceded by the same 
number
-  // of commas.
+  // Keep track of the number of commas before the matching tokens, when
+  // aligning assignments we will only align a sequence of matching tokens
+  // if they are preceded by the same number of commas.
   unsigned CommasBeforeLastMatch = 0;
   unsigned CommasBeforeMatch = 0;
 
@@ -271,10 +272,10 @@
   continue;
 
 // If there is more than one matching token per line, or if the number of
-// preceding commas, or the scope depth, do not match anymore, end the
-// sequence.
-if (FoundMatchOnLine || CommasBeforeMatch != CommasBeforeLastMatch ||
-NestingLevel != NestingLevelOfLastMatch)
+// preceding commas when aligning assignments, or the scope depth, do not
+// match anymore, end the sequence.
+if (FoundMatchOnLine || NestingLevel != NestingLevelOfLastMatch ||
+(AligningAssignments && CommasBeforeMatch != CommasBeforeLastMatch))
   AlignCurrentSequence();
 
 CommasBeforeLastMatch = CommasBeforeMatch;
@@ -321,7 +322,7 @@
 
 return C.Kind == tok::equal;
   },
-  Changes);
+  Changes, true);
 }
 
 void WhitespaceManager::alignConsecutiveDeclarations() {
@@ -336,7 +337,7 @@
   //   SomeVeryLongType const& v3;
 
   AlignTokens(Style, [](Change const ) { return C.IsStartOfDeclName; },
-  Changes);
+  Changes, false);
 }
 
 void WhitespaceManager::alignTrailingComments() {


Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -9310,6 +9310,10 @@
   verifyFormat("int  oneTwoThree{0}; // comment\n"
"unsigned oneTwo; // comment",
Alignment);
+  verifyFormat("template  struct pair {};\n"
+   "inta;\n"
+   "pair b;",
+   Alignment);
   EXPECT_EQ("float const a = 5;\n"
 "\n"
 "int oneTwoThree = 123;",
Index: lib/Format/WhitespaceManager.cpp
===
--- lib/Format/WhitespaceManager.cpp
+++ lib/Format/WhitespaceManager.cpp
@@ -197,7 +197,8 @@
 // finalize the previous sequence.
 template 
 static void AlignTokens(const FormatStyle , F &,
-SmallVector ) {
+SmallVector ,
+bool AligningAssignments) {
   unsigned MinColumn = 0;
   unsigned MaxColumn = UINT_MAX;
 
@@ -214,9 +215,9 @@
   unsigned NestingLevelOfLastMatch = 0;
   unsigned NestingLevel = 0;
 
-  // Keep track of the number of commas before the matching tokens, we will only
-  // align a sequence of matching tokens if they are preceded by the same number
-  // of commas.
+  // Keep track of the number of commas before the matching tokens, when
+  // aligning assignments we will only align a sequence of matching tokens
+  // if they are preceded by the same number of commas.
   unsigned CommasBeforeLastMatch = 0;
   unsigned CommasBeforeMatch = 0;
 
@@ -271,10 

[PATCH] D24193: Allow variables with asm labels in naked functions

[Nikola Smiljanić via cfe-commits]

nikola created this revision.
nikola added reviewers: hans, rnk, compnerd.
nikola added a subscriber: cfe-commits.
Herald added a subscriber: aemerson.

I think the current mode is too restrictive, it will emit error for any 
statement inside a naked function. Code I'm trying to compile for ARM declares 
registers as variables to improve readability and passes them as input operands 
to inline assembly.

register uint32_t Something asm("rax");


https://reviews.llvm.org/D24193

Files:
  lib/Sema/SemaDecl.cpp
  test/Sema/attr-naked.c

Index: test/Sema/attr-naked.c
===
--- test/Sema/attr-naked.c
+++ test/Sema/attr-naked.c
@@ -48,3 +48,12 @@
"r"(z) // expected-error{{parameter references not allowed in 
naked functions}}
);
 }
+
+__attribute__((naked)) void t10() {  // expected-note{{attribute is here}}
+  int x; // expected-error{{non-ASM statement in naked function is not 
supported}}
+}
+
+__attribute__((naked)) void t11() {
+  register int x asm("eax");
+}
+
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -11792,6 +11792,14 @@
 
 if (FD && FD->hasAttr()) {
   for (const Stmt *S : Body->children()) {
+if (auto *DS = dyn_cast(S)) {
+  if (DS->isSingleDecl()) {
+if (auto *Var = dyn_cast_or_null(DS->getSingleDecl())) {
+  if (Var->hasAttr())
+continue;
+}
+  }
+}
 if (!isa(S) && !isa(S)) {
   Diag(S->getLocStart(), diag::err_non_asm_stmt_in_naked_function);
   Diag(FD->getAttr()->getLocation(), diag::note_attribute);


Index: test/Sema/attr-naked.c
===
--- test/Sema/attr-naked.c
+++ test/Sema/attr-naked.c
@@ -48,3 +48,12 @@
"r"(z) // expected-error{{parameter references not allowed in naked functions}}
);
 }
+
+__attribute__((naked)) void t10() {  // expected-note{{attribute is here}}
+  int x; // expected-error{{non-ASM statement in naked function is not supported}}
+}
+
+__attribute__((naked)) void t11() {
+  register int x asm("eax");
+}
+
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -11792,6 +11792,14 @@
 
 if (FD && FD->hasAttr()) {
   for (const Stmt *S : Body->children()) {
+if (auto *DS = dyn_cast(S)) {
+  if (DS->isSingleDecl()) {
+if (auto *Var = dyn_cast_or_null(DS->getSingleDecl())) {
+  if (Var->hasAttr())
+continue;
+}
+  }
+}
 if (!isa(S) && !isa(S)) {
   Diag(S->getLocStart(), diag::err_non_asm_stmt_in_naked_function);
   Diag(FD->getAttr()->getLocation(), diag::note_attribute);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D15509: Suggest missing 'template' for dependent member templates

[Nikola Smiljanić via cfe-commits]

nikola added a comment.

Would anyone be kind enough to review this?


http://reviews.llvm.org/D15509



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

Re: [PATCH] D15509: Suggest missing 'template' for dependent member templates

[Nikola Smiljanić via cfe-commits]

nikola added a comment.

Ping.


http://reviews.llvm.org/D15509



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

Re: [PATCH] D15588: PR25356 - False positive with -Wreturn-stack-address

[Nikola Smiljanić via cfe-commits]

nikola added a comment.

Ping.


http://reviews.llvm.org/D15588



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

[PATCH] D15509: Suggest missing 'template' for dependent member templates

[Nikola Smiljanić via cfe-commits]

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

Current diagnostic says "expected expression" or "reference to non-static 
member function must be called". This should fix PR13566 and PR18995

http://reviews.llvm.org/D15509

Files:
  lib/Parse/ParseTemplate.cpp
  lib/Parse/RAIIObjectsForParser.h
  test/SemaTemplate/dependent-template-recover.cpp

Index: test/SemaTemplate/dependent-template-recover.cpp
===
--- test/SemaTemplate/dependent-template-recover.cpp
+++ test/SemaTemplate/dependent-template-recover.cpp
@@ -11,12 +11,35 @@
 T::getAs(); // expected-error{{use 'template' keyword to treat 'getAs' as a dependent template name}}
 t->T::getAs(); // expected-error{{use 'template' keyword to treat 'getAs' as a dependent template name}}
 
-// FIXME: We can't recover from these yet
-(*t).f2(); // expected-error{{expected expression}}
-(*t).f2<0>(); // expected-error{{expected expression}}
+(*t).f2(); // expected-error{{use 'template' keyword to treat 'f2' as a dependent template name}}
+(*t).f2<0>(); // expected-error{{use 'template' keyword to treat 'f2' as a dependent template name}}
   }
 };
 
+namespace PR13566 {
+template 
+struct S
+{
+  template 
+  void foo();
+
+  template 
+  void foo(int);
+};
+
+template 
+void bar()
+{
+  S s;
+  s.foo<1>(); // expected-error{{use 'template' keyword to treat 'foo' as a dependent template name}}
+  s.foo<1>(0); // expected-error{{use 'template' keyword to treat 'foo' as a dependent template name}}
+}
+
+void instantiate() {
+  bar();
+}
+}
+
 namespace PR9401 {
   // From GCC PR c++/45558
   template 
Index: lib/Parse/RAIIObjectsForParser.h
===
--- lib/Parse/RAIIObjectsForParser.h
+++ lib/Parse/RAIIObjectsForParser.h
@@ -442,6 +442,18 @@
 void skipToEnd();
   };
 
+  /// \brief RAII object that suppresses all diagnostics
+  class SuppressAllDiagnostics {
+DiagnosticsEngine 
+bool OldVal;
+  public:
+SuppressAllDiagnostics(DiagnosticsEngine ) : Diags(Diags) {
+  OldVal = Diags.getSuppressAllDiagnostics();
+  Diags.setSuppressAllDiagnostics(true);
+}
+~SuppressAllDiagnostics() { Diags.setSuppressAllDiagnostics(OldVal); }
+  };
+
 } // end namespace clang
 
 #endif
Index: lib/Parse/ParseTemplate.cpp
===
--- lib/Parse/ParseTemplate.cpp
+++ lib/Parse/ParseTemplate.cpp
@@ -1209,34 +1209,35 @@
 ExprArg.get(), Loc);
 }
 
-/// \brief Determine whether the current tokens can only be parsed as a 
-/// template argument list (starting with the '<') and never as a '<' 
-/// expression.
+/// \brief Determine whether the current tokens (starting with the '<') can be
+/// parsed as a template argument list
 bool Parser::IsTemplateArgumentList(unsigned Skip) {
   struct AlwaysRevertAction : TentativeParsingAction {
 AlwaysRevertAction(Parser ) : TentativeParsingAction(P) { }
 ~AlwaysRevertAction() { Revert(); }
   } Tentative(*this);
-  
+
   while (Skip) {
 ConsumeToken();
 --Skip;
   }
-  
+
   // '<'
   if (!TryConsumeToken(tok::less))
 return false;
 
   // An empty template argument list.
   if (Tok.is(tok::greater))
 return true;
-  
-  // See whether we have declaration specifiers, which indicate a type.
-  while (isCXXDeclarationSpecifier() == TPResult::True)
-ConsumeToken();
-  
-  // If we have a '>' or a ',' then this is a template argument list.
-  return Tok.isOneOf(tok::greater, tok::comma);
+
+  SuppressAllDiagnostics S(Diags);
+  GreaterThanIsOperatorScope G(GreaterThanIsOperator, false);
+  TemplateArgList TemplateArgs;
+  if (ParseTemplateArgumentList(TemplateArgs))
+return false;
+
+  // Closing '>'
+  return Tok.is(tok::greater);
 }
 
 /// ParseTemplateArgumentList - Parse a C++ template-argument-list
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits