hokein added inline comments.
================
Comment at: clang-tools-extra/pseudo/include/clang-pseudo/Grammar.h:22
//
+// The grammar supports extensions, which have the syntax form of
+// [key=value;key=value]. Extensions are associated with a grammar symbol (
----------------
sammccall wrote:
> Hmm, I think we misunderstood each other...
>
> I think annotations is a good name for the syntactic [foo=bar] bit. I think
> the comment here is now harder to follow, and would prefer the old version.
>
> It's specifically "bar" which indicates some identifier whose semantics will
> be provided externally, so "bar" is a reference to an extension.
>
> If you want to mention the concept of an extension, then maybe at the end...
> `Each unique annotation value creates an extension point. The Override guard
> is implemented in CXX.cpp by binding the ExtensionID for Override to a C++
> function that performs the check.`
oops, I misunderstood your "extensionID" comment completely (sorry). Bring it
back now.
================
Comment at: clang-tools-extra/pseudo/include/clang-pseudo/Grammar.h:213
std::vector<Nonterminal> Nonterminals;
+ // A table of extensions values, sorted by name.
+ // ExtensionID is the index of the table.
----------------
sammccall wrote:
> attribute values, or extension names
>
> (attributes are syntactic and have string values, extensions are semantic and
> can be referred to by names).
renamed to attribute values.
================
Comment at: clang-tools-extra/pseudo/lib/grammar/GrammarBNF.cpp:246
+ auto KV = ExtText.split('=');
+ if (KV.first == ExtText) { // no separator in Line
+ Diagnostics.push_back(
----------------
sammccall wrote:
> If we drop this line, then [foo] will be equivalent to [foo=], i.e. attribute
> is present and points at the empty string. This seems reasonable to me, and
> is a good syntax for boolean attributes (where [foo] denotes set and no
> attribute denotes unset)
This syntax for bool-type attributes seems good to me.
Note that there is a subtle difference with the string-type attributes, where
we use the empty string as the sentinel attribute value (denote unset) whose
ExtensionID is 0. I think it should not be a big issue (we only handle them is
in the bnf parsing time, and propagate field values in `Rule`).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D126536/new/
https://reviews.llvm.org/D126536
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
