benshi001 marked an inline comment as done. benshi001 added inline comments.
================ Comment at: clang/include/clang/Basic/DiagnosticFrontendKinds.td:159 +def err_verify_nonconst_addrspace : Error< + "qualifier 'const' is needed for variables in address space '%0'">; ---------------- Anastasia wrote: > Ok, do you plan to pass anything other than `__flash` here? If not then you > don't need to pass an argument in this diagnostic. Yes. There may be `__flash1 ... __flash5` in the future. ================ Comment at: clang/test/CodeGen/avr-flash.c:1 +// RUN: %clang_cc1 -triple avr -emit-llvm-only -verify %s + ---------------- Anastasia wrote: > benshi001 wrote: > > benshi001 wrote: > > > Anastasia wrote: > > > > If you are only checking the diagnostics you should pass: > > > > > > > > `-emit-llvm-only` -> `-syntax-only` > > > > > > > > and also it should be moved to `clang/test/Sema`. > > > This test can not run with `-syntax-only`. Since the check is performed > > > in the codegen stage. > > > > > > I would like to do the check in the Sema stage, but there is no target > > > specific code in the sema stage. And such checks are better to be put > > > into AVRTargetCodeGenInfo. > > I haved moved all `__flash` related to the CodeGen part, now it has nothing > > to do with `Sema/-syntax-only`. > > I would like to do the check in the Sema stage, but there is no target > > specific code in the sema stage. And such checks are better to be put into > > AVRTargetCodeGenInfo > > Sema has target hooks and access to the target specific address spaces so you > could indeed move them to Sema. Normally we do diagnostics as early as > possible but there is no strict rule and there are some tradeoffs. So, you > can also keep it here to allow better code incapsulation and simplify > maintenance. > > If you need to implement the pointer conversion logic you might move this > completely to `Sema`. Btw just to highlight the following change > https://reviews.llvm.org/D62574 that generalizes some logic that might be > useful for your pointer conversions. Thanks. I will try your way. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96853/new/ https://reviews.llvm.org/D96853 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
