<div dir="ltr">On Thu, May 2, 2013 at 4:25 PM, Douglas Gregor <span dir="ltr"><<a href="mailto:dgregor@apple.com" target="_blank" class="cremed">dgregor@apple.com</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: dgregor<br>
Date: Thu May 2 18:25:32 2013<br>
New Revision: 180973<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=180973&view=rev" target="_blank" class="cremed">http://llvm.org/viewvc/llvm-project?rev=180973&view=rev</a><br>
Log:<br>
Use attribute argument information to determine when to parse attribute arguments as expressions.<br>
<br>
This change partly addresses a heinous problem we have with the<br>
parsing of attribute arguments that are a lone identifier. Previously,<br>
we would end up parsing the 'align' attribute of this as an expression<br>
"(Align)":<br>
<br>
template<unsigned Size, unsigned Align><br>
class my_aligned_storage<br>
{<br>
__attribute__((align((Align)))) char storage[Size];<br>
};<br>
<br>
while this would parse as a "parameter name" 'Align':<br>
<br>
template<unsigned Size, unsigned Align><br>
class my_aligned_storage<br>
{<br>
__attribute__((align(Align))) char storage[Size];<br>
};<br>
<br>
The code that handles the alignment attribute would completely ignore<br>
the parameter name, so the while the first of these would do what's<br>
expected, the second would silently be equivalent to<br>
<br>
template<unsigned Size, unsigned Align><br>
class my_aligned_storage<br>
{<br>
__attribute__((align)) char storage[Size];<br>
};<br>
<br>
i.e., use the maximal alignment rather than the specified alignment.<br>
<br>
Address this by sniffing the "Args" provided in the TableGen<br>
description of attributes. If the first argument is "obviously"<br>
something that should be treated as an expression (rather than an<br>
identifier to be matched later), parse it as an expression.<br>
<br>
Fixes <rdar://problem/13700933>.<br>
<br>
<br>
Modified:<br>
cfe/trunk/include/clang/Parse/CMakeLists.txt<br>
cfe/trunk/include/clang/Parse/Makefile<br>
cfe/trunk/lib/Parse/CMakeLists.txt<br>
cfe/trunk/lib/Parse/ParseDecl.cpp<br>
cfe/trunk/lib/Sema/SemaDecl.cpp<br>
cfe/trunk/test/SemaObjC/format-arg-attribute.m<br>
cfe/trunk/test/SemaTemplate/attributes.cpp<br>
cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp<br>
cfe/trunk/utils/TableGen/TableGen.cpp<br>
cfe/trunk/utils/TableGen/TableGenBackends.h<br>
<br>
Modified: cfe/trunk/include/clang/Parse/CMakeLists.txt<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Parse/CMakeLists.txt?rev=180973&r1=180972&r2=180973&view=diff" target="_blank" class="cremed">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Parse/CMakeLists.txt?rev=180973&r1=180972&r2=180973&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/include/clang/Parse/CMakeLists.txt (original)<br>
+++ cfe/trunk/include/clang/Parse/CMakeLists.txt Thu May 2 18:25:32 2013<br>
@@ -1,3 +1,8 @@<br>
+clang_tablegen(AttrExprArgs.inc -gen-clang-attr-expr-args-list<br>
+ -I ${CMAKE_CURRENT_SOURCE_DIR}/../../<br>
+ SOURCE ../Basic/Attr.td<br>
+ TARGET ClangAttrExprArgs)<br>
+<br>
clang_tablegen(AttrLateParsed.inc -gen-clang-attr-late-parsed-list<br>
-I ${CMAKE_CURRENT_SOURCE_DIR}/../../<br>
SOURCE ../Basic/Attr.td<br>
<br>
Modified: cfe/trunk/include/clang/Parse/Makefile<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Parse/Makefile?rev=180973&r1=180972&r2=180973&view=diff" target="_blank" class="cremed">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Parse/Makefile?rev=180973&r1=180972&r2=180973&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/include/clang/Parse/Makefile (original)<br>
+++ cfe/trunk/include/clang/Parse/Makefile Thu May 2 18:25:32 2013<br>
@@ -1,11 +1,17 @@<br>
CLANG_LEVEL := ../../..<br>
TD_SRC_DIR = $(PROJ_SRC_DIR)/../Basic<br>
-BUILT_SOURCES = AttrLateParsed.inc<br>
+BUILT_SOURCES = AttrExprArgs.inc AttrLateParsed.inc<br>
<br>
TABLEGEN_INC_FILES_COMMON = 1<br>
<br>
include $(CLANG_LEVEL)/Makefile<br>
<br>
+$(ObjDir)/AttrExprArgs.inc.tmp : $(TD_SRC_DIR)/Attr.td $(CLANG_TBLGEN) \<br>
+ $(ObjDir)/.dir<br>
+ $(Echo) "Building Clang attribute expression arguments table with tblgen"<br>
+ $(Verb) $(ClangTableGen) -gen-clang-attr-expr-args-list -o $(call SYSPATH, $@) \<br>
+ -I $(PROJ_SRC_DIR)/../../ $<<br>
+<br>
$(ObjDir)/AttrLateParsed.inc.tmp : $(TD_SRC_DIR)/Attr.td $(CLANG_TBLGEN) \<br>
$(ObjDir)/.dir<br>
$(Echo) "Building Clang attribute late-parsed table with tblgen"<br>
<br>
Modified: cfe/trunk/lib/Parse/CMakeLists.txt<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/CMakeLists.txt?rev=180973&r1=180972&r2=180973&view=diff" target="_blank" class="cremed">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/CMakeLists.txt?rev=180973&r1=180972&r2=180973&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Parse/CMakeLists.txt (original)<br>
+++ cfe/trunk/lib/Parse/CMakeLists.txt Thu May 2 18:25:32 2013<br>
@@ -17,6 +17,7 @@ add_clang_library(clangParse<br>
<br>
add_dependencies(clangParse<br>
ClangAttrClasses<br>
+ ClangAttrExprArgs<br>
ClangAttrLateParsed<br>
ClangAttrList<br>
ClangAttrParsedAttrList<br>
<br>
Modified: cfe/trunk/lib/Parse/ParseDecl.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseDecl.cpp?rev=180973&r1=180972&r2=180973&view=diff" target="_blank" class="cremed">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseDecl.cpp?rev=180973&r1=180972&r2=180973&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Parse/ParseDecl.cpp (original)<br>
+++ cfe/trunk/lib/Parse/ParseDecl.cpp Thu May 2 18:25:32 2013<br>
@@ -178,6 +178,12 @@ void Parser::ParseGNUAttributes(ParsedAt<br>
}<br>
}<br>
<br>
+/// \brief Determine whether the given attribute has all expression arguments.<br>
+static bool attributeHasExprArgs(const IdentifierInfo &II) {<br>
+ return llvm::StringSwitch<bool>(II.getName())<br>
+#include "clang/Parse/AttrExprArgs.inc"<br>
+ .Default(false);<br>
+}<br>
<br>
/// Parse the arguments to a parameterized GNU attribute or<br>
/// a C++11 attribute in "gnu" namespace.<br>
@@ -247,6 +253,10 @@ void Parser::ParseGNUAttributeArgs(Ident<br>
TypeParsed = true;<br>
break;<br>
}<br>
+ // If the attribute has all expression arguments, and not a "parameter",<br>
+ // break out to handle it below.<br>
+ if (attributeHasExprArgs(*AttrName))<br>
+ break;<br>
ParmName = Tok.getIdentifierInfo();<br>
ParmLoc = ConsumeToken();<br>
break;<br>
<br>
Modified: cfe/trunk/lib/Sema/SemaDecl.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=180973&r1=180972&r2=180973&view=diff" target="_blank" class="cremed">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=180973&r1=180972&r2=180973&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Sema/SemaDecl.cpp (original)<br>
+++ cfe/trunk/lib/Sema/SemaDecl.cpp Thu May 2 18:25:32 2013<br>
@@ -10633,8 +10633,8 @@ FieldDecl *Sema::CheckFieldDecl(Declarat<br>
// FIXME: We need to pass in the attributes given an AST<br>
// representation, not a parser representation.<br>
if (D) {<br>
- // FIXME: What to pass instead of TUScope?<br>
- ProcessDeclAttributes(TUScope, NewFD, *D);<br>
+ // FIXME: The current scope is almost... but not entirely... correct here.<br>
+ ProcessDeclAttributes(getCurScope(), NewFD, *D);<br>
<br>
if (NewFD->hasAttrs())<br>
CheckAlignasUnderalignment(NewFD);<br>
<br>
Modified: cfe/trunk/test/SemaObjC/format-arg-attribute.m<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/format-arg-attribute.m?rev=180973&r1=180972&r2=180973&view=diff" target="_blank" class="cremed">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/format-arg-attribute.m?rev=180973&r1=180972&r2=180973&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/test/SemaObjC/format-arg-attribute.m (original)<br>
+++ cfe/trunk/test/SemaObjC/format-arg-attribute.m Thu May 2 18:25:32 2013<br>
@@ -14,7 +14,7 @@ union u1 { int i; } __attribute__((forma<br>
enum e1 { E1V0 } __attribute__((format_arg(1))); // expected-warning {{'format_arg' attribute only applies to functions}}<br>
<br>
extern NSString *ff3 (const NSString *) __attribute__((format_arg(3-2)));<br>
-extern NSString *ff4 (const NSString *) __attribute__((format_arg(foo))); // expected-error {{attribute takes one argument}}<br>
+extern NSString *ff4 (const NSString *) __attribute__((format_arg(foo))); // expected-error {{use of undeclared identifier 'foo'}}<br>
<br>
/* format_arg formats must take and return a string. */<br>
extern NSString *fi0 (int) __attribute__((format_arg(1))); // expected-error {{format argument not a string type}}<br>
<br>
Modified: cfe/trunk/test/SemaTemplate/attributes.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaTemplate/attributes.cpp?rev=180973&r1=180972&r2=180973&view=diff" target="_blank" class="cremed">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaTemplate/attributes.cpp?rev=180973&r1=180972&r2=180973&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/test/SemaTemplate/attributes.cpp (original)<br>
+++ cfe/trunk/test/SemaTemplate/attributes.cpp Thu May 2 18:25:32 2013<br>
@@ -1,4 +1,4 @@<br>
-// RUN: %clang_cc1 -fsyntax-only -verify %s<br>
+// RUN: %clang_cc1 -std=gnu++11 -fsyntax-only -verify %s<br>
<br>
namespace attribute_aligned {<br>
template<int N><br>
@@ -18,6 +18,26 @@ namespace attribute_aligned {<br>
check_alignment<2>::t c2;<br>
check_alignment<3>::t c3; // expected-note 2 {{in instantiation}}<br>
check_alignment<4>::t c4;<br>
+<br>
+ template<unsigned Size, unsigned Align><br>
+ class my_aligned_storage<br>
+ {<br>
+ __attribute__((align(Align))) char storage[Size];<br>
+ };<br>
+<br>
+ template<typename T><br>
+ class C {<br>
+ public:<br>
+ C() {<br>
+ static_assert(sizeof(t) == sizeof(T), "my_aligned_storage size wrong");<br>
+ static_assert(alignof(t) == alignof(T), "my_aligned_storage align wrong"); // expected-warning{{'alignof' applied to an expression is a GNU extension}}<br>
+ }<br>
+<br>
+ private:<br>
+ my_aligned_storage<sizeof(T), alignof(T)> t;<br>
+ };<br>
+<br>
+ C<double> cd;<br>
}<br>
<br>
namespace PR9049 {<br>
<br>
Modified: cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp?rev=180973&r1=180972&r2=180973&view=diff" target="_blank" class="cremed">http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp?rev=180973&r1=180972&r2=180973&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp (original)<br>
+++ cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp Thu May 2 18:25:32 2013<br>
@@ -971,6 +971,48 @@ void EmitClangAttrClass(RecordKeeper &Re<br>
OS << "#endif\n";<br>
}<br>
<br>
+// Emits the LateParsed property for attributes.<br></blockquote><div><br></div><div style>I don't believe you ;-)</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+void EmitClangAttrExprArgsList(RecordKeeper &Records, raw_ostream &OS) {<br>
+ emitSourceFileHeader("llvm::StringSwitch code to match attributes with "<br>
+ "expression arguments", OS);<br>
+<br>
+ std::vector<Record*> Attrs = Records.getAllDerivedDefinitions("Attr");<br>
+<br>
+ for (std::vector<Record*>::iterator I = Attrs.begin(), E = Attrs.end();<br>
+ I != E; ++I) {<br>
+ Record &Attr = **I;<br>
+<br>
+ // Determine whether the first argument is something that is always<br>
+ // an expression.<br>
+ std::vector<Record *> Args = Attr.getValueAsListOfDefs("Args");<br>
+ if (Args.empty() || Args[0]->getSuperClasses().empty())<br>
+ continue;<br>
+<br>
+ // Check whether this is one of the argument kinds that implies an<br>
+ // expression.<br>
+ // FIXME: Aligned is weird.<br>
+ if (!llvm::StringSwitch<bool>(Args[0]->getSuperClasses().back()->getName())<br>
+ .Case("AlignedArgument", true)<br>
+ .Case("BoolArgument", true)<br>
+ .Case("DefaultIntArgument", true)<br>
+ .Case("IntArgument", true)<br>
+ .Case("ExprArgument", true)<br>
+ .Case("UnsignedArgument", true)<br>
+ .Case("VariadicUnsignedArgument", true)<br>
+ .Case("VariadicExprArgument", true)<br>
+ .Default(false))<br>
+ continue;<br>
+<br>
+ std::vector<Record*> Spellings = Attr.getValueAsListOfDefs("Spellings");<br>
+<br>
+ for (std::vector<Record*>::const_iterator I = Spellings.begin(),<br>
+ E = Spellings.end(); I != E; ++I) {<br>
+ OS << ".Case(\"" << (*I)->getValueAsString("Name") << "\", "<br>
+ << "true" << ")\n";<br>
+ }<br>
+ }<br>
+}<br>
+<br>
// Emits the class method definitions for attributes.<br>
void EmitClangAttrImpl(RecordKeeper &Records, raw_ostream &OS) {<br>
emitSourceFileHeader("Attribute classes' member function definitions", OS);<br>
<br>
Modified: cfe/trunk/utils/TableGen/TableGen.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/TableGen/TableGen.cpp?rev=180973&r1=180972&r2=180973&view=diff" target="_blank" class="cremed">http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/TableGen/TableGen.cpp?rev=180973&r1=180972&r2=180973&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/utils/TableGen/TableGen.cpp (original)<br>
+++ cfe/trunk/utils/TableGen/TableGen.cpp Thu May 2 18:25:32 2013<br>
@@ -24,6 +24,7 @@ using namespace clang;<br>
<br>
enum ActionType {<br>
GenClangAttrClasses,<br>
+ GenClangAttrExprArgsList,<br>
GenClangAttrImpl,<br>
GenClangAttrList,<br>
GenClangAttrPCHRead,<br>
@@ -62,6 +63,10 @@ namespace {<br>
"Generate option parser implementation"),<br>
clEnumValN(GenClangAttrClasses, "gen-clang-attr-classes",<br>
"Generate clang attribute clases"),<br>
+ clEnumValN(GenClangAttrExprArgsList,<br>
+ "gen-clang-attr-expr-args-list",<br>
+ "Generate a clang attribute expression "<br>
+ "arguments list"),<br>
clEnumValN(GenClangAttrImpl, "gen-clang-attr-impl",<br>
"Generate clang attribute implementations"),<br>
clEnumValN(GenClangAttrList, "gen-clang-attr-list",<br>
@@ -143,6 +148,9 @@ bool ClangTableGenMain(raw_ostream &OS,<br>
case GenClangAttrClasses:<br>
EmitClangAttrClass(Records, OS);<br>
break;<br>
+ case GenClangAttrExprArgsList:<br>
+ EmitClangAttrExprArgsList(Records, OS);<br>
+ break;<br>
case GenClangAttrImpl:<br>
EmitClangAttrImpl(Records, OS);<br>
break;<br>
<br>
Modified: cfe/trunk/utils/TableGen/TableGenBackends.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/TableGen/TableGenBackends.h?rev=180973&r1=180972&r2=180973&view=diff" target="_blank" class="cremed">http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/TableGen/TableGenBackends.h?rev=180973&r1=180972&r2=180973&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/utils/TableGen/TableGenBackends.h (original)<br>
+++ cfe/trunk/utils/TableGen/TableGenBackends.h Thu May 2 18:25:32 2013<br>
@@ -30,6 +30,7 @@ void EmitClangASTNodes(RecordKeeper &RK,<br>
const std::string &N, const std::string &S);<br>
<br>
void EmitClangAttrClass(RecordKeeper &Records, raw_ostream &OS);<br>
+void EmitClangAttrExprArgsList(RecordKeeper &Records, raw_ostream &OS);<br>
void EmitClangAttrImpl(RecordKeeper &Records, raw_ostream &OS);<br>
void EmitClangAttrList(RecordKeeper &Records, raw_ostream &OS);<br>
void EmitClangAttrPCHRead(RecordKeeper &Records, raw_ostream &OS);<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu" class="cremed">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank" class="cremed">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</blockquote></div><br></div></div>