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

Nathan Chancellor via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Mar 3 16:09:50 PST 2020


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 between `combinations` and `permutations`? I assume the order of the qualifiers? Wouldn't it just be better to merge `combinations` into `permutations` or was there some deeper reasoning for the compartmentalization?
> `combinations` tests a combination of different `asm-qualifiers` together. `permutations` are just permutations of the combinations that have not been tested above. I may not even have my nomenclature correct.  Shall I combine them?
I assume that you want permutations since you want to make sure that the ordering does not matter, right? If you just care about combinations then

```
  asm inline goto volatile("" ::::foo);
  asm inline volatile goto("" ::::foo);

  asm goto inline volatile("" ::::foo);
  asm goto volatile inline("" ::::foo);

  asm volatile goto inline("" ::::foo); // note, this one should probably be added in permutations
  asm volatile inline goto("" ::::foo);
``` 

could just be distilled down to one of those since they are the same combination of qualifiers (combinations do not care about order). I would say that moving `combinations` into `permutations` would be wise since `permutations` tests the same thing that `combinations` does and more.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75563





More information about the cfe-commits mailing list