lebedev.ri added inline comments.

================
Comment at: lib/Frontend/HeaderIncludeGen.cpp:55
+  // Simplify Filename that starts with "./"
+  if (Filename.startswith("./"));
+    Filename=Filename.substr(2);
----------------
skan wrote:
> lebedev.ri wrote:
> > skan wrote:
> > > craig.topper wrote:
> > > > skan wrote:
> > > > > lebedev.ri wrote:
> > > > > > xiangzhangllvm wrote:
> > > > > > > Need remove ";" ? 
> > > > > > This was fixed but no test was added?
> > > > > > Was some existing test failing previously? Which one?
> > > > > The test is in the file 'clang_H_opt.c'  which is included in this 
> > > > > patch.
> > > > The extra semicolon would have caused the body of the 'if' to execute 
> > > > unconditionally. Did any existing test case fail for that in your local 
> > > > testing? Or did you not test with that mistake?
> > > i fixed the mistake in the updated patch.  I ran the test in 
> > > 'clang_H_opt.c' alone for this patch. The extra semicolon caused the body 
> > > of `if` to exeute always, which didn't cause the test to fail. 
> > > The extra semicolon caused the body of if to exeute always, which didn't 
> > > cause the test to fail.
> > 
> > That is the question.
> > Is there test coverage with that error?
> No. I just run alll the tests for clang, and all of them can pass even if the 
> extra semicolon exits. If we want to add  a test to cover that error, we have 
> to add a headerfile outside the 'test/Driver' directory. Do we need to add 
> the test to cover that error?
Then yes please, do add test coverage for that bug :)


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

https://reviews.llvm.org/D62115



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

Reply via email to