[PATCH] D93401: [flang][driver] Add support for `-D`, `-U`

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski added inline comments.


================
Comment at: flang/include/flang/Frontend/CompilerInvocation.h:89
+  /// These values are found in the preprocessor options.
+  void setFortranOpts();
 };
----------------
clementval wrote:
> @awarzynski I was looking at this file recently and I was wondering why we have different case style? Is there a particular reason? 
Thanks for pointing this out! See:https://reviews.llvm.org/D118381

>  Is there a particular reason?

Tl;Dr This is a typo - I blame the reviewers :)

**A bit longer discussion**
The driver strives to follow Flang's [[ https://github.com/llvm/llvm-project/blob/main/flang/docs/C%2B%2Bstyle.md | C++ style ]]. That was pretty much the only style used/available in LLVM Flang when the driver was introduced. Basically, it looked like the only available option. I didn't realise that in lowering and code-gen people decided to use the MLIR style. In hindsight, the driver should follow that too (to be better aligned with the rest of LLVM).

Flang's C++ style is very different to LLVM's, Clang's or MLIR's coding styles. Since the driver  "glues" these projects together, it effectively needs to keep switching between the styles (i.e. LLVM and Clang's APIs fallow their style, MLIR's APIs fallow MLIR style, and Flang's parser/sema APIs follow Flang's style). To me personally, this has been very confusing and a major pain point.

I think that it would be good for the overall health of the project if the driver was refactored to follow the MLIR style. What are you thoughts?


================
Comment at: flang/include/flang/Frontend/PreprocessorOptions.h:29
+#endif // LLVM_FLANG_PREPROCESSOROPTIONS_H
\ No newline at end of file

----------------
clementval wrote:
> You have a couple of these on your patch. 
Fixed in https://reviews.llvm.org/D97080


================
Comment at: flang/test/Flang-Driver/macro_def_undef.f90:39
+end
\ No newline at end of file

----------------
@clementval Fixed in https://reviews.llvm.org/D99292


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93401