[PATCH] D90241: [openmp][openacc] Check for duplicate clauses for directive

Valentin Clement via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Oct 28 07:10:31 PDT 2020


clementval added inline comments.


================
Comment at: llvm/test/TableGen/directive3.td:30
+// CHECK: error: Clause TDLC_ClauseA already defined on directive TDL_DirA
+// CHECK: error: One or more clauses are defined multiple times on directive TDL_DirA
----------------
kiranchandramohan wrote:
> Nit: Is this second error giving any additional information? Is this twin errors so that we get all occurrences of errors rather than fail at the first one?
The first error is the specific error triggered for each duplicate found. The last one is the general error mentioning that one or more duplicates have been found. I'm happy to remove the global error if it is confusing. 


================
Comment at: llvm/utils/TableGen/DirectiveEmitter.cpp:151-162
   const auto &DirectiveLanguages =
       Records.getAllDerivedDefinitions("DirectiveLanguage");
 
   if (DirectiveLanguages.size() != 1) {
     PrintError("A single definition of DirectiveLanguage is needed.");
     return;
   }
----------------
kiranchandramohan wrote:
> Is this code from 151 to 162 similar for all occurrences of HasDuplicateClausesInDirectives? If so would it be better to pull it into a function and use that function?
I extract it to a function. I plan to add a wrapper for Records so we can get rid of all the `Records.getAllDerivedDefinitions("DirectiveLanguage");` and have simple accessor and also pass just a reference to the records to all those functions that now takes vector of Records. But that will be in a next patch. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90241



More information about the llvm-commits mailing list