Meinersbur added inline comments.

================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:4011
   unsigned NamedModifiersNumber = 0;
-  SmallVector<const OMPIfClause *, OMPC_unknown + 1> FoundNameModifiers(
-      OMPD_unknown + 1);
+  SmallVector<const OMPIfClause *, unsigned(OMPC_unknown) + 1>
+  FoundNameModifiers(unsigned(OMPD_unknown) + 1);
----------------
ABataev wrote:
> jdoerfert wrote:
> > ABataev wrote:
> > > jdoerfert wrote:
> > > > ABataev wrote:
> > > > > JonChesterfield wrote:
> > > > > > I wonder if it would be worth wrapping the accesses to 
> > > > > > FoundNameModifiers in functor that does the enum class to unsigned 
> > > > > > conversion. E.g. a class instance that contains the small vector 
> > > > > > and exposes operator[] that takes the enum class.
> > > > > > 
> > > > > > FoundNameModifiers[unsigned(val)] is quite a lot of line noise.
> > > > > Why `map`? It can be represented as a simple c-style array without 
> > > > > any problems.
> > > > No, these are not enums that auto-convert to unsigned.
> > > Convert them manually. Replacing the simple hash array with map does not 
> > > look like a good solution.
> > I feel this this is a micro optimization that will make the code less 
> > maintainable and, as Jon noted, "FoundNameModifiers[unsigned(val)] is quite 
> > a lot of line noise.". 
> > Take a look at the first version and let me know if you want to go back to 
> > that one for this part, if so, I can do that.
> You already introduced special wrapper class in the same file, maybe you can 
> reuse for implicit conversions?
We are introducing an `EnumeratedArray` class in 
https://reviews.llvm.org/D68827#change-XphX8PAWFr8V . It should reduce the need 
for casting and `OpenMPDirectiveKindExWrapper`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69853



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to