zahiraam added inline comments.

================
Comment at: clang/lib/Frontend/FrontendAction.cpp:845-855
+  if (Input.isFile() && Input.getFile() == "-") {
+    Preprocessor &PP = CI.getPreprocessor();
+    std::string PredefineBuffer;
+    PredefineBuffer.reserve(4080);
+    llvm::raw_string_ostream Predefines(PredefineBuffer);
+    Predefines << PP.getPredefines();
+    MacroBuilder Builder(Predefines);
----------------
aaron.ballman wrote:
> zahiraam wrote:
> > aaron.ballman wrote:
> > > zahiraam wrote:
> > > > aaron.ballman wrote:
> > > > > I'm confused as to why we want to predefine this macro *only* when 
> > > > > the input source is stdin? So I'm not certain I understand why this 
> > > > > change is desired.
> > > > > 
> > > > > e.g., https://godbolt.org/z/E8Y67381r (note how there's no 
> > > > > `__FLT_EVAL_METHOD__` defined there)
> > > > I was offering a solution  to the issue raised by @glandium in 
> > > > https://reviews.llvm.org/D109239. I thought that the issue was only 
> > > > when the source is stdin, but obviously not.  The default setting 
> > > > should happen under no condition. 
> > > There have been a lot of reviews over this stuff, so I may be remembering 
> > > incorrectly... but I thought it was a conscious decision to *not* 
> > > predefine `__FLT_EVAL_METHOD__` because that value changes depending on 
> > > pragmas in the TU. I thought the way that macro worked was that we 
> > > registered it as a macro and we figure out its value at expansion time.
> > > 
> > > (The original report confused me into thinking that `__FLT_EVAL_METHOD__` 
> > > was behaving as though it was never defined, so expansion would never 
> > > result in a correct value except when the eval method is `0`.)
> > Yes, you are remembering correctly. I looked at older comments and it's 
> > something that was intended. We didn't want to pre-define it, until it is 
> > set at expansion.
> Thank you for double-checking! I would recommend that you abandon this 
> change, and instead make an NFC change to 
> https://clang.llvm.org/docs/UsersManual.html#a-note-about-flt-eval-method to 
> make it clear that this macro does not appear when dumping preprocessor macro 
> definitions but is instead resolved when expanding it as a macro definition.
Good idea! I seem to be regularly falling into forgetting this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124004

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

Reply via email to