[PATCH] D130066: [pseudo] Key guards by RuleID, add guards to literals (and 0).

Thu Jul 21 13:42:55 PDT 2022

sammccall added inline comments.

================
Comment at: clang-tools-extra/pseudo/gen/Main.cpp:102
       llvm::StringRef Name = G.table().AttributeValues[EID];
-      assert(!Name.empty());
       Out.os() << llvm::formatv("EXTENSION({0}, {1})\n", Name, EID);
----------------
hokein wrote:
> I think this invariant is still true even in this patch, ExtensionID for empty attribute value is 0, and we start from 1 in the loop.
Hmm, the assert was firing...

Ah I see what was going on: we were inserting "" at 0 but were inserting it *again* if it was in UniqueAttributeValues, so we had two copies. Fixed.

================
Comment at: clang-tools-extra/pseudo/lib/cxx/CXX.cpp:156

 llvm::DenseMap<ExtensionID, RuleGuard> buildGuards() {
+#define TOKEN_GUARD(kind, cond)                                                \
----------------
hokein wrote:
> we might probably to consider to split it out from the CXX.cpp at some point in the future. I think currently it is fine.
Yeah, I'd like to keep this all lumped together with no public interfaces for now until the patterns are stable - I wouldn't be surprised if we have to refactor these macros a few more times.

================
Comment at: clang-tools-extra/pseudo/lib/cxx/CXX.cpp:171
+      {(RuleID)Rule::non_function_declarator_0declarator,
+       SYMBOL_GUARD(declarator, !isFunctionDeclarator(&N))},
+      {(RuleID)Rule::contextual_override_0identifier,
----------------
hokein wrote:
> nit: add a blank line, I think function-declarator guard is different than the contextual-sensitive guard.
Yeah, good call.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130066