[PATCH] D51265: Headers: fix collisions with .h files of other projects
Qix- added inline comments. Comment at: lib/Headers/stdarg.h:30 #ifndef _VA_LIST +#ifndef _VA_LIST_T typedef __builtin_va_list va_list; Super nit-picky but you could condense this a bit by using ``` #if !defined(_VA_LIST) && !defined(_VA_LIST_T) ``` and a single `#endif` (revert the addition of line 34). It's arguably easier to understand intent instead of adding another level of nesting. Same thing goes for the other two sections. Just a suggestion. Repository: rC Clang https://reviews.llvm.org/D51265 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D32635: [libcxx] regex: fix backreferences in forward assertions
Qix- added a comment. Ping @EricWF - few years but this is still an issue, rendering ECMAscript regex backreferences almost entirely broken in libcxx :/ Would be great to get a champion for it. OP has indicated on Github that they'd be happy to rebase. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D32635/new/ https://reviews.llvm.org/D32635 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D99877: [Clang] Allow processing of attributes on statements by plugins
Qix- abandoned this revision. Qix- added a comment. Closing in favor of more complete plugin attribute changes discussed in IRC. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99877/new/ https://reviews.llvm.org/D99877 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D99861: [Clang] Record tokens in attribute arguments for user-defined C++/C2x attributes
Qix- abandoned this revision. Qix- added a comment. Closing in favor of more complete plugin attribute changes discussed in IRC. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99861/new/ https://reviews.llvm.org/D99861 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D99861: [Clang] Record tokens in attribute arguments for user-defined C++/C2x attributes
Qix- created this revision.
Qix- added a reviewer: aaron.ballman.
Qix- added a project: clang.
Herald added a subscriber: mgorny.
Qix- requested review of this revision.
Herald added a subscriber: cfe-commits.
Currently, user-defined attributes (e.g. those registered via Clang plugins)
don't hold much information or allow for much to be done with them, as their
argument tokens are discarded entirely in almost all cases.
However, since they are quite flexible with their syntax (pretty much anything
can go in there), it would be massively helpful if plugins could be handed the
pre-processed and parsed tokens as a listed to be consumed by third-party
plugins and the like.
This diff creates a trailing data list for `ParsedAttr` objects and places the
"recorded" tokens there from the lexer. I would have piggy-backed off of the
normal backtrack system but unfortunately it doesn't call the handler for
tokens being replayed from the backtrack cache, which in many cases we *do*
care about. So I had to create a second recording callback.
The new plugin directory shows how this would be used from a plugin.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D99861
Files:
clang/examples/CMakeLists.txt
clang/examples/PrintAttributeTokens/CMakeLists.txt
clang/examples/PrintAttributeTokens/PrintAttributeTokens.cpp
clang/examples/PrintAttributeTokens/PrintAttributeTokens.exports
clang/examples/PrintAttributeTokens/README.txt
clang/include/clang/Lex/Preprocessor.h
clang/include/clang/Sema/ParsedAttr.h
clang/lib/Lex/Preprocessor.cpp
clang/lib/Parse/ParseDecl.cpp
clang/lib/Parse/ParseDeclCXX.cpp
clang/lib/Sema/ParsedAttr.cpp
clang/test/Frontend/plugin-print-attr-tokens.cpp
Index: clang/test/Frontend/plugin-print-attr-tokens.cpp
===
--- /dev/null
+++ clang/test/Frontend/plugin-print-attr-tokens.cpp
@@ -0,0 +1,12 @@
+// RUN: %clang -cc1 -load %llvmshlibdir/PrintAttributeTokens%pluginext -fsyntax-only -ast-dump -verify %s
+// REQUIRES: plugins, examples
+
+// expected-no-diagnostics
+[[print_tokens(
+the, mitochondria,
+, Of::The($cell))]] void
+fn1a() {}
+[[plugin::print_tokens("a string")]] void fn1b() {}
+[[plugin::print_tokens()]] void fn1c() {}
+[[plugin::print_tokens(some_ident)]] void fn1d() {}
+[[plugin::print_tokens(int)]] void fn1e() {}
Index: clang/lib/Sema/ParsedAttr.cpp
===
--- clang/lib/Sema/ParsedAttr.cpp
+++ clang/lib/Sema/ParsedAttr.cpp
@@ -45,10 +45,11 @@
else if (HasParsedType)
return totalSizeToAlloc(0, 0, 0, 1, 0);
+detail::PropertyData, Token>(0, 0, 0, 1, 0, 0);
return totalSizeToAlloc(NumArgs, 0, 0, 0, 0);
+ detail::PropertyData, Token>(NumArgs, 0, 0, 0, 0,
+ NumTokens);
}
AttributeFactory::AttributeFactory() {
Index: clang/lib/Parse/ParseDeclCXX.cpp
===
--- clang/lib/Parse/ParseDeclCXX.cpp
+++ clang/lib/Parse/ParseDeclCXX.cpp
@@ -4096,13 +4096,39 @@
LO.CPlusPlus ? ParsedAttr::AS_CXX11 : ParsedAttr::AS_C2x;
// If the attribute isn't known, we will not attempt to parse any
- // arguments.
+ // arguments. Instead, we just record the tokens and add the attribute
+ // directly. The recording happens here because this is the only place
+ // where user-defined (via plugins) attributes are parsed, and thus
+ // they care about the token stream directly.
if (!hasAttribute(LO.CPlusPlus ? AttrSyntax::CXX : AttrSyntax::C, ScopeName,
AttrName, getTargetInfo(), getLangOpts())) {
-// Eat the left paren, then skip to the ending right paren.
+// Begin recording session.
+SmallVector RecordedTokens;
+assert(!PP.hasTokenRecorder());
+PP.setTokenRecorder(
+[&RecordedTokens](const Token &Tok) { RecordedTokens.push_back(Tok); });
+
+// Eat the left paren.
ConsumeParen();
+
+// skip to the ending right paren.
SkipUntil(tok::r_paren);
-return false;
+
+// End recording session.
+PP.setTokenRecorder(nullptr);
+
+// Add new attribute with the token list.
+// We assert that we have at least one token,
+// since we have to ignore the final r_paren.
+assert(RecordedTokens.size() > 0);
+Attrs.addNew(
+AttrName,
+SourceRange(ScopeLoc.isValid() ? ScopeLoc : AttrNameLoc, AttrNameLoc),
+ScopeName, ScopeLoc, nullptr, 0,
+getLangOpts().CPlusPlus ? ParsedAttr::AS_CXX11 : ParsedAttr::AS_C2x,
+RecordedTokens.data(), RecordedTokens.size() - 2);
+
+return true;
}
if (ScopeName && (ScopeName->isStr("gnu") || ScopeName->isStr("__gnu__"))) {
Index: clang/lib/Parse/ParseDecl.cpp
===
--- clang/lib/Parse/ParseDecl.cpp
+++ clang/lib/Parse/ParseDecl.cpp
[PATCH] D99877: [Clang] Allow processing of attributes on statements by plugins
Qix- created this revision.
Qix- added a reviewer: aaron.ballman.
Qix- added a project: clang.
Qix- requested review of this revision.
Herald added a subscriber: cfe-commits.
Pretty cut and dry; currently plugins can create attributes for declarations
but any that are not recognized produce a diagnostic, leaving no hook for
plugins to react to them.
This adds a quick check to the attribute to see if the implementation would
like to handle it first, akin to how declarations are handled.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D99877
Files:
clang/include/clang/Sema/ParsedAttr.h
clang/lib/Sema/SemaStmtAttr.cpp
Index: clang/lib/Sema/SemaStmtAttr.cpp
===
--- clang/lib/Sema/SemaStmtAttr.cpp
+++ clang/lib/Sema/SemaStmtAttr.cpp
@@ -430,11 +430,12 @@
case ParsedAttr::AT_Unlikely:
return handleUnlikely(S, St, A, Range);
default:
-// N.B., ClangAttrEmitter.cpp emits a diagnostic helper that ensures a
-// declaration attribute is not written on a statement, but this code is
-// needed for attributes in Attr.td that do not list any subjects.
-S.Diag(A.getRange().getBegin(), diag::err_decl_attribute_invalid_on_stmt)
-<< A << St->getBeginLoc();
+if (A.getInfo().handleStmtAttribute(S, St, A) ==
ParsedAttrInfo::NotHandled)
+ // N.B., ClangAttrEmitter.cpp emits a diagnostic helper that ensures a
+ // declaration attribute is not written on a statement, but this code is
+ // needed for attributes in Attr.td that do not list any subjects.
+ S.Diag(A.getRange().getBegin(), diag::err_decl_attribute_invalid_on_stmt)
+ << A << St->getBeginLoc();
return nullptr;
}
}
Index: clang/include/clang/Sema/ParsedAttr.h
===
--- clang/include/clang/Sema/ParsedAttr.h
+++ clang/include/clang/Sema/ParsedAttr.h
@@ -117,6 +117,13 @@
const ParsedAttr &Attr) const {
return NotHandled;
}
+ /// If this ParsedAttrInfo knows how to handle this ParsedAttr applied to
this
+ /// Stmt then do so and return either AttributeApplied if it was applied or
+ /// AttributeNotApplied if it wasn't. Otherwise return NotHandled.
+ virtual AttrHandling handleStmtAttribute(Sema &S, Stmt *St,
+ const ParsedAttr &Attr) const {
+return NotHandled;
+ }
static const ParsedAttrInfo &get(const AttributeCommonInfo &A);
};
Index: clang/lib/Sema/SemaStmtAttr.cpp
===
--- clang/lib/Sema/SemaStmtAttr.cpp
+++ clang/lib/Sema/SemaStmtAttr.cpp
@@ -430,11 +430,12 @@
case ParsedAttr::AT_Unlikely:
return handleUnlikely(S, St, A, Range);
default:
-// N.B., ClangAttrEmitter.cpp emits a diagnostic helper that ensures a
-// declaration attribute is not written on a statement, but this code is
-// needed for attributes in Attr.td that do not list any subjects.
-S.Diag(A.getRange().getBegin(), diag::err_decl_attribute_invalid_on_stmt)
-<< A << St->getBeginLoc();
+if (A.getInfo().handleStmtAttribute(S, St, A) == ParsedAttrInfo::NotHandled)
+ // N.B., ClangAttrEmitter.cpp emits a diagnostic helper that ensures a
+ // declaration attribute is not written on a statement, but this code is
+ // needed for attributes in Attr.td that do not list any subjects.
+ S.Diag(A.getRange().getBegin(), diag::err_decl_attribute_invalid_on_stmt)
+ << A << St->getBeginLoc();
return nullptr;
}
}
Index: clang/include/clang/Sema/ParsedAttr.h
===
--- clang/include/clang/Sema/ParsedAttr.h
+++ clang/include/clang/Sema/ParsedAttr.h
@@ -117,6 +117,13 @@
const ParsedAttr &Attr) const {
return NotHandled;
}
+ /// If this ParsedAttrInfo knows how to handle this ParsedAttr applied to this
+ /// Stmt then do so and return either AttributeApplied if it was applied or
+ /// AttributeNotApplied if it wasn't. Otherwise return NotHandled.
+ virtual AttrHandling handleStmtAttribute(Sema &S, Stmt *St,
+ const ParsedAttr &Attr) const {
+return NotHandled;
+ }
static const ParsedAttrInfo &get(const AttributeCommonInfo &A);
};
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D99877: [Clang] Allow processing of attributes on statements by plugins
Qix- updated this revision to Diff 336172.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D99877/new/
https://reviews.llvm.org/D99877
Files:
clang/include/clang/Sema/ParsedAttr.h
clang/lib/Sema/SemaStmtAttr.cpp
Index: clang/lib/Sema/SemaStmtAttr.cpp
===
--- clang/lib/Sema/SemaStmtAttr.cpp
+++ clang/lib/Sema/SemaStmtAttr.cpp
@@ -430,11 +430,12 @@
case ParsedAttr::AT_Unlikely:
return handleUnlikely(S, St, A, Range);
default:
-// N.B., ClangAttrEmitter.cpp emits a diagnostic helper that ensures a
-// declaration attribute is not written on a statement, but this code is
-// needed for attributes in Attr.td that do not list any subjects.
-S.Diag(A.getRange().getBegin(), diag::err_decl_attribute_invalid_on_stmt)
-<< A << St->getBeginLoc();
+if (A.getInfo().handleStmtAttribute(S, St, A) ==
ParsedAttrInfo::NotHandled)
+ // N.B., ClangAttrEmitter.cpp emits a diagnostic helper that ensures a
+ // declaration attribute is not written on a statement, but this code is
+ // needed for attributes in Attr.td that do not list any subjects.
+ S.Diag(A.getRange().getBegin(), diag::err_decl_attribute_invalid_on_stmt)
+ << A << St->getBeginLoc();
return nullptr;
}
}
Index: clang/include/clang/Sema/ParsedAttr.h
===
--- clang/include/clang/Sema/ParsedAttr.h
+++ clang/include/clang/Sema/ParsedAttr.h
@@ -117,6 +117,13 @@
const ParsedAttr &Attr) const {
return NotHandled;
}
+ /// If this ParsedAttrInfo knows how to handle this ParsedAttr applied to
this
+ /// Stmt then do so and return either AttributeApplied if it was applied or
+ /// AttributeNotApplied if it wasn't. Otherwise return NotHandled.
+ virtual AttrHandling handleStmtAttribute(Sema &S, Stmt *St,
+ const ParsedAttr &Attr) const {
+return NotHandled;
+ }
static const ParsedAttrInfo &get(const AttributeCommonInfo &A);
};
Index: clang/lib/Sema/SemaStmtAttr.cpp
===
--- clang/lib/Sema/SemaStmtAttr.cpp
+++ clang/lib/Sema/SemaStmtAttr.cpp
@@ -430,11 +430,12 @@
case ParsedAttr::AT_Unlikely:
return handleUnlikely(S, St, A, Range);
default:
-// N.B., ClangAttrEmitter.cpp emits a diagnostic helper that ensures a
-// declaration attribute is not written on a statement, but this code is
-// needed for attributes in Attr.td that do not list any subjects.
-S.Diag(A.getRange().getBegin(), diag::err_decl_attribute_invalid_on_stmt)
-<< A << St->getBeginLoc();
+if (A.getInfo().handleStmtAttribute(S, St, A) == ParsedAttrInfo::NotHandled)
+ // N.B., ClangAttrEmitter.cpp emits a diagnostic helper that ensures a
+ // declaration attribute is not written on a statement, but this code is
+ // needed for attributes in Attr.td that do not list any subjects.
+ S.Diag(A.getRange().getBegin(), diag::err_decl_attribute_invalid_on_stmt)
+ << A << St->getBeginLoc();
return nullptr;
}
}
Index: clang/include/clang/Sema/ParsedAttr.h
===
--- clang/include/clang/Sema/ParsedAttr.h
+++ clang/include/clang/Sema/ParsedAttr.h
@@ -117,6 +117,13 @@
const ParsedAttr &Attr) const {
return NotHandled;
}
+ /// If this ParsedAttrInfo knows how to handle this ParsedAttr applied to this
+ /// Stmt then do so and return either AttributeApplied if it was applied or
+ /// AttributeNotApplied if it wasn't. Otherwise return NotHandled.
+ virtual AttrHandling handleStmtAttribute(Sema &S, Stmt *St,
+ const ParsedAttr &Attr) const {
+return NotHandled;
+ }
static const ParsedAttrInfo &get(const AttributeCommonInfo &A);
};
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D99861: [Clang] Record tokens in attribute arguments for user-defined C++/C2x attributes
Qix- updated this revision to Diff 336173.
Qix- added a comment.
Updated the diff to include a lot more context (-U). Thanks again for the
tip :)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D99861/new/
https://reviews.llvm.org/D99861
Files:
clang/examples/CMakeLists.txt
clang/examples/PrintAttributeTokens/CMakeLists.txt
clang/examples/PrintAttributeTokens/PrintAttributeTokens.cpp
clang/examples/PrintAttributeTokens/PrintAttributeTokens.exports
clang/examples/PrintAttributeTokens/README.txt
clang/include/clang/Lex/Preprocessor.h
clang/include/clang/Sema/ParsedAttr.h
clang/lib/Lex/Preprocessor.cpp
clang/lib/Parse/ParseDecl.cpp
clang/lib/Parse/ParseDeclCXX.cpp
clang/lib/Sema/ParsedAttr.cpp
clang/test/Frontend/plugin-print-attr-tokens.cpp
Index: clang/test/Frontend/plugin-print-attr-tokens.cpp
===
--- /dev/null
+++ clang/test/Frontend/plugin-print-attr-tokens.cpp
@@ -0,0 +1,12 @@
+// RUN: %clang -cc1 -load %llvmshlibdir/PrintAttributeTokens%pluginext -fsyntax-only -ast-dump -verify %s
+// REQUIRES: plugins, examples
+
+// expected-no-diagnostics
+[[print_tokens(
+the, mitochondria,
+, Of::The($cell))]] void
+fn1a() {}
+[[plugin::print_tokens("a string")]] void fn1b() {}
+[[plugin::print_tokens()]] void fn1c() {}
+[[plugin::print_tokens(some_ident)]] void fn1d() {}
+[[plugin::print_tokens(int)]] void fn1e() {}
Index: clang/lib/Sema/ParsedAttr.cpp
===
--- clang/lib/Sema/ParsedAttr.cpp
+++ clang/lib/Sema/ParsedAttr.cpp
@@ -45,10 +45,11 @@
else if (HasParsedType)
return totalSizeToAlloc(0, 0, 0, 1, 0);
+detail::PropertyData, Token>(0, 0, 0, 1, 0, 0);
return totalSizeToAlloc(NumArgs, 0, 0, 0, 0);
+ detail::PropertyData, Token>(NumArgs, 0, 0, 0, 0,
+ NumTokens);
}
AttributeFactory::AttributeFactory() {
Index: clang/lib/Parse/ParseDeclCXX.cpp
===
--- clang/lib/Parse/ParseDeclCXX.cpp
+++ clang/lib/Parse/ParseDeclCXX.cpp
@@ -4096,13 +4096,39 @@
LO.CPlusPlus ? ParsedAttr::AS_CXX11 : ParsedAttr::AS_C2x;
// If the attribute isn't known, we will not attempt to parse any
- // arguments.
+ // arguments. Instead, we just record the tokens and add the attribute
+ // directly. The recording happens here because this is the only place
+ // where user-defined (via plugins) attributes are parsed, and thus
+ // they care about the token stream directly.
if (!hasAttribute(LO.CPlusPlus ? AttrSyntax::CXX : AttrSyntax::C, ScopeName,
AttrName, getTargetInfo(), getLangOpts())) {
-// Eat the left paren, then skip to the ending right paren.
+// Begin recording session.
+SmallVector RecordedTokens;
+assert(!PP.hasTokenRecorder());
+PP.setTokenRecorder(
+[&RecordedTokens](const Token &Tok) { RecordedTokens.push_back(Tok); });
+
+// Eat the left paren.
ConsumeParen();
+
+// skip to the ending right paren.
SkipUntil(tok::r_paren);
-return false;
+
+// End recording session.
+PP.setTokenRecorder(nullptr);
+
+// Add new attribute with the token list.
+// We assert that we have at least one token,
+// since we have to ignore the final r_paren.
+assert(RecordedTokens.size() > 0);
+Attrs.addNew(
+AttrName,
+SourceRange(ScopeLoc.isValid() ? ScopeLoc : AttrNameLoc, AttrNameLoc),
+ScopeName, ScopeLoc, nullptr, 0,
+getLangOpts().CPlusPlus ? ParsedAttr::AS_CXX11 : ParsedAttr::AS_C2x,
+RecordedTokens.data(), RecordedTokens.size() - 2);
+
+return true;
}
if (ScopeName && (ScopeName->isStr("gnu") || ScopeName->isStr("__gnu__"))) {
Index: clang/lib/Parse/ParseDecl.cpp
===
--- clang/lib/Parse/ParseDecl.cpp
+++ clang/lib/Parse/ParseDecl.cpp
@@ -2828,7 +2828,7 @@
ArgsVector ArgExprs;
ArgExprs.push_back(ArgExpr.get());
Attrs.addNew(KWName, KWLoc, nullptr, KWLoc, ArgExprs.data(), 1,
- ParsedAttr::AS_Keyword, EllipsisLoc);
+ ParsedAttr::AS_Keyword, nullptr, 0, EllipsisLoc);
}
ExprResult Parser::ParseExtIntegerArgument() {
Index: clang/lib/Lex/Preprocessor.cpp
===
--- clang/lib/Lex/Preprocessor.cpp
+++ clang/lib/Lex/Preprocessor.cpp
@@ -970,6 +970,9 @@
if (OnToken)
OnToken(Result);
}
+
+ if (OnRecordedToken)
+OnRecordedToken(Result);
}
/// Lex a header-name token (including one formed from header-name-tokens if
Index: clang/include/clang/Sema/ParsedAttr.h
===
--- clang/include/clang/Sema/ParsedAttr.h
+++ clang/include/clang/Sema/ParsedAttr.h
@@ -1
[PATCH] D99861: [Clang] Record tokens in attribute arguments for user-defined C++/C2x attributes
Qix- planned changes to this revision.
Qix- marked 5 inline comments as done.
Qix- added inline comments.
Comment at: clang/examples/PrintAttributeTokens/CMakeLists.txt:3-10
+if( NOT MSVC ) # MSVC mangles symbols differently, and
+ # PrintAttributeTokens.export contains C++ symbols.
+ if( NOT LLVM_REQUIRES_RTTI )
+if( NOT LLVM_REQUIRES_EH )
+ set(LLVM_EXPORTED_SYMBOL_FILE
${CMAKE_CURRENT_SOURCE_DIR}/PrintAttributeTokens.exports)
+endif()
+ endif()
aaron.ballman wrote:
> I'm not certain what this is actually doing -- the .exports file is empty
> (and no other plugin has this sort of .exports file thing). Is this needed?
Most likely not - the directory was copied from the PrintFunctionNames
directory, which also has an empty exports file. I'm not completely familiar
with the LLVM build configs so I figured it was required. Happy to remove it if
need be :)
Comment at: clang/examples/PrintAttributeTokens/PrintAttributeTokens.cpp:37
+ const ParsedAttr &Attr) const override {
+llvm::errs() << "PrintAttributeTokens ---\n";
+D->dump(llvm::errs());
aaron.ballman wrote:
> Should probably use `llvm::outs()` instead (here and elsewhere in the plugin).
Sure thing, though should PrintFunctionNames be updated as well? Mostly copied
the conventions over from that.
Comment at: clang/examples/PrintAttributeTokens/PrintAttributeTokens.cpp:48-50
+if (tokens[i].isLiteral()) {
+ llvm::errs() << "\t=" << tokens[i].getLiteralData();
+}
aaron.ballman wrote:
> It'd probably be handy to also print identifier tokens.
Yeah I caught that the other day too, thanks for reminding me.
Comment at: clang/include/clang/Sema/ParsedAttr.h:284
+ Token *getTokensBuffer() { return getTrailingObjects(); }
+ Token const *getTokensBuffer() const { return getTrailingObjects(); }
+
aaron.ballman wrote:
>
Agreed; should 279 be updated too?
Comment at: clang/include/clang/Sema/ParsedAttr.h:486
+ const Token *getTokens() const {
+assert(NumTokens > 0 && "No Tokens to retrieve!");
+return getTokensBuffer();
aaron.ballman wrote:
> I think it'd be reasonable to return `nullptr` in this case, WDYT?
Sure, that's fine. I was trying to think why I did the assertion but nullptr
seems fine too.
Comment at: clang/lib/Parse/ParseDeclCXX.cpp:4100-4102
+ // directly. The recording happens here because this is the only place
+ // where user-defined (via plugins) attributes are parsed, and thus
+ // they care about the token stream directly.
aaron.ballman wrote:
> I think plugins will expect these tokens to be available regardless of which
> attribute syntax is used, so you may need to look into also doing work for
> GNU and declspec attribute argument lists.
As far as I understand (and perhaps poorly communicated in the comment) is that
plugin-defined attributes will always hit this codepath as opposed to the
built-in attribute parsers.
I could be wrong here, though. Are arbitrary tokens even allowed in
GNU/Declspec attributes? I thought it was just the C2x/C++ attributes syntax
that allowed non-identifier tokens in the first place.
Either way, from what I could tell (trying a few different implementations of
this change), this is the only place where user-defined attributes are parsed.
I certainly could be missing something though.
Comment at: clang/lib/Parse/ParseDeclCXX.cpp:4121-4122
+// Add new attribute with the token list.
+// We assert that we have at least one token,
+// since we have to ignore the final r_paren.
+assert(RecordedTokens.size() > 0);
aaron.ballman wrote:
> To be clear, we should have at least *two* tokens, right? One for the left
> paren and one for the right?
This was confusing for me too at first.
By 4112, the left paren token has already been emitted by the lexer;
`ConsumeParen` advances to the next token *after* the left-paren; _that_ token
then becomes the first in the recording session.
Further, `SkipUntil(tok::r_paren)` checks if the current token is an `r_paren`,
and if not, advances and then starts over. It doesn't advance _then_ check.
It's the presence of the left paren that advances us to this stage of the
parsing in the first place and thus is emitted before our recording session
starts. An empty argument list (`[[some_attr()]]`) thus only has an `r_paren`
token emitted to the recording session, which is then ignored.
This is the existing design of the lexer it seems - I wasn't expecting the
"reversed" order of operations, and the names of the lexer methods didn't help,
either :)
Comment at: clang/test/Frontend/plugin-print-attr-tokens.cpp:9-12
+[[plugin::print_tokens("a string")]] void fn1b()
[PATCH] D99861: [Clang] Record tokens in attribute arguments for user-defined C++/C2x attributes
Qix- marked 3 inline comments as done. Qix- added a comment. So I went back and checked and I remember why I didn't add explicit support for GNU/declspec attributes - they actually perform symbol lookups in the surrounding scope. I believe there's an issue right now with plugins that GNU-style attributes parser assumes that they're being parsed from the Tablegen stuff and thus expect the attribute classes to be parameterized as e.g. taking arguments or not. Therefore, putting arbitrary tokens in GNU-style and (as far as I can tell) declspec-style attributes is expressely not supported by the compiler. Also I believe this makes sense, too, because those syntaxes aren't spec'd to allow arbitrary tokens like C2x and C++ attributes are. In fact, in my testing, the GNU-style attributes in plugins can't accept arguments whatsoever without getting some sort of compilation error. Maybe I'm not using the plugin system correctly, though. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99861/new/ https://reviews.llvm.org/D99861 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D99861: [Clang] Record tokens in attribute arguments for user-defined C++/C2x attributes
Qix- marked an inline comment as not done. Qix- added inline comments. Comment at: clang/lib/Parse/ParseDeclCXX.cpp:4100-4102 + // directly. The recording happens here because this is the only place + // where user-defined (via plugins) attributes are parsed, and thus + // they care about the token stream directly. Qix- wrote: > aaron.ballman wrote: > > I think plugins will expect these tokens to be available regardless of > > which attribute syntax is used, so you may need to look into also doing > > work for GNU and declspec attribute argument lists. > As far as I understand (and perhaps poorly communicated in the comment) is > that plugin-defined attributes will always hit this codepath as opposed to > the built-in attribute parsers. > > I could be wrong here, though. Are arbitrary tokens even allowed in > GNU/Declspec attributes? I thought it was just the C2x/C++ attributes syntax > that allowed non-identifier tokens in the first place. > > Either way, from what I could tell (trying a few different implementations of > this change), this is the only place where user-defined attributes are > parsed. I certainly could be missing something though. See my latest top-level comment. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99861/new/ https://reviews.llvm.org/D99861 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D99861: [Clang] Record tokens in attribute arguments for user-defined C++/C2x attributes
Qix- updated this revision to Diff 336614.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D99861/new/
https://reviews.llvm.org/D99861
Files:
clang/examples/CMakeLists.txt
clang/examples/PrintAttributeTokens/CMakeLists.txt
clang/examples/PrintAttributeTokens/PrintAttributeTokens.cpp
clang/examples/PrintAttributeTokens/PrintAttributeTokens.exports
clang/examples/PrintAttributeTokens/README.txt
clang/include/clang/Lex/Preprocessor.h
clang/include/clang/Sema/ParsedAttr.h
clang/lib/Lex/Preprocessor.cpp
clang/lib/Parse/ParseDecl.cpp
clang/lib/Parse/ParseDeclCXX.cpp
clang/lib/Sema/ParsedAttr.cpp
clang/test/Frontend/plugin-print-attr-tokens.cpp
Index: clang/test/Frontend/plugin-print-attr-tokens.cpp
===
--- /dev/null
+++ clang/test/Frontend/plugin-print-attr-tokens.cpp
@@ -0,0 +1,12 @@
+// RUN: %clang -cc1 -load %llvmshlibdir/PrintAttributeTokens%pluginext -fsyntax-only -ast-dump -verify %s
+// REQUIRES: plugins, examples
+
+// expected-no-diagnostics
+[[print_tokens(
+the, mitochondria,
+, Of::The($cell))]] void
+fn1a() {}
+[[plugin::print_tokens("a string")]] void fn1b() {}
+[[plugin::print_tokens()]] void fn1c() {}
+[[plugin::print_tokens(some_ident)]] void fn1d() {}
+[[plugin::print_tokens(int)]] void fn1e() {}
Index: clang/lib/Sema/ParsedAttr.cpp
===
--- clang/lib/Sema/ParsedAttr.cpp
+++ clang/lib/Sema/ParsedAttr.cpp
@@ -45,10 +45,11 @@
else if (HasParsedType)
return totalSizeToAlloc(0, 0, 0, 1, 0);
+detail::PropertyData, Token>(0, 0, 0, 1, 0, 0);
return totalSizeToAlloc(NumArgs, 0, 0, 0, 0);
+ detail::PropertyData, Token>(NumArgs, 0, 0, 0, 0,
+ NumTokens);
}
AttributeFactory::AttributeFactory() {
Index: clang/lib/Parse/ParseDeclCXX.cpp
===
--- clang/lib/Parse/ParseDeclCXX.cpp
+++ clang/lib/Parse/ParseDeclCXX.cpp
@@ -4096,13 +4096,39 @@
LO.CPlusPlus ? ParsedAttr::AS_CXX11 : ParsedAttr::AS_C2x;
// If the attribute isn't known, we will not attempt to parse any
- // arguments.
+ // arguments. Instead, we just record the tokens and add the attribute
+ // directly. The recording happens here because this is the only place
+ // where user-defined (via plugins) attributes are parsed, and thus
+ // they care about the token stream directly.
if (!hasAttribute(LO.CPlusPlus ? AttrSyntax::CXX : AttrSyntax::C, ScopeName,
AttrName, getTargetInfo(), getLangOpts())) {
-// Eat the left paren, then skip to the ending right paren.
+// Begin recording session.
+SmallVector RecordedTokens;
+assert(!PP.hasTokenRecorder());
+PP.setTokenRecorder(
+[&RecordedTokens](const Token &Tok) { RecordedTokens.push_back(Tok); });
+
+// Eat the left paren.
ConsumeParen();
+
+// skip to the ending right paren.
SkipUntil(tok::r_paren);
-return false;
+
+// End recording session.
+PP.setTokenRecorder(nullptr);
+
+// Add new attribute with the token list.
+// We assert that we have at least one token,
+// since we have to ignore the final r_paren.
+assert(RecordedTokens.size() > 0);
+Attrs.addNew(
+AttrName,
+SourceRange(ScopeLoc.isValid() ? ScopeLoc : AttrNameLoc, AttrNameLoc),
+ScopeName, ScopeLoc, nullptr, 0,
+getLangOpts().CPlusPlus ? ParsedAttr::AS_CXX11 : ParsedAttr::AS_C2x,
+RecordedTokens.data(), RecordedTokens.size() - 2);
+
+return true;
}
if (ScopeName && (ScopeName->isStr("gnu") || ScopeName->isStr("__gnu__"))) {
Index: clang/lib/Parse/ParseDecl.cpp
===
--- clang/lib/Parse/ParseDecl.cpp
+++ clang/lib/Parse/ParseDecl.cpp
@@ -2828,7 +2828,7 @@
ArgsVector ArgExprs;
ArgExprs.push_back(ArgExpr.get());
Attrs.addNew(KWName, KWLoc, nullptr, KWLoc, ArgExprs.data(), 1,
- ParsedAttr::AS_Keyword, EllipsisLoc);
+ ParsedAttr::AS_Keyword, nullptr, 0, EllipsisLoc);
}
ExprResult Parser::ParseExtIntegerArgument() {
Index: clang/lib/Lex/Preprocessor.cpp
===
--- clang/lib/Lex/Preprocessor.cpp
+++ clang/lib/Lex/Preprocessor.cpp
@@ -970,6 +970,9 @@
if (OnToken)
OnToken(Result);
}
+
+ if (OnRecordedToken)
+OnRecordedToken(Result);
}
/// Lex a header-name token (including one formed from header-name-tokens if
Index: clang/include/clang/Sema/ParsedAttr.h
===
--- clang/include/clang/Sema/ParsedAttr.h
+++ clang/include/clang/Sema/ParsedAttr.h
@@ -18,6 +18,7 @@
#include "clang/Basic/AttributeCommonInfo.h"
#include "clang/Basic/Diagnostic.h"
#include "cla
[PATCH] D99861: [Clang] Record tokens in attribute arguments for user-defined C++/C2x attributes
Qix- added a comment. @aaron.ballman updated with comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99861/new/ https://reviews.llvm.org/D99861 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D99861: [Clang] Record tokens in attribute arguments for user-defined C++/C2x attributes
Qix- added a comment. I'm not sure exactly how to continue after the last few comments - what should the approach be for this patch? Or are these things we can shoot for in later patches? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99861/new/ https://reviews.llvm.org/D99861 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
