[PATCH] D75563: [clang][Parse] properly parse asm-qualifiers, asm inline

This revision was automatically updated to reflect the committed changes. Closed by commit rG246398ece711: [clang][Parse] properly parse asm-qualifiers, asm inline (authored by nickdesaulniers). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D75563: [clang][Parse] properly parse asm-qualifiers, asm inline

aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75563/new/ https://reviews.llvm.org/D75563 ___ cfe-commits mailing list

[PATCH] D75563: [clang][Parse] properly parse asm-qualifiers, asm inline

nickdesaulniers updated this revision to Diff 249997. nickdesaulniers marked 4 inline comments as done. nickdesaulniers added a comment. - restore order between Diag and SkipUntil Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75563/new/

[PATCH] D75563: [clang][Parse] properly parse asm-qualifiers, asm inline

aaron.ballman added inline comments. Comment at: clang/lib/Parse/ParseStmtAsm.cpp:937 +case AQ_goto: return "goto"; +case AQ_unspecified:; + } nickdesaulniers wrote: > aaron.ballman wrote: > > This looks wrong to me -- it flows through to an unreachable

[PATCH] D75563: [clang][Parse] properly parse asm-qualifiers, asm inline

nickdesaulniers updated this revision to Diff 249760. nickdesaulniers added a comment. - drop `auto` as type is no longer implicit Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75563/new/ https://reviews.llvm.org/D75563 Files:

[PATCH] D75563: [clang][Parse] properly parse asm-qualifiers, asm inline

nickdesaulniers updated this revision to Diff 249754. nickdesaulniers marked 8 inline comments as done. nickdesaulniers added a comment. - address latest review comments, more refactoring Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75563/new/

[PATCH] D75563: [clang][Parse] properly parse asm-qualifiers, asm inline

nickdesaulniers updated this revision to Diff 249757. nickdesaulniers added a comment. - remove isUnspecified(), was partially committed from refactoring I changed my mind on Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75563/new/

[PATCH] D75563: [clang][Parse] properly parse asm-qualifiers, asm inline

nickdesaulniers added inline comments. Comment at: clang/lib/Parse/ParseStmtAsm.cpp:937 +case AQ_goto: return "goto"; +case AQ_unspecified:; + } aaron.ballman wrote: > This looks wrong to me -- it flows through to an unreachable despite being >

[PATCH] D75563: [clang][Parse] properly parse asm-qualifiers, asm inline

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/DiagnosticParseKinds.td:17 + "expected 'volatile', 'inline', 'goto', or '('">, CatInlineAsm; +def err_global_asm_qualifier_ignored : Error< + "meaningless '%0' on asm outside function">, CatInlineAsm;

[PATCH] D75563: [clang][Parse] properly parse asm-qualifiers, asm inline

nickdesaulniers updated this revision to Diff 249412. nickdesaulniers added a comment. This revision is now accepted and ready to land. - small refactorings Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75563/new/ https://reviews.llvm.org/D75563

[PATCH] D75563: [clang][Parse] properly parse asm-qualifiers, asm inline

nickdesaulniers planned changes to this revision. nickdesaulniers marked 3 inline comments as done. nickdesaulniers added a comment. Sorry, got a little trigger happy with the refactoring. Having a weekend to think about this more definitely helps. It's nice to avoid instantiating a whole

[PATCH] D75563: [clang][Parse] properly parse asm-qualifiers, asm inline

nickdesaulniers updated this revision to Diff 249254. nickdesaulniers added a comment. This revision is now accepted and ready to land. - DRY up getting the Qualifier, and checking Tok.is Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75563/new/

[PATCH] D75563: [clang][Parse] properly parse asm-qualifiers, asm inline

nickdesaulniers updated this revision to Diff 249246. nickdesaulniers added a comment. This revision is now accepted and ready to land. - rebase, divorce from DeclSpec Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75563/new/

[PATCH] D75563: [clang][Parse] properly parse asm-qualifiers, asm inline

nickdesaulniers planned changes to this revision. nickdesaulniers added a comment. In D75563#1911375 , @aaron.ballman wrote: > Thank you for working on this, this LGTM! If you wanted a follow-up patch > beyond adding semantic support for the `inline`

[PATCH] D75563: [clang][Parse] properly parse asm-qualifiers, asm inline

nickdesaulniers planned changes to this revision. nickdesaulniers marked an inline comment as done. nickdesaulniers added inline comments. Comment at: clang/lib/Parse/Parser.cpp:1545 +Diag(Tok, diag::err_global_asm_qualifier_ignored) +<<

[PATCH] D75563: [clang][Parse] properly parse asm-qualifiers, asm inline

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. Thank you for working on this, this LGTM! If you wanted a follow-up patch beyond adding semantic support for the `inline` keyword, I think it might make sense to investigate

[PATCH] D75563: [clang][Parse] properly parse asm-qualifiers, asm inline

nickdesaulniers updated this revision to Diff 248827. nickdesaulniers added a comment. - add release note about change in behavior of -Wduplicate-decl-specifier Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75563/new/

[PATCH] D75563: [clang][Parse] properly parse asm-qualifiers, asm inline

nickdesaulniers updated this revision to Diff 248825. nickdesaulniers added a comment. - use better error name - reorder new errors - git-clang-format HEAD~ Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75563/new/ https://reviews.llvm.org/D75563

[PATCH] D75563: [clang][Parse] properly parse asm-qualifiers, asm inline

nickdesaulniers marked an inline comment as done. nickdesaulniers added inline comments. Comment at: clang/include/clang/Basic/DiagnosticParseKinds.td:15 -def warn_asm_qualifier_ignored : Warning< - "ignored %0 qualifier on asm">, CatInlineAsm, InGroup; -def

[PATCH] D75563: [clang][Parse] properly parse asm-qualifiers, asm inline

nickdesaulniers updated this revision to Diff 248816. nickdesaulniers marked an inline comment as done. nickdesaulniers added a comment. Herald added subscribers: jfb, aheejin. - completely drop use of Parse::ParseTypeQualifierListOpt - move paren parsing into helper - fix up test cases for

[PATCH] D75563: [clang][Parse] properly parse asm-qualifiers, asm inline

nickdesaulniers marked 4 inline comments as done. nickdesaulniers added inline comments. Comment at: clang/lib/Parse/ParseStmtAsm.cpp:746-755 ParseTypeQualifierListOpt(DS, AR_VendorAttributesParsed); // GNU asms accept, but warn, about type-qualifiers other than

[PATCH] D75563: [clang][Parse] properly parse asm-qualifiers, asm inline

aaron.ballman added inline comments. Comment at: clang/lib/Parse/ParseStmtAsm.cpp:704 + AQ = DeclSpec::AQ_goto; +else { + if (EndLoc.isValid()) nickdesaulniers wrote: > aaron.ballman wrote: > > I would expect a diagnostic if the unknown token is

[PATCH] D75563: [clang][Parse] properly parse asm-qualifiers, asm inline

nickdesaulniers marked 2 inline comments as done and an inline comment as not done. nickdesaulniers added inline comments. Comment at: clang/lib/Parse/ParseStmtAsm.cpp:725 /// [GNU] gnu-asm-statement: -/// 'asm' type-qualifier[opt] '(' asm-argument ')' ';' +///

[PATCH] D75563: [clang][Parse] properly parse asm-qualifiers, asm inline

nickdesaulniers updated this revision to Diff 248603. nickdesaulniers marked 9 inline comments as done. nickdesaulniers added a comment. - address review comments, add tests for global asm statements. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D75563: [clang][Parse] properly parse asm-qualifiers, asm inline

nickdesaulniers added inline comments. Comment at: clang/lib/Parse/ParseStmtAsm.cpp:704 + AQ = DeclSpec::AQ_goto; +else { + if (EndLoc.isValid()) aaron.ballman wrote: > I would expect a diagnostic if the unknown token is anything other than a >

[PATCH] D75563: [clang][Parse] properly parse asm-qualifiers, asm inline

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/DiagnosticParseKinds.td:32 "'asm goto' cannot have output constraints">; +def err_asm_duplicate_qual : Error<"duplicate asm qualifier %0">; } `duplicate asm qualifier '%0'` (with the

[PATCH] D75563: [clang][Parse] properly parse asm-qualifiers, asm inline

nickdesaulniers marked an inline comment as done. nickdesaulniers added inline comments. Comment at: clang/lib/Parse/ParseStmtAsm.cpp:746-755 ParseTypeQualifierListOpt(DS, AR_VendorAttributesParsed); // GNU asms accept, but warn, about type-qualifiers other than

[PATCH] D75563: [clang][Parse] properly parse asm-qualifiers, asm inline

nickdesaulniers updated this revision to Diff 248226. nickdesaulniers marked an inline comment as done. nickdesaulniers added a comment. - git-clang-format HEAD~ Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75563/new/

[PATCH] D75563: [clang][Parse] properly parse asm-qualifiers, asm inline

nickdesaulniers marked 4 inline comments as done. nickdesaulniers added inline comments. Comment at: clang/test/Parser/asm-qualifiers.c:20 + +void combinations(void) { + asm volatile inline(""); nathanchance wrote: > nickdesaulniers wrote: > > nathanchance

[PATCH] D75563: [clang][Parse] properly parse asm-qualifiers, asm inline

nickdesaulniers updated this revision to Diff 248225. nickdesaulniers added a comment. - combine combinations into permutations test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75563/new/ https://reviews.llvm.org/D75563 Files:

[PATCH] D75563: [clang][Parse] properly parse asm-qualifiers, asm inline

nathanchance added inline comments. Comment at: clang/test/Parser/asm-qualifiers.c:20 + +void combinations(void) { + asm volatile inline(""); nickdesaulniers wrote: > nathanchance wrote: > > I'm probably being dense but what is intended to be tested differently

[PATCH] D75563: [clang][Parse] properly parse asm-qualifiers, asm inline

nickdesaulniers added inline comments. Comment at: clang/test/Parser/asm-qualifiers.c:20 + +void combinations(void) { + asm volatile inline(""); nathanchance wrote: > I'm probably being dense but what is intended to be tested differently > between

[PATCH] D75563: [clang][Parse] properly parse asm-qualifiers, asm inline

nathanchance added inline comments. Comment at: clang/test/Parser/asm-qualifiers.c:20 + +void combinations(void) { + asm volatile inline(""); I'm probably being dense but what is intended to be tested differently between `combinations` and `permutations`? I

[PATCH] D75563: [clang][Parse] properly parse asm-qualifiers, asm inline

nickdesaulniers updated this revision to Diff 248027. nickdesaulniers added a comment. - make sure to initialize GNUAsmQualifiers Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75563/new/ https://reviews.llvm.org/D75563 Files:

[PATCH] D75563: [clang][Parse] properly parse asm-qualifiers, asm inline

nickdesaulniers updated this revision to Diff 248020. nickdesaulniers added a comment. - seems `arc diff` wiped out my previous changes... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75563/new/ https://reviews.llvm.org/D75563 Files:

[PATCH] D75563: [clang][Parse] properly parse asm-qualifiers, asm inline

nickdesaulniers updated this revision to Diff 248019. nickdesaulniers added a comment. - remove impossible case in ParseGNUAsmQualifierListOpt - simplify SetGNUAsmQual - fix typo in comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75563/new/

[PATCH] D75563: [clang][Parse] properly parse asm-qualifiers, asm inline

nickdesaulniers created this revision. nickdesaulniers added a reviewer: aaron.ballman. Herald added a project: clang. Herald added a subscriber: cfe-commits. nickdesaulniers added a comment. I still plan to add CodeGen support for `asm inline` before the next release, which should be as simple

[PATCH] D75563: [clang][Parse] properly parse asm-qualifiers, asm inline

nickdesaulniers added a comment. I still plan to add CodeGen support for `asm inline` before the next release, which should be as simple as emitting an additional `inlinehint`, but I'd like to save that for a follow up patch on top of this. Repository: rG LLVM Github Monorepo CHANGES SINCE