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 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.
Great suggestion, done. Thanks!
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D75563/new/
https://reviews.llvm.org/D75563
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
